Skip to content

Commit 96593b0

Browse files
xia-chaoxia-chao
committed
zlib: refuse incomplete-member reset only for gzip
zlib-wrapped deflate still allows reset after flush. The dictionary test discards the first member and reuses the compressor. Signed-off-by: Xia Chao <shapirolutts@gmail.com>
1 parent 5e511b4 commit 96593b0

3 files changed

Lines changed: 103 additions & 86 deletions

File tree

‎doc/api/zlib.md‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,13 +2198,14 @@ For Zstd streams, cancel the current frame and start a new session while
21982198
preserving the configured parameters and dictionary. If `pledgedSrcSize` was
21992199
configured for a Zstd compressor, it applies again to the next frame.
22002200

2201-
Resetting a gzip or zlib-wrapped deflate stream after it has emitted output
2202-
for an incomplete member causes the stream to error with
2203-
`ERR_ZLIB_INCOMPLETE_FRAME`. Resetting at that point would discard the
2204-
member state while the bytes already written out remain at the start of the
2205-
output stream, leaving it undecodable. Call `.end()`, or start over with a
2206-
new stream, instead. Raw deflate has no wrapper header, so `reset()` after a
2207-
flush still concatenates.
2201+
Resetting a gzip stream after it has emitted output for an incomplete member
2202+
causes the stream to error with `ERR_ZLIB_INCOMPLETE_FRAME`. Resetting at
2203+
that point would discard the member state while the bytes already written out
2204+
remain at the start of the output stream, leaving it undecodable. Call
2205+
`.end()`, or start over with a new gzip stream, instead.
2206+
zlib-wrapped deflate may still `reset()` after a flush; callers that reuse
2207+
the compressor discard the first output. Raw deflate has no wrapper header,
2208+
so `reset()` after a flush still concatenates.
22082209

22092210
Calling `reset()` while a write is in progress throws an `Error`.
22102211

‎src/node_zlib.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,16 +1253,16 @@ CompressionError ZlibContext::GetErrorInfo() const {
12531253

12541254

12551255
CompressionError ZlibContext::ResetStream() {
1256-
// deflateReset() is deflateEnd + deflateInit: a new stream. Bytes already
1257-
// written out cannot be taken back, so refuse reset on wrapper formats
1258-
// (gzip / zlib deflate) once an incomplete member has emitted output.
1259-
// Unflushed internal state alone is cancelled by deflateReset; raw deflate
1260-
// has no wrapper header, so flush+reset still concatenates.
1261-
if ((mode_ == GZIP || mode_ == DEFLATE) && !stream_complete_ &&
1262-
output_emitted_) {
1256+
// deflateReset() is deflateEnd + deflateInit: a new stream. gzip emits a
1257+
// wrapper header on the first write; those bytes cannot be taken back, so
1258+
// refuse reset once an incomplete gzip member has emitted output.
1259+
// zlib-wrapped deflate still allows reset after flush: callers discard the
1260+
// first member (test-zlib-dictionary.js). Raw deflate has no wrapper header,
1261+
// so flush+reset still concatenates.
1262+
if (mode_ == GZIP && !stream_complete_ && output_emitted_) {
12631263
return CompressionError(
1264-
"Cannot reset a zlib stream with an incomplete member; end the "
1265-
"stream or discard the output produced so far",
1264+
"Cannot reset a gzip stream with an incomplete member; end the "
1265+
"stream or start a new gzip compressor",
12661266
"ERR_ZLIB_INCOMPLETE_FRAME",
12671267
Z_STREAM_ERROR);
12681268
}

‎test/parallel/test-zlib-reset-incomplete-output.js‎

Lines changed: 86 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
'use strict';
22

3-
// Tests that reset() refuses to run on gzip / zlib-wrapped deflate once an
4-
// incomplete member has already emitted output.
3+
// Tests that reset() refuses to run on gzip once an incomplete member has
4+
// already emitted output.
55
//
6-
// deflateReset() is equivalent to deflateEnd + deflateInit. Bytes that have
7-
// already been written out cannot be taken back, so the next member would be
8-
// appended to that fragment and gunzip/inflate fail with Z_DATA_ERROR.
6+
// deflateReset() is equivalent to deflateEnd + deflateInit. gzip writes a
7+
// header on the first write, so those bytes cannot be taken back.
98
//
10-
// gzip and zlib deflate emit a header on the first write, so write-then-reset
11-
// is already unsafe. Raw deflate has no wrapper header: a small write may emit
9+
// zlib-wrapped deflate still allows reset after flush: official
10+
// test-zlib-dictionary.js discards the first member and reuses the
11+
// compressor. Raw deflate has no wrapper header: a small write may emit
1212
// nothing, and flush+reset still concatenates.
1313

1414
require('../common');
@@ -29,77 +29,93 @@ async function writeHello(stream) {
2929
});
3030
}
3131

32-
for (const [name, create, decompress] of [
33-
['Gzip', zlib.createGzip, zlib.gunzipSync],
34-
['Deflate', zlib.createDeflate, zlib.inflateSync],
35-
]) {
36-
test(`${name} reset throws when write has emitted wrapper output`,
37-
async () => {
38-
const stream = create();
39-
const chunks = [];
40-
stream.on('data', (chunk) => chunks.push(chunk));
41-
42-
await writeHello(stream);
43-
assert.ok(Buffer.concat(chunks).length > 0);
44-
45-
stream.reset();
46-
stream.end(Buffer.from('world'));
47-
48-
await assert.rejects(finished(stream), {
49-
code: 'ERR_ZLIB_INCOMPLETE_FRAME',
50-
});
51-
});
52-
53-
test(`${name} reset throws when flush has emitted incomplete output`,
54-
async () => {
55-
const stream = create();
56-
const chunks = [];
57-
stream.on('data', (chunk) => chunks.push(chunk));
32+
test('Gzip reset throws when write has emitted wrapper output', async () => {
33+
const stream = zlib.createGzip();
34+
const chunks = [];
35+
stream.on('data', (chunk) => chunks.push(chunk));
5836

59-
stream.write(Buffer.from('hello'));
60-
await new Promise((resolve) => stream.flush(resolve));
61-
assert.ok(Buffer.concat(chunks).length > 0);
37+
await writeHello(stream);
38+
assert.ok(Buffer.concat(chunks).length > 0);
6239

63-
stream.reset();
64-
stream.end(Buffer.from('world'));
40+
stream.reset();
41+
stream.end(Buffer.from('world'));
6542

66-
await assert.rejects(finished(stream), {
67-
code: 'ERR_ZLIB_INCOMPLETE_FRAME',
68-
});
69-
});
43+
await assert.rejects(finished(stream), {
44+
code: 'ERR_ZLIB_INCOMPLETE_FRAME',
45+
});
46+
});
7047

71-
test(`${name} flush followed by end still produces a valid stream`,
72-
async () => {
73-
const stream = create();
74-
const chunks = [];
75-
stream.on('data', (chunk) => chunks.push(chunk));
76-
77-
stream.write(Buffer.from('hello'));
78-
await new Promise((resolve) => stream.flush(resolve));
79-
stream.end(Buffer.from('world'));
80-
await finished(stream);
81-
82-
assert.strictEqual(
83-
decompress(Buffer.concat(chunks)).toString(),
84-
'helloworld',
85-
);
86-
});
48+
test('Gzip reset throws when flush has emitted incomplete output', async () => {
49+
const stream = zlib.createGzip();
50+
const chunks = [];
51+
stream.on('data', (chunk) => chunks.push(chunk));
8752

88-
test(`${name} reset before any write still works`, async () => {
89-
const stream = create();
90-
const chunks = [];
91-
stream.on('data', (chunk) => chunks.push(chunk));
53+
stream.write(Buffer.from('hello'));
54+
await new Promise((resolve) => stream.flush(resolve));
55+
assert.ok(Buffer.concat(chunks).length > 0);
9256

93-
stream.reset();
94-
stream.end(Buffer.from('hello'));
95-
await finished(stream);
57+
stream.reset();
58+
stream.end(Buffer.from('world'));
9659

97-
assert.strictEqual(
98-
decompress(Buffer.concat(chunks)).toString(),
99-
'hello',
100-
);
60+
await assert.rejects(finished(stream), {
61+
code: 'ERR_ZLIB_INCOMPLETE_FRAME',
10162
});
102-
}
63+
});
64+
65+
test('Gzip flush followed by end still produces a valid stream', async () => {
66+
const stream = zlib.createGzip();
67+
const chunks = [];
68+
stream.on('data', (chunk) => chunks.push(chunk));
69+
70+
stream.write(Buffer.from('hello'));
71+
await new Promise((resolve) => stream.flush(resolve));
72+
stream.end(Buffer.from('world'));
73+
await finished(stream);
74+
75+
assert.strictEqual(
76+
zlib.gunzipSync(Buffer.concat(chunks)).toString(),
77+
'helloworld',
78+
);
79+
});
80+
81+
test('Gzip reset before any write still works', async () => {
82+
const stream = zlib.createGzip();
83+
const chunks = [];
84+
stream.on('data', (chunk) => chunks.push(chunk));
85+
86+
stream.reset();
87+
stream.end(Buffer.from('hello'));
88+
await finished(stream);
89+
90+
assert.strictEqual(
91+
zlib.gunzipSync(Buffer.concat(chunks)).toString(),
92+
'hello',
93+
);
94+
});
95+
96+
test('Deflate reset after flush still works when first output is discarded',
97+
async () => {
98+
const stream = zlib.createDeflate();
99+
const chunks = [];
100+
let take = false;
101+
stream.on('data', (chunk) => {
102+
if (take) {
103+
chunks.push(chunk);
104+
}
105+
});
106+
107+
stream.write(Buffer.from('hello'));
108+
await new Promise((resolve) => stream.flush(resolve));
109+
stream.reset();
110+
take = true;
111+
stream.end(Buffer.from('world'));
112+
await finished(stream);
113+
114+
assert.strictEqual(
115+
zlib.inflateSync(Buffer.concat(chunks)).toString(),
116+
'world',
117+
);
118+
});
103119

104120
test('DeflateRaw reset after write without emitted output still works',
105121
async () => {

0 commit comments

Comments
 (0)