Skip to content

Commit 9046035

Browse files
Ic3b3rgrichardlau
authored andcommitted
zlib: validate flush king for all streams
Signed-off-by: Ic3b3rg <s.ceccarini94@gmail.com> PR-URL: #63746Fixes: #63701 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 98be430 commit 9046035

2 files changed

Lines changed: 72 additions & 56 deletions

File tree

‎lib/zlib.js‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -358,17 +358,10 @@ ZlibBase.prototype.flush = function(kind, callback) {
358358
kind=this._defaultFullFlushFlag;
359359
}
360360

361-
// Reject kinds outside the brotli operation range. Otherwise the fake-chunk
362-
// path forwards an unsupported flush flag to the native encoder/decoder,
363-
// which spins at 100% CPU without making progress (see #63701).
364-
if(this._flushBoundIdx===FLUSH_BOUND_IDX_BROTLI){
365-
constmin=FLUSH_BOUND[FLUSH_BOUND_IDX_BROTLI][0];
366-
constmax=FLUSH_BOUND[FLUSH_BOUND_IDX_BROTLI][1];
367-
if(typeofkind!=='number'||NumberIsNaN(kind)||
368-
kind<min||kind>max){
369-
thrownewERR_OUT_OF_RANGE('kind',`>= ${min} and <= ${max}`,kind);
370-
}
371-
}
361+
kind=checkRangesOrGetDefault(
362+
kind,'kind',
363+
FLUSH_BOUND[this._flushBoundIdx][0],FLUSH_BOUND[this._flushBoundIdx][1],
364+
this._defaultFullFlushFlag);
372365

373366
if(this.writableFinished){
374367
if(callback)
Lines changed: 68 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
'use strict';
2-
// Regression test for https://github.com/nodejs/node/issues/63701
3-
// BrotliCompress/BrotliDecompress.flush(kind) used to spin at 100% CPU when
4-
// kind was outside the brotli operation range (0..3) — e.g. Z_FINISH (4) or
5-
// Z_BLOCK (5). It must now throw ERR_OUT_OF_RANGE before reaching the native
6-
// layer.
2+
// Regression test for https://github.com/nodejs/node/issues/63701.
3+
// Invalid Brotli flush kinds used to spin in native code. flush(kind) should
4+
// reject invalid kinds before writing the fake flush chunk.
75

86
require('../common');
97
constassert=require('assert');
@@ -14,56 +12,81 @@ const {
1412
BROTLI_OPERATION_FLUSH,
1513
BROTLI_OPERATION_FINISH,
1614
BROTLI_OPERATION_EMIT_METADATA,
15+
Z_NO_FLUSH,
1716
Z_BLOCK,
1817
Z_FINISH,
18+
ZSTD_e_continue,
19+
ZSTD_e_flush,
20+
ZSTD_e_end,
1921
}=zlib.constants;
2022

21-
// Brotli operations 0..3 are valid and must not throw synchronously from
22-
// flush(). Some operations (e.g. FINISH on a decoder with no input) emit a
23-
// Z_BUF_ERROR asynchronously — that's expected and unrelated to the input
24-
// validation under test, so we attach a noop error handler.
25-
for(constvalidKindof[
26-
BROTLI_OPERATION_PROCESS,
27-
BROTLI_OPERATION_FLUSH,
28-
BROTLI_OPERATION_FINISH,
29-
BROTLI_OPERATION_EMIT_METADATA,
30-
]){
31-
constc=zlib.createBrotliCompress();
32-
c.on('error',()=>{});
33-
c.flush(validKind);
23+
constnoop=()=>{};
3424

35-
constd=zlib.createBrotliDecompress();
36-
d.on('error',()=>{});
37-
d.flush(validKind);
38-
}
25+
constflushKindTestCases=[
26+
{
27+
factories: [zlib.createGzip],
28+
validKinds: [Z_NO_FLUSH,Z_FINISH,Z_BLOCK],
29+
invalidKinds: [-1,6,100],
30+
},
31+
{
32+
factories: [zlib.createBrotliCompress,zlib.createBrotliDecompress],
33+
validKinds: [
34+
BROTLI_OPERATION_PROCESS,
35+
BROTLI_OPERATION_FLUSH,
36+
BROTLI_OPERATION_FINISH,
37+
BROTLI_OPERATION_EMIT_METADATA,
38+
],
39+
invalidKinds: [-1,Z_FINISH,Z_BLOCK,6,100],
40+
},
41+
{
42+
factories: [zlib.createZstdCompress,zlib.createZstdDecompress],
43+
validKinds: [ZSTD_e_continue,ZSTD_e_flush,ZSTD_e_end],
44+
invalidKinds: [-1,3,Z_FINISH,Z_BLOCK,100],
45+
},
46+
];
3947

40-
// Values outside [0, 3] must throw ERR_OUT_OF_RANGE for both compress and
41-
// decompress streams. Z_FINISH (4) and Z_BLOCK (5) previously hung at 100% CPU.
42-
constoutOfRange=[-1,Z_FINISH,Z_BLOCK,6,7,100];
48+
for(const{ factories, validKinds }offlushKindTestCases){
49+
for(constfactoryoffactories){
50+
for(constkindofvalidKinds){
51+
conststream=factory();
52+
stream.on('error',noop);
53+
stream.flush(kind);
54+
}
55+
}
56+
}
4357

44-
for(constfactoryof[zlib.createBrotliCompress,zlib.createBrotliDecompress]){
45-
for(constkindofoutOfRange){
46-
assert.throws(
47-
()=>factory().flush(kind),
48-
{code: 'ERR_OUT_OF_RANGE',name: 'RangeError'},
49-
);
58+
for(const{ factories, invalidKinds }offlushKindTestCases){
59+
for(constfactoryoffactories){
60+
for(constkindofinvalidKinds){
61+
assert.throws(
62+
()=>factory().flush(kind),
63+
{code: 'ERR_OUT_OF_RANGE',name: 'RangeError'},
64+
);
65+
}
5066
}
5167
}
5268

53-
// Non-number kinds must also throw, instead of silently triggering the
54-
// fake-chunk path with an undefined buffer.
55-
for(constfactoryof[zlib.createBrotliCompress,zlib.createBrotliDecompress]){
56-
for(constkindof['foobar',null,{},NaN]){
57-
assert.throws(
58-
()=>factory().flush(kind),
59-
{code: 'ERR_OUT_OF_RANGE'},
60-
);
69+
for(const{ factories }offlushKindTestCases){
70+
for(constfactoryoffactories){
71+
for(constkindof['foobar',null,{}]){
72+
assert.throws(
73+
()=>factory().flush(kind),
74+
{code: 'ERR_INVALID_ARG_TYPE',name: 'TypeError'},
75+
);
76+
}
6177
}
6278
}
6379

64-
// flush() with no arguments (or with only a callback) must still work —
65-
// it uses the stream's _defaultFullFlushFlag, which is a valid brotli op.
66-
zlib.createBrotliCompress().flush();
67-
zlib.createBrotliCompress().flush(()=>{});
68-
zlib.createBrotliDecompress().flush();
69-
zlib.createBrotliDecompress().flush(()=>{});
80+
for(const{ factories }offlushKindTestCases){
81+
for(constfactoryoffactories){
82+
for(constkindof[undefined,NaN]){
83+
conststream=factory();
84+
stream.on('error',noop);
85+
stream.flush(kind);
86+
}
87+
88+
conststream=factory();
89+
stream.on('error',noop);
90+
stream.flush(noop);
91+
}
92+
}

0 commit comments

Comments
 (0)