http2: cleanup endStream logic

PR-URL: nodejs#24063 Reviewed-By: Matteo Collina <[email protected]> Note: Landed with one collaborator approval after PR was open for 18 days
leak4mk0 · Nov 21, 2018 · 6026582 · 6026582
1 parent ee80aaa
commit 6026582
Showing 1 changed file with 17 additions and 19 deletions.
diff --git 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);
     }