From 15f3af55199ca7dbdcd1fc79f407a22b1f37e641 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 7 Nov 2018 10:50:23 -0800 Subject: [PATCH] http2: cleanup endStream logic PR-URL: https://github.com/nodejs/node/pull/24063 Reviewed-By: Matteo Collina Note: Landed with one collaborator approval after PR was open for 18 days --- lib/internal/http2/core.js | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 163ede9e7a9ac2..4a2ece2333fc80 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2005,11 +2005,11 @@ class Http2Stream extends Duplex { function processHeaders(headers) { assertIsObject(headers, 'headers'); headers = Object.assign(Object.create(null), headers); - if (headers[HTTP2_HEADER_STATUS] == null) - headers[HTTP2_HEADER_STATUS] = HTTP_STATUS_OK; + const statusCode = + headers[HTTP2_HEADER_STATUS] = + headers[HTTP2_HEADER_STATUS] | 0 || HTTP_STATUS_OK; headers[HTTP2_HEADER_DATE] = utcDate(); - const statusCode = headers[HTTP2_HEADER_STATUS] |= 0; // This is intentionally stricter than the HTTP/1 implementation, which // allows values between 100 and 999 (inclusive) in order to allow for // backwards compatibility with non-spec compliant code. With HTTP/2, @@ -2342,26 +2342,22 @@ class ServerHttp2Stream extends Http2Stream { } headers = processHeaders(headers); - const statusCode = headers[HTTP2_HEADER_STATUS] |= 0; - - // Payload/DATA frames are not permitted in these cases so set - // the options.endStream option to true so that the underlying - // bits do not attempt to send any. - if (statusCode === HTTP_STATUS_NO_CONTENT || - statusCode === HTTP_STATUS_RESET_CONTENT || - statusCode === HTTP_STATUS_NOT_MODIFIED || - this.headRequest === true) { - options.endStream = true; - } - const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); this[kSentHeaders] = headers; state.flags |= STREAM_FLAGS_HEADERS_SENT; - // Close the writable side if the endStream option is set - if (options.endStream) + // Close the writable side if the endStream option is set or status + // is one of known codes with no payload, or it's a head request + const statusCode = headers[HTTP2_HEADER_STATUS] | 0; + if (!!options.endStream || + statusCode === HTTP_STATUS_NO_CONTENT || + statusCode === HTTP_STATUS_RESET_CONTENT || + statusCode === HTTP_STATUS_NOT_MODIFIED || + this.headRequest === true) { + options.endStream = true; this.end(); + } const ret = this[kHandle].respond(headersList, streamOptions); if (ret < 0) @@ -2414,7 +2410,8 @@ class ServerHttp2Stream extends Http2Stream { // Payload/DATA frames are not permitted in these cases if (statusCode === HTTP_STATUS_NO_CONTENT || statusCode === HTTP_STATUS_RESET_CONTENT || - statusCode === HTTP_STATUS_NOT_MODIFIED) { + statusCode === HTTP_STATUS_NOT_MODIFIED || + this.headRequest) { throw new ERR_HTTP2_PAYLOAD_FORBIDDEN(statusCode); } @@ -2475,7 +2472,8 @@ class ServerHttp2Stream extends Http2Stream { // Payload/DATA frames are not permitted in these cases if (statusCode === HTTP_STATUS_NO_CONTENT || statusCode === HTTP_STATUS_RESET_CONTENT || - statusCode === HTTP_STATUS_NOT_MODIFIED) { + statusCode === HTTP_STATUS_NOT_MODIFIED || + this.headRequest) { throw new ERR_HTTP2_PAYLOAD_FORBIDDEN(statusCode); }