http2: ServerHttp2Stream.headersSent property, proxied by Http2ServerResponse

This avoids brittle abstraction-leaking to guess whether or not ServerHttp2Stream.respond() may still be called on HEAD requests where the stream is ended before headers are sent.

Author: sebdeckers

lib/internal/http2/compat.js

@@ -262,7 +262,6 @@ class Http2ServerResponse extends Stream {
     this[kState] = {
       sendDate: true,
       statusCode: constants.HTTP_STATUS_OK,
-      headersSent: false,
       headerCount: 0,
       trailerCount: 0,
       closed: false,
@@ -300,8 +299,8 @@ class Http2ServerResponse extends Stream {
   }
 
   get headersSent() {
-    var state = this[kState];
-    return state.headersSent;
+    var stream = this[kStream];
+    return stream.headersSent;
   }
 
   get sendDate() {
@@ -388,7 +387,7 @@ class Http2ServerResponse extends Stream {
   }
 
   flushHeaders() {
-    if (this[kState].headersSent === false)
+    if (this[kStream].headersSent === false)
       this[kBeginSend]();
   }
 
@@ -480,19 +479,17 @@ class Http2ServerResponse extends Stream {
   }
 
   sendInfo(code, headers) {
-    var state = this[kState];
-    if (state.headersSent === true) {
+    var stream = this[kStream];
+    if (stream.headersSent === true) {
       throw new errors.Error('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND');
     }
     if (headers && typeof headers !== 'object')
       throw new errors.TypeError('ERR_HTTP2_HEADERS_OBJECT');
-    var stream = this[kStream];
     if (stream === undefined) return;
     code |= 0;
     if (code < 100 || code >= 200)
       throw new errors.RangeError('ERR_HTTP2_INVALID_INFO_STATUS', code);
 
-    state.headersSent = true;
     headers[constants.HTTP2_HEADER_STATUS] = code;
     stream.respond(headers);
   }
@@ -509,17 +506,14 @@ class Http2ServerResponse extends Stream {
   }
 
   [kBeginSend](options) {
-    var state = this[kState];
-    var stream = this[kStream];
-    if (state.headersSent === false) {
-      state.headersSent = true;
+    const stream = this[kStream];
+    if (stream !== undefined && stream.headersSent === false) {
+      const state = this[kState];
       const headers = this[kHeaders] || Object.create(null);
       headers[constants.HTTP2_HEADER_STATUS] = state.statusCode;
-      var _writableState = stream._writableState;
-      if (stream.destroyed === false &&
-        _writableState.ending === false &&
-        _writableState.finished === false
-      ) {
+      if (stream.finished === true)
+        options.endStream = true;
+      if (stream.destroyed === false) {
         stream.respond(headers, options);
       }
     }

lib/internal/http2/core.js

@@ -1534,6 +1534,11 @@ class ServerHttp2Stream extends Http2Stream {
     debug(`[${sessionName(session[kType])}] created serverhttp2stream`);
   }
 
+  // true if the HEADERS frame has been sent
+  get headersSent() {
+    return this[kState].headersSent;
+  }
+
   // true if the remote peer accepts push streams
   get pushAllowed() {
     return this[kSession].remoteSettings.enablePush;

test/parallel/test-http2-compat-serverresponse-end.js

@@ -1,12 +1,20 @@
 // Flags: --expose-http2
 'use strict';
 
+const { strictEqual } = require('assert');
 const { mustCall, mustNotCall } = require('../common');
-const { createServer, connect } = require('http2');
-
-// Http2ServerResponse.end
+const {
+  createServer,
+  connect,
+  constants: {
+    HTTP2_HEADER_STATUS,
+    HTTP_STATUS_OK
+  }
+} = require('http2');
 
 {
+  // Http2ServerResponse.end callback is called only the first time,
+  // but may be invoked repeatedly without throwing errors.
   const server = createServer(mustCall((request, response) => {
     response.end(mustCall(() => {
       server.close();
@@ -31,3 +39,39 @@ const { createServer, connect } = require('http2');
     }));
   }));
 }
+
+{
+  // Http2ServerResponse.end is not necessary on HEAD requests since the stream
+  // is already closed. Headers, however, can still be sent to the client.
+  const server = createServer(mustCall((request, response) => {
+    strictEqual(response.finished, true);
+    response.writeHead(HTTP_STATUS_OK, {foo: 'bar'});
+    response.flushHeaders();
+    response.end(mustNotCall());
+  }));
+  server.listen(0, mustCall(() => {
+    const {port} = server.address();
+    const url = `http://localhost:${port}`;
+    const client = connect(url, mustCall(() => {
+      const headers = {
+        ':path': '/',
+        ':method': 'HEAD',
+        ':scheme': 'http',
+        ':authority': `localhost:${port}`
+      };
+      const request = client.request(headers);
+      request.on('response', mustCall((headers, flags) => {
+        strictEqual(headers[HTTP2_HEADER_STATUS], HTTP_STATUS_OK);
+        strictEqual(flags, 5); // the end of stream flag is set
+        strictEqual(headers.foo, 'bar');
+      }));
+      request.on('data', mustNotCall());
+      request.on('end', mustCall(() => {
+        client.destroy();
+        server.close();
+      }));
+      request.end();
+      request.resume();
+    }));
+  }));
+}