From 94d6f693c6b4a3317fa9f3e84dee2dd8217a3ad2 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 11 Nov 2018 18:05:21 +0100 Subject: [PATCH] http2: compat head request should not start finished http2 streams for HEAD requests are never writable, thus writableState is not interesting for checking whether a response is ended or aborted. Refs: https://github.com/nodejs/node/issues/24283 PR-URL: https://github.com/nodejs/node/pull/24339 --- lib/internal/http2/compat.js | 7 ++-- .../test-http2-compat-head-finished.js | 40 +++++++++++++++++++ .../test-http2-compat-serverresponse-end.js | 2 +- 3 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http2-compat-head-finished.js diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 4622d97614a1bb..1129f45aa13e5a 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -436,9 +436,10 @@ class Http2ServerResponse extends Stream { get finished() { const stream = this[kStream]; + const state = this[kState]; return stream.destroyed || - stream._writableState.ended || - this[kState].closed; + state.closed || + (stream.headRequest ? state.ending : stream._writableState.ended); } get socket() { @@ -630,7 +631,7 @@ class Http2ServerResponse extends Stream { if (chunk !== null && chunk !== undefined) this.write(chunk, encoding); - const isFinished = this.finished; + const isFinished = stream._writableState.ended; state.headRequest = stream.headRequest; state.ending = true; diff --git a/test/parallel/test-http2-compat-head-finished.js b/test/parallel/test-http2-compat-head-finished.js new file mode 100644 index 00000000000000..414d341c46b4d4 --- /dev/null +++ b/test/parallel/test-http2-compat-head-finished.js @@ -0,0 +1,40 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { strictEqual } = require('assert'); +const h2 = require('http2'); + +// Http2ServerResponse.finished cannot be true before res.end() is +// called or the request is aborted. +const server = h2 + .createServer() + .listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall((req, res) => { + strictEqual(res.finished, false); + res.writeHead(400); + strictEqual(res.finished, false); + strictEqual(res.headersSent, true); + res.end(); + strictEqual(res.finished, true); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'HEAD', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(function() { + client.close(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 0e846a5948e3cc..2b038c0800eced 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -121,7 +121,7 @@ const { // Http2ServerResponse.end is necessary on HEAD requests in compat // for http1 compatibility const server = createServer(mustCall((request, response) => { - strictEqual(response.finished, true); + strictEqual(response.finished, false); response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); response.end('data', mustCall()); }));