From cc82f541e5cd5ab309f7990999df3731b278f345 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 2 Sep 2017 21:18:38 -0400 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/15154 Reviewed-By: Timothy Gu Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/internal/http2/compat.js | 6 ++++-- test/parallel/test-http2-compat-serverrequest.js | 5 +++++ test/parallel/test-http2-compat-serverresponse-headers.js | 3 +++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index e9a02c5c1bb3f1..cb4ba5f5d08731 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -249,7 +249,8 @@ class Http2ServerRequest extends Readable { const state = this[kState]; if (state.closed) return; - state.closedCode = code; + if (code !== undefined) + state.closedCode = code; state.closed = true; this.push(null); this[kStream] = undefined; @@ -522,7 +523,8 @@ class Http2ServerResponse extends Stream { const state = this[kState]; if (state.closed) return; - state.closedCode = code; + if (code !== undefined) + state.closedCode = code; state.closed = true; this.end(); this[kStream] = undefined; diff --git a/test/parallel/test-http2-compat-serverrequest.js b/test/parallel/test-http2-compat-serverrequest.js index 6c2448f4a51467..ebc91b7a9edabd 100644 --- a/test/parallel/test-http2-compat-serverrequest.js +++ b/test/parallel/test-http2-compat-serverrequest.js @@ -21,6 +21,9 @@ server.listen(0, common.mustCall(function() { httpVersionMinor: 0 }; + assert.strictEqual(request.closed, false); + assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); + assert.strictEqual(request.statusCode, expected.statusCode); assert.strictEqual(request.httpVersion, expected.version); assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor); @@ -31,6 +34,8 @@ server.listen(0, common.mustCall(function() { assert.strictEqual(request.socket, request.connection); response.on('finish', common.mustCall(function() { + assert.strictEqual(request.closed, true); + assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); server.close(); })); response.end(); diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index 290c8a515fd2db..717d8b00352546 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -84,7 +84,10 @@ server.listen(0, common.mustCall(function() { response.sendDate = false; assert.strictEqual(response.sendDate, false); + assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); + response.on('finish', common.mustCall(function() { + assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); server.close(); })); response.end();