From b61220ed84a121fbc74811cf45633f5c9746faa8 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 14 Sep 2017 14:02:54 -0400 Subject: [PATCH] http2: fix subsequent end calls to not throw Calling Http2ServerResponse.end multiple times should never cause the code to throw an error, subsequent calls should instead return false. Fix behaviour to match http1. Fixes: https://github.com/nodejs/node/issues/15385 PR-URL: https://github.com/nodejs/node/pull/15414 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- lib/internal/http2/compat.js | 7 +++---- test/parallel/test-http2-compat-serverresponse-end.js | 5 ++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index c36b887f178c8d..15e7358627dd8d 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -466,14 +466,13 @@ class Http2ServerResponse extends Stream { cb = encoding; encoding = 'utf8'; } + if (stream === undefined || stream.finished === true) { + return false; + } if (chunk !== null && chunk !== undefined) { this.write(chunk, encoding); } - if (stream === undefined) { - return; - } - if (typeof cb === 'function') { stream.once('finish', cb); } diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 6c07f6d83e62e0..92a0fab4e0a49d 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -4,7 +4,7 @@ const { mustCall, mustNotCall, hasCrypto, skip } = require('../common'); if (!hasCrypto) skip('missing crypto'); -const { strictEqual } = require('assert'); +const { doesNotThrow, strictEqual } = require('assert'); const { createServer, connect, @@ -19,6 +19,9 @@ const { // but may be invoked repeatedly without throwing errors. const server = createServer(mustCall((request, response) => { strictEqual(response.closed, false); + response.on('finish', mustCall(() => process.nextTick( + mustCall(() => doesNotThrow(() => response.end('test', mustNotCall()))) + ))); response.end(mustCall(() => { server.close(); }));