zlib auto-reset on trailing data causes repeated flushing

[chrisdickinson]

6f6a979 changed the behavior of the zlib module's decompression transform (specifically Zlib.prototype._processChunk): if an incoming chunk would end the transform, but has trailing data, the transform resets itself and re-sends the chunk, optimistically assuming that the chunk represents a concatenated stream. However, it looks like this behavior can hang Node, and breaks some tests in npm. I'm looking into this at present, but want to make others aware!

[chrisdickinson]

Per @othiym23, this seems like a pretty severe regression:

We should get this resolved before v0.11.15 is released, because npm install will break for a substantial number of users immediately.

[othiym23]

It seems that any file in an npm tarball that's more than one stream-chunk long will trigger this behavior in v0.11.15-pre, causing any attempt to unpack / install the related package will cause npm to hang, potentially using a large amount of CPU. This probably affects most other applications using zlib as part of a pipeline as well.

[chrisdickinson]

A semi-related reproduction. Before the patch, this code would write once and then halt -- afterward, it errors (so, that's one breaking change).

I've come around to the idea that zlib's transform streams shouldn't transparently handle decompressing concatenated compressed streams -- really, an outer stream should be in charge of that, and all node should be concerned with is giving the client the leftover bytes so they can manually .reset() or create a new zlib transform and re-write the remaining bytes.

[chrisdickinson]

Update: An actual reproduction! Zero Any bytes trailing a compressed stream that is being decompressed with Unzip blows up.

[chrisdickinson]

Made a patch to fix the reproduction. Unfortunately npm still blows up with this patch, though at least it's ending the stream now.

npm encounters the following error:

56 error argv "/Users/chris/projects/iojs/node" "/Users/chris/projects/iojs/npm/bin/npm-cli.js" "install"
57 error node v0.11.15-pre
58 error npm  v2.1.17
59 error code Z_DATA_ERROR
60 error errno -3
61 error unknown compression method
62 error If you need help, you may report this error at:
62 error     <http://github.com/npm/npm/issues>
63 verbose exit [ -3, true ]

Since there are trailing "zero" bytes in the tar.gz it's trying to decompress, it blows up when trying to restart the stream.

Edit: those trailing zeros may be due to block-stream.

[chrisdickinson]

/cc @luismreis, who originally authored the patch.

[chrisdickinson]

I suppose the remaining questions are:

1. Do we back this out for now and attempt to reintroduce later?
2. When re-introducing this, do we want to reintroduce this (roughly) as-is, or as an additional option for the zlib decompression stream constructors?

[rmg]

I had to rollback my CI environments to work around this. I couldn't run npm install to completion on any module being tested. That seems like a pretty critical breakage.

@chrisdickinson your proposal to revert for now (0.11.15, I'm guessing?) and reintroduce later as an opt-in option sounds reasonable to me.