Repository navigation
Node v15.6.0 changes the behavior of .end on zlib.DeflateRaw #37027
Description
- Version: v15.6.0
- Platform: Linux ArchLaptop 5.10.9-arch1-1 deps: update openssl to 1.0.1j #1 SMP PREEMPT Tue, 19 Jan 2021 22:06:06 +0000 x86_64 GNU/Linux
- Subsystem: zlib
What steps will reproduce the bug?
The following code:
const {DeflateRaw} = require('zlib');
class MyStream extends DeflateRaw {
constructor(options) {
super(options);
}
write(chunk, enc, cb) {
console.log("Write: " + chunk + " " + enc + " " + cb);
}
}
console.log("Running");
const s = new MyStream();
s.end(Buffer.from('hello world'));produces the following output on Node v15.6.0:
Running
On Node v15.5.0 and below, it produces the output:
Running
Write: hello world undefined undefined
How often does it reproduce? Is there a required condition?
This reproduces 100% of the time
What is the expected behavior?
Calling end should call the overridden write method with the supplied data, as in Node v15.5.0 and below.
What do you see instead?
The overridden write method is not called.
Additional information
This leads to node-archiver producing invalid ZIP files: archiverjs/node-archiver#491
PR #36618 seems like it might be related
Activity
Trott commentedon Jan 22, 2021
@nodejs/zlib @nodejs/streams
lpinca commentedon Jan 23, 2021
The issue seems to be caused by a4fce32. Reverting it makes the example work as expected.
mcollina commentedon Jan 23, 2021
@ronag wdyt? I have a feeling we should be reverting a4fce32 and add a unit test.
lpinca commentedon Jan 23, 2021
I think a4fce32 is fine. The documentation does not mention that
writable.end()callswritable.write()andwritable._write()is still called as expected.I would only revert on v15.x and mark #36817 as semver-major.
pgrzesik commentedon Jan 27, 2021
Hello 👋 To clarify - @lpinca - does that mean that the original PR (#36817) that caused issues for some packages depending on undocumented behavior will be merged to the next major version? Thanks in advance 🙇
mcollina commentedon Jan 27, 2021
Yes, we do not plan to revert on master, only on v15.
pgrzesik commentedon Jan 27, 2021
Thanks for clarification @mcollina 🙇
lpinca commentedon Mar 5, 2021
I'm closing this as it was addressed by #37060.
severn-everett commentedon Jun 3, 2021
@mcollina Why was this not reverted on master? I'm running into the same issue on 16.3.0.
ronag commentedon Jun 3, 2021
@severn-everett Because the new behavior is considered correct.
fix: Downgrade node version