Skip to content

Commit cc82f54

Browse files
apapirovskiMylesBorins
authored andcommitted
http2: fix closedCode NaN, increase test coverage
[kFinish](code) can be triggered from a 'finish' event (for example when calling response.end) which does not pass code. That tries to set closedCode to undefined resulting in NaN closedCode instead of NGHTTP2_NO_ERROR. Check for code !== undefined before setting. Adds tests for closed and closedCode. PR-URL: #15154 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 0b9fde4 commit cc82f54

File tree

3 files changed

+12
-2
lines changed

3 files changed

+12
-2
lines changed

lib/internal/http2/compat.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ class Http2ServerRequest extends Readable {
249249
const state = this[kState];
250250
if (state.closed)
251251
return;
252-
state.closedCode = code;
252+
if (code !== undefined)
253+
state.closedCode = code;
253254
state.closed = true;
254255
this.push(null);
255256
this[kStream] = undefined;
@@ -522,7 +523,8 @@ class Http2ServerResponse extends Stream {
522523
const state = this[kState];
523524
if (state.closed)
524525
return;
525-
state.closedCode = code;
526+
if (code !== undefined)
527+
state.closedCode = code;
526528
state.closed = true;
527529
this.end();
528530
this[kStream] = undefined;

test/parallel/test-http2-compat-serverrequest.js

+5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ server.listen(0, common.mustCall(function() {
2121
httpVersionMinor: 0
2222
};
2323

24+
assert.strictEqual(request.closed, false);
25+
assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR);
26+
2427
assert.strictEqual(request.statusCode, expected.statusCode);
2528
assert.strictEqual(request.httpVersion, expected.version);
2629
assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor);
@@ -31,6 +34,8 @@ server.listen(0, common.mustCall(function() {
3134
assert.strictEqual(request.socket, request.connection);
3235

3336
response.on('finish', common.mustCall(function() {
37+
assert.strictEqual(request.closed, true);
38+
assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR);
3439
server.close();
3540
}));
3641
response.end();

test/parallel/test-http2-compat-serverresponse-headers.js

+3
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ server.listen(0, common.mustCall(function() {
8484
response.sendDate = false;
8585
assert.strictEqual(response.sendDate, false);
8686

87+
assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR);
88+
8789
response.on('finish', common.mustCall(function() {
90+
assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR);
8891
server.close();
8992
}));
9093
response.end();

0 commit comments

Comments
 (0)