mafintosh / tar-stream

tar-stream is a streaming tar parser and generator.
MIT License
408 stars 92 forks source link

make sure to emit error when entry handling is done #39

Closed fujifish closed 9 years ago

fujifish commented 9 years ago

If the extractor gets destroyed during entry processing and an error is then given to onunlock then the error is not emitted at all.

In my case this happened when using tar-fs and the 'pump' method encountered an error extracting an entry. The extractor got destroyed through the pump destroyer method without being given the error, and when tar-fs unlocked the entry the extractor was already destroyed, so the error was never emitted.

To reproduce, create a tar that contains a file with read only permission, and try to extract it twice to the same place. There should be a "EACCES" error emitted when trying to override the read only file during the second extraction.

fujifish commented 9 years ago

Any chance you'll be able to pull this soon?

mafintosh commented 9 years ago

thanks for the pr. i've been traveling but will get to this tonight or tomorrow

mafintosh commented 9 years ago

i tried reproducing the error you described above with no luck. I seem to always get an error emitted. Could you send me a test case to reproduce it?

fujifish commented 9 years ago

It seems that it only happens on large enough files. The file size I tested with was ~40K. With small files the error event is correctly emitted.

Here's the code to reproduce:

var fs = require('fs');
var path = require('path');
var zlib = require('zlib');
var tar = require('tar-fs');

var source = '/tmp/tartest.tgz';
var target = '/tmp/tartest';

var extractor = tar.extract(target);

extractor.on('error', function(err) {
  console.log(err);
});

extractor.on('finish', function() {
  console.log('finished!');
});

extractor.on('close', function() {
  console.log('closed!');
});

// the file reader
var reader = fs.createReadStream(source);

// the gunzipper
var gunzip = zlib.createGunzip();

reader.pipe(gunzip).pipe(extractor);

I created /tmp/tartest.tgz that contains two files:

  1. hello.txt, mod 0444, size 40K
  2. world.txt, mod 0666, size whatever

Running the test scripts for the first time prints finished! as expected. Running it a second time I would expect the error to be emitted followed by the closed! message, but only the closed message is displayed. Performing the same with a small hello.txt file prints the error message correctly.

fujifish commented 9 years ago

Any updates?

mafintosh commented 9 years ago

Ok I can reproduce now. Interestingly it only happens if the tarball is gzipped which makes it seem to me like there is something else weird going on.

mafintosh commented 9 years ago

tar-fs 1.7.0 now always forwards errors so this should be fixed. I'd like to keep the current behaviour of tar-stream where you have to use .destroy(err) to emit an error. Sorry for keeping this open for a while - I've been busy recently :)