Fix maximum call stack size exceeded

[EvanOxfeld]

Most of the way fix for #16. Working in node 0.10.3 on jquery-ui-1.10.0.custom.zip but not in node 0.8.x testing against

var unzip = require('unzip');
var fs = require('fs');

fs.createReadStream('jquery-ui-1.10.0.custom.zip')
    .pipe(unzip.Extract({ path: 'test' }))
    .on('error', function (err) { console.log('error', err); })
    .on('close', function () { console.log('closed'); });

Current hang up is 34f30cc and more specifically the MatchStream starting on parse.js line 158. For archives that contain variable length file data, unzip.Parse needs to stop sending data to the zlib inflater stream when the data descriptor signature is reached. In Node 0.10.x the MatchStream stops the flow of data correctly because the process.nextTick on line 165 happens on the current tick before I/O. Unfortunately, in Node 0.8.x I/O can happen before a process.nextTick, which causes extra writes to the MatchStream and erroneous data to flow into the zlib inflater stream.

Feedback is much appreciated.

/cc @satazor

[EvanOxfeld]

To be node 0.8.x compatible, Parse can't process variable length file data in a way that's dependent on process.nextTick behavior - this is what currently happens inside of the function passed to the MatchStream constructor in Parse which is called when the MatchStream reaches the end of the file length data, indicated by the data descriptor signature.

The solution I'm kicking around is to modify the match-stream module's API to take the original function called once the descriptor has been reached as well as a callback that fires when the match-stream ends. The new callback would return any data additional data (i.e. data past the descriptor signature) stored in the MatchStream's internal read queue. So instead this section of Parse looks something like this:

...
//if (!fileSizeKnown)
var descriptorSig = new Buffer(4);
descriptorSig.writeUInt32LE(0x08074b50, 0);

var matchStream = new MatchStream({ pattern: descriptorSig }, function (buf, matched) {
  if (!matched) {
    return hasEntryListener ? this.push(buf) : null;
  }
  if (hasEntryListener) {
    this.push(buf);
  }
  return this.push(null); //end matchStream
}, function (err, extra) {
  if (err) return self.emit(err);
  self._pullStream.prepend(extra);
  self._processDataDescriptor(entry);
});

self._pullStream.pipe(matchStream);
if (hasEntryListener) {
  matchStream.pipe(inflater).pipe(entry);
}

[EvanOxfeld]

I added 548c16b, but entries sometimes fail to emit end using node 0.8.22 and jquery-ui-1.10.0.custom.zip. All tests pass in node 0.10.3.

[satazor]

@EvanOxfeld good to know you're almost there. I don't know if you're testing a lot of zip files but it would be good to test zip files generated by common tools on unix, windows and macosx so cover most cases.

Tell me if you need help getting those integrated.

[danheberden]

Please see #24 regarding tests and persisting failures

[danheberden]

pulling in this branch causes this

node 0.8.22:

node 0.10.3:

[satazor]

Bump!

[satazor]

@EvanOxfeld we've managed to fix bower extraction of zip files for both node 0.8.x and 0.10.x on unzip 0.1.7 with the following hack:

zlib.Z_DEFAULT_CHUNK = 1024*8;

Hope that helps.

[jhurliman]

Bump

[paulirish]

please merge, @nearinfinity ! :)

[inolen]

Has there been any progress on this? I was trying out this library tonight and unfortunately hit this snag on one of the zip files I was working with, and this branch did indeed fix the issue.

[antoine-richard]

Thanks for this patch which fixes the issue here on a node 0.10.
Any news on getting this PR merged @EvanOxfeld ?

[inolen]

Circled back to the project depending on this, this still appears to be an issue.

Using this branch alone no longer resolves the problem, however, updating the references to process.nextTick in the pullstream dependency seems to do the trick.

Is there any chance this will be resolved soon?

[magalhas]

I faced this today while unzipping a big file.Is this getting fixed any time soon ?

[garthk]

@EvanOxfeld, you haven't made any changes for 9 months; @joeferner for 2 years. You're active with other projects, just not this one. Meanwhile, there's plenty of activity around this project driven by its need for maintenance.

It might be time to npm owner add the maintainer of one of the leading forks so they can publish their fixes, tackle the other 30+ open bugs, and generally maintain the module for the users and 70+ published dependents.

@enyojs, @YuriMilchev, @tsouza, @shannonmpoole, @lloydsheng, @kenvifire, @zauberlabs, @jeanlauliac, @TimNZ, @wibblymat, and @proxv: if @EvanOxfeld stays dormant, or if he wakes up but only to hand it over: around which of your forks should we consolidate, merge #23, apply all your fixes to entry.size etc, and publish to NPM as unzip or unzip2 depending?

[shannon]

I would have to say my fork probably isn't the best choice. I just tweaked the size check and have since moved on from using the library all together. I've found I could accomplish what I needed using zlib and tar.

[prust]

See #50 (Should this project have a future?) for related discussion.