node-unzip icon indicating copy to clipboard operation
node-unzip copied to clipboard

Published 20 hours ago • verified publisher icon EvanOxfeld

Unzip stops prematurely in Node 0.10.0

Open EvanOxfeld opened this issue 11 years ago • 21 comments

Unzip stops prematurely in Node 0.10.0 without an error when inflating compressed files that are larger than the zlib stream's highWaterMark (16 kb). I believe the issue is the line self._untilStream.pipe(vars.compressedSize, inflater).pipe(entry) - the inflater stream never emits 'end' or 'finish'.

Code:

if (fileSizeKnown) {
  entry.size = vars.uncompressedSize;
  self._untilStream.pipe(vars.compressedSize, inflater).pipe(entry);
}

The error surfaced while I was investigating #19.

EvanOxfeld avatar Mar 13 '13 01:03 EvanOxfeld

I'm having a similar issue with Node 0.8 following d40cead09a8aa184c4bc5294f0665a0bcdf9f803 (which I think is related to the readable-stream version change), although this is for unknown file sizes. For some files, the final drain event on inflator isn't being emitted. Works ok with 1.4 + my fix for #16, but 1.5 hangs as the Parser never emits an 'end'. Fixes itself again with node 0.10.0 plus my

Sounds like it could be the same thing or shall I open a new issue?

timsweb avatar Mar 13 '13 09:03 timsweb

Thanks, @timsweb. I believe I'm having the same problem with the inflater stream - the readable side of that stream awaits a drain event that's never emitted. The stream instances in Node 0.10.0 are equivalent to Node 0.8.x + the readable-stream module upgraded in d40cead. Let's leave it as one issue for now

EvanOxfeld avatar Mar 13 '13 10:03 EvanOxfeld

Just to update everyone I haven't been able to reproduce the error with a pullstream test case.

Perhaps the issue is with writing data greater than the zlib stream's highWaterMark - there were changes to how zlib flushes its buffer between Node 0.9.12 and Node 0.10.0.

Next step is for a reduced test case using zlib.

EvanOxfeld avatar Mar 14 '13 17:03 EvanOxfeld

@EvanOxfeld In node 0.10.0, I'm getting the following error:

TypeError: listener must be a function
    at TypeError (<anonymous>)
    at Parse.EventEmitter.once (events.js:171:11)
    at Extract._write (/Users/satazor/Work/twitter/bower/node_modules/unzip/lib/extract.js:59:23)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at Extract.Writable.write (_stream_writable.js:172:11)
    at write (_stream_readable.js:547:24)
    at flow (_stream_readable.js:556:7)
    at ReadStream.pipeOnReadable (_stream_readable.js:588:5)
    at ReadStream.EventEmitter.emit (events.js:92:17)

In this related to your code directly or a dependency?

satazor avatar Mar 17 '13 10:03 satazor

You need to depend on pullstream 2.1 -> https://github.com/nearinfinity/node-pullstream/pull/5. It broke due to a change in the flush method for node 10.

Bower depends on unzip and that's why I'm stuck installing my new yeoman-based project with a fresh node install :wink:

edit: wait a minute. seems like bower is using an ancient version of this package. apologies!

SpoBo avatar Mar 17 '13 15:03 SpoBo

@SpoBo we locked to 0.1.4 because 0.1.5 caused unzip to never fire the callback. The fix for node 0.10.0 made it 0.8.x incompatible. @EvanOxfeld can you please look at this?

satazor avatar Mar 17 '13 15:03 satazor

After resolving nearinfinity/node-pullstream#6 without resolving this unzip issue, I'm more confident that the problem results from last minute changes to Transform streams in 0.10. I'll troubleshoot some more today, and in the unlikely event that there's a problem with Node itself, get a reduced test case out to the Node project.

Sorry that this issue remains open. I kept up fairly closely with 0.9.x and had hoped that 0.10 wouldn't break unzip and some of my other libraries.

EvanOxfeld avatar Mar 17 '13 20:03 EvanOxfeld

Also perhaps this issue is related to joyent/node#5051. I still need to confirm, though.

EvanOxfeld avatar Mar 17 '13 20:03 EvanOxfeld

The commit in node that created this issue is related to Transforms streams, not zlib - joyent/node@e26622bd18fc86033cea393125cad49c577b524b. While I'd like a resolution on why the zlib inflateRaw stream stalls and stops unzip prematurely, perhaps one temporary option is to use the stream base classes from readable-stream @0.2.0 rather than from node 0.10.

Also here's the reduced test case I've worked on. One major difference between the test case and the unzip module is that pullstream is a long-running passthrough stream writing to a zlib inflateRaw stream per file within a zip archive.

EvanOxfeld avatar Mar 18 '13 11:03 EvanOxfeld

Just a +1 to say that I'm getting this same issue under node 0.10.0. Good luck fixing it guys, this library looks so much nicer than any others I've seen for handling zip files, so I hope it'll be usable again soon.

simonpb avatar Mar 20 '13 11:03 simonpb

I am seeing the same problem under node 0.8.22. Stepping through the code and it almost seems to have something to do with the DirWriter. The Directory close event happens while the file within it is still streaming. Strange. Has anyone figured this out?

Looking at the DirWriter trace:

/Users/username/project/node_modules/phantomjs/tmp setting size to undefined
Archive:  /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx.zip
W create /Users/username/project/node_modules/phantomjs/tmp 493
W created /Users/username/project/node_modules/phantomjs null
   creating: phantomjs-1.8.2-macosx/
    add undefined -> /Users/username/project/node_modules/phantomjs/tmp
DW Process p=undefined tmp
DW Entry undefined
DW not recursive



/Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/ setting size to undefined
W create /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx 493
   creating: phantomjs-1.8.2-macosx/bin/
    add undefined -> /Users/username/project/node_modules/phantomjs/tmp
DW Process p=true tmp
W created /Users/username/project/node_modules/phantomjs/tmp null
  inflating: phantomjs-1.8.2-macosx/bin/phantomjs
    add undefined -> /Users/username/project/node_modules/phantomjs/tmp
DW Process p=true tmp
DW Child Ready Directory /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx
  resuming undefined
 === open the pipes /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx
 ==== unblock entry undefined
DW Process p=undefined phantomjs-1.8.2-macosx/
DW Drain
DW Process p=undefined phantomjs-1.8.2-macosx/
DW Drain
 W Finish /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx undefined
 W Finish Stating /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx undefined
 W Finish Stated /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx undefined { dev: 16777218,
  mode: 16877,
  nlink: 2,
  uid: 501,
  gid: 501,
  rdev: 0,
  blksize: 4096,
  ino: 63598345,
  size: 68,
  blocks: 0,
  atime: Mon Mar 25 2013 18:10:12 GMT-0400 (EDT),
  mtime: Mon Mar 25 2013 18:10:12 GMT-0400 (EDT),
  ctime: Mon Mar 25 2013 18:10:12 GMT-0400 (EDT) }
   W Finish chmod 3
   W Finish chown 2
   W Finish utimes 1
* DW Child end phantomjs-1.8.2-macosx/
DW Process p=false tmp
DW Entry undefined
DW not recursive



/Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/ setting size to undefined
W create /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin 493
W created /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx null
DW Child Ready Directory /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin
  resuming undefined
 === open the pipes /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin
 ==== unblock entry undefined
DW Process p=undefined bin/
DW Drain
DW Process p=undefined bin/
DW Drain
 W Finish /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin undefined
 W Finish Stating /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin undefined
 W Finish Stated /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin undefined { dev: 16777218,
  mode: 16877,
  nlink: 2,
  uid: 501,
  gid: 501,
  rdev: 0,
  blksize: 4096,
  ino: 63598436,
  size: 68,
  blocks: 0,
  atime: Mon Mar 25 2013 18:10:54 GMT-0400 (EDT),
  mtime: Mon Mar 25 2013 18:10:54 GMT-0400 (EDT),
  ctime: Mon Mar 25 2013 18:10:54 GMT-0400 (EDT) }
   W Finish chmod 3
   W Finish chown 2
   W Finish utimes 1
* DW Child end bin/
DW Process p=false tmp
DW Entry undefined
DW not recursive



/Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/phantomjs setting size to undefined
W create /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/phantomjs 493
W created /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin null
FW open [] /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/phantomjs
DW Child Ready File /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/phantomjs
  resuming undefined
 === open the pipes /Users/username/project/node_modules/phantomjs/tmp/phantomjs-1.8.2-macosx/bin/phantomjs
 ==== unblock entry undefined
    -- fw wrote, _stream says false 0
    -- fw wrote, _stream says false 1
    -- fw wrote, _stream says false 1
    -- fw wrote, _stream says false 0
    -- fw wrote, _stream says false 0
    -- fw wrote, _stream says false 0
process.on("exit") completion called

richtera avatar Mar 25 '13 15:03 richtera

@EvanOxfeld What's the status of this? Do you need help? If so let me know. A lot of bower users are experience issues because of this and I would like to release a new version with this solved.

satazor avatar Mar 30 '13 18:03 satazor

@satazor I'm working on this issue this morning, using node 0.10.2, and trying to isolate the problem to node-unzip, node-pullstream or node itself. I could use some help building a reduced test case. The one I've written up so far is here - https://gist.github.com/EvanOxfeld/5186611

Sorry for the bower issues. I definitely want to fix node-unzip for node 0.10 as fast as possible.

EvanOxfeld avatar Apr 01 '13 12:04 EvanOxfeld

@richtera Thanks for DirWriter stack trace. Although unzip.Parse() fails and does not use the fstream dependency, perhaps there's an issue with fstream as well.

EvanOxfeld avatar Apr 01 '13 12:04 EvanOxfeld

Likely unzip.Parse doesn't complete because the entry streams emitted by unzip.Parse need to be consumed by being piped or having read() called on them, otherwise they exert backpressure upstream. Interestingly, changing entry.js line 15 to PassThrough.call(this, { highWaterMark: 64 * 1024 }) (or an even larger highWaterMark) extends parsing, but parsing may not finish. Still, I'm not sure why increasing the entry stream's high water mark has a collective effect on parsing the entire archive when there's an entry stream per archived file.

@satazor However, in the case of unzip.Extract the entry streams should be consumed by the fstream DirWriter. I'm hoping unzip.Extract just needs removal of the recursive process.nextTick() calls that blow up the call stack. I'll get back to unzip.Extract tonight, but in the meantime if anyone has a chance to investigate further, your efforts would be much appreciated.

@richtera Given the previous paragraph about unzip.Extract, your stack trace should be handy for this issue after all. Thanks again for your help.

EvanOxfeld avatar Apr 01 '13 13:04 EvanOxfeld

A fix for unzip.Parse is coming soon, then I'll look into the recursive process.nextTick calls causing unzip.Extract to blow up.

EvanOxfeld avatar Apr 02 '13 00:04 EvanOxfeld

Made the below updates that should resolve the unzip.Parse issues. Thanks to @wanderview for the suggestion.

  • If you never register an on('entry') listener, unzip.Parse consumes and throws away the same bytes that would be piped through to an entry stream.
  • If you register an on('entry') listener, call entry.autodrain() on each entry with contents you don't want to consume. autodrain() will throw away any bytes written to a given entry stream.

EvanOxfeld avatar Apr 02 '13 03:04 EvanOxfeld

I went ahead and published the unzip.Parse fixes (outlined in my prior comment) to NPM as 0.1.6.

EvanOxfeld avatar Apr 02 '13 03:04 EvanOxfeld

Do you know that Bower is still broken? https://github.com/twitter/bower/pull/377#issuecomment-16017577

The comment suggests that Bower folks will not update it until unzip will work with both 0.8 and 0.10. Can you put a check for each version and execute accordingly? Bower is used by a lot of projects that need this to work.

clavecoder avatar Apr 08 '13 02:04 clavecoder

Hello I was having an issue with close not being called when these errors were happening for parser.on('error'):

entry:

Error: invalid signature: 0xa
    at /var/www/test/unzip-test/node_modules/unzip/lib/parse.js:59:13
    at processImmediate [as _immediateCallback] (timers.js:345:15)

entry:

Error: invalid code lengths set
    at Zlib._binding.onerror (zlib.js:295:17)

I ended up googling around and found this issue:

https://github.com/EvanOxfeld/node-unzip/issues/47

So i tried cloning this repo:

https://github.com/AnkurMali/node-unzip

And this fixed my problem,. The errors completely went away and I was able to extract the file (no error) and the close method was being called.

ashelley avatar Feb 13 '15 23:02 ashelley

Hi, I installed from npm, used as:

fs
            .createReadStream(req.file.path)
            .pipe(
              unzip.Extract({
                path: webBuildFolder
              }).on('close', function() {
                console.log('NOW!');
              })
            )

And getting error with Mac packed archives:

invalid signature: 0xa

tomitrescak avatar Oct 24 '17 06:10 tomitrescak