http2: refactor trailers API

Rather than using the `'fetchTrailers'` event to collect trailers,
a new `getTrailers` callback option is supported. If not set, the
internals will skip calling out for trailers at all. Expands the
test to make sure trailers work from the client side also.

PR-URL: https://github.com/nodejs/node/pull/14239
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

doc/api/http2.md

@@ -346,6 +346,9 @@ added: REPLACEME
   * `weight` {number} Specifies the relative dependency of a stream in relation
     to other streams with the same `parent`. The value is a number between `1`
     and `256` (inclusive).
+  * `getTrailers` {Function} Callback function invoked to collect trailer
+    headers.
+
 * Returns: {ClientHttp2Stream}
 
 For HTTP/2 Client `Http2Session` instances only, the `http2session.request()`
@@ -371,6 +374,16 @@ req.on('response', (headers) => {
 });
 ```
 
+When set, the `options.getTrailers()` function is called immediately after
+queuing the last chunk of payload data to be sent. The callback is passed a
+single object (with a `null` prototype) that the listener may used to specify
+the trailing header fields to send to the peer.
+
+*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
+"pseudo-header" fields (e.g. `':method'`, `':path'`, etc). An `'error'` event
+will be emitted if the `getTrailers` callback attempts to set such header
+fields.
+
 #### http2session.rstStream(stream, code)
 <!-- YAML
 added: REPLACEME
@@ -617,27 +630,6 @@ added: REPLACEME
 The `'error'` event is emitted when an error occurs during the processing of
 an `Http2Stream`.
 
-#### Event: 'fetchTrailers'
-<!-- YAML
-added: REPLACEME
--->
-
-The `'fetchTrailers'` event is emitted by the `Http2Stream` immediately after
-queuing the last chunk of payload data to be sent. The listener callback is
-passed a single object (with a `null` prototype) that the listener may used
-to specify the trailing header fields to send to the peer.
-
-```js
-stream.on('fetchTrailers', (trailers) => {
-  trailers['ABC'] = 'some value to send';
-});
-```
-
-*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
-"pseudo-header" fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
-will be emitted if the `'fetchTrailers'` event handler attempts to set such
-header fields.
-
 #### Event: 'frameError'
 <!-- YAML
 added: REPLACEME
@@ -991,6 +983,8 @@ added: REPLACEME
 * `options` {Object}
   * `endStream` {boolean} Set to `true` to indicate that the response will not
     include payload data.
+  * `getTrailers` {function} Callback function invoked to collect trailer
+    headers.
 * Returns: {undefined}
 
 ```js
@@ -1002,6 +996,29 @@ server.on('stream', (stream) => {
 });
 ```
 
+When set, the `options.getTrailers()` function is called immediately after
+queuing the last chunk of payload data to be sent. The callback is passed a
+single object (with a `null` prototype) that the listener may used to specify
+the trailing header fields to send to the peer.
+
+```js
+const http2 = require('http2');
+const server = http2.createServer();
+server.on('stream', (stream) => {
+  stream.respond({ ':status': 200 }, {
+    getTrailers(trailers) {
+      trailers['ABC'] = 'some value to send';
+    }
+  });
+  stream.end('some data');
+});
+```
+
+*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
+"pseudo-header" fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
+will be emitted if the `getTrailers` callback attempts to set such header
+fields.
+
 #### http2stream.respondWithFD(fd[, headers[, options]])
 <!-- YAML
 added: REPLACEME
@@ -1011,6 +1028,8 @@ added: REPLACEME
 * `headers` {[Headers Object][]}
 * `options` {Object}
   * `statCheck` {Function}
+  * `getTrailers` {Function} Callback function invoked to collect trailer
+    headers.
   * `offset` {number} The offset position at which to begin reading
   * `length` {number} The amount of data from the fd to send
 
@@ -1020,8 +1039,7 @@ attempting to read data using the file descriptor, the `Http2Stream` will be
 closed using an `RST_STREAM` frame using the standard `INTERNAL_ERROR` code.
 
 When used, the `Http2Stream` object's Duplex interface will be closed
-automatically. HTTP trailer fields cannot be sent. The `'fetchTrailers'` event
-will *not* be emitted.
+automatically.
 
 ```js
 const http2 = require('http2');
@@ -1052,6 +1070,39 @@ The `offset` and `length` options may be used to limit the response to a
 specific range subset. This can be used, for instance, to support HTTP Range
 requests.
 
+When set, the `options.getTrailers()` function is called immediately after
+queuing the last chunk of payload data to be sent. The callback is passed a
+single object (with a `null` prototype) that the listener may used to specify
+the trailing header fields to send to the peer.
+
+```js
+const http2 = require('http2');
+const fs = require('fs');
+
+const fd = fs.openSync('/some/file', 'r');
+
+const server = http2.createServer();
+server.on('stream', (stream) => {
+  const stat = fs.fstatSync(fd);
+  const headers = {
+    'content-length': stat.size,
+    'last-modified': stat.mtime.toUTCString(),
+    'content-type': 'text/plain'
+  };
+  stream.respondWithFD(fd, headers, {
+    getTrailers(trailers) {
+      trailers['ABC'] = 'some value to send';
+    }
+  });
+});
+server.on('close', () => fs.closeSync(fd));
+```
+
+*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
+"pseudo-header" fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
+will be emitted if the `getTrailers` callback attempts to set such header
+fields.
+
 #### http2stream.respondWithFile(path[, headers[, options]])
 <!-- YAML
 added: REPLACEME
@@ -1061,6 +1112,8 @@ added: REPLACEME
 * `headers` {[Headers Object][]}
 * `options` {Object}
   * `statCheck` {Function}
+  * `getTrailers` {Function} Callback function invoked to collect trailer
+    headers.
   * `offset` {number} The offset position at which to begin reading
   * `length` {number} The amount of data from the fd to send
 
@@ -1068,8 +1121,7 @@ Sends a regular file as the response. The `path` must specify a regular file
 or an `'error'` event will be emitted on the `Http2Stream` object.
 
 When used, the `Http2Stream` object's Duplex interface will be closed
-automatically. HTTP trailer fields cannot be sent. The `'fetchTrailers'` event
-will *not* be emitted.
+automatically.
 
 The optional `options.statCheck` function may be specified to give user code
 an opportunity to set additional content headers based on the `fs.Stat` details
@@ -1120,6 +1172,29 @@ The `offset` and `length` options may be used to limit the response to a
 specific range subset. This can be used, for instance, to support HTTP Range
 requests.
 
+When set, the `options.getTrailers()` function is called immediately after
+queuing the last chunk of payload data to be sent. The callback is passed a
+single object (with a `null` prototype) that the listener may used to specify
+the trailing header fields to send to the peer.
+
+```js
+const http2 = require('http2');
+const server = http2.createServer();
+server.on('stream', (stream) => {
+  function getTrailers(trailers) {
+    trailers['ABC'] = 'some value to send';
+  }
+  stream.respondWithFile('/some/file',
+                         { 'content-type': 'text/plain' },
+                         { getTrailers });
+});
+```
+
+*Note*: The HTTP/1 specification forbids trailers from containing HTTP/2
+"pseudo-header" fields (e.g. `':status'`, `':path'`, etc). An `'error'` event
+will be emitted if the `getTrailers` callback attempts to set such header
+fields.
+
 ### Class: Http2Server
 <!-- YAML
 added: REPLACEME

lib/internal/http2/core.js

@@ -214,7 +214,7 @@ function onSessionHeaders(id, cat, flags, headers) {
 }
 
 // Called to determine if there are trailers to be sent at the end of a
-// Stream. The 'fetchTrailers' event is emitted and passed a holder object.
+// Stream. The 'getTrailers' callback is invoked and passed a holder object.
 // The trailers to return are set on that object by the handler. Once the
 // event handler returns, those are sent off for processing. Note that this
 // is a necessarily synchronous operation. We need to know immediately if
@@ -229,9 +229,11 @@ function onSessionTrailers(id) {
   assert(stream !== undefined,
          'Internal HTTP/2 Failure. Stream does not exist. Please ' +
          'report this as a bug in Node.js');
-
+  const getTrailers = stream[kState].getTrailers;
+  if (typeof getTrailers !== 'function')
+    return [];
   const trailers = Object.create(null);
-  stream.emit('fetchTrailers', trailers);
+  getTrailers.call(stream, trailers);
   const headersList = mapToHeaders(trailers, assertValidPseudoHeaderTrailer);
   if (!Array.isArray(headersList)) {
     process.nextTick(() => stream.emit('error', headersList));
@@ -407,8 +409,7 @@ function requestOnConnect(headers, options) {
   const session = this[kSession];
   debug(`[${sessionName(session[kType])}] connected.. initializing request`);
   const streams = session[kState].streams;
-  // ret will be either the reserved stream ID (if positive)
-  // or an error code (if negative)
+
   validatePriorityOptions(options);
   const handle = session[kHandle];
 
@@ -418,11 +419,20 @@ function requestOnConnect(headers, options) {
     return;
   }
 
+  let getTrailers = false;
+  if (typeof options.getTrailers === 'function') {
+    getTrailers = true;
+    this[kState].getTrailers = options.getTrailers;
+  }
+
+  // ret will be either the reserved stream ID (if positive)
+  // or an error code (if negative)
   const ret = handle.submitRequest(headersList,
                                    !!options.endStream,
                                    options.parent | 0,
                                    options.weight | 0,
-                                   !!options.exclusive);
+                                   !!options.exclusive,
+                                   getTrailers);
 
   // In an error condition, one of three possible response codes will be
   // possible:
@@ -1095,7 +1105,15 @@ class ClientHttp2Session extends Http2Session {
                                   options.endStream);
     }
 
+    if (options.getTrailers !== undefined &&
+      typeof options.getTrailers !== 'function') {
+      throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
+                                 'getTrailers',
+                                 options.getTrailers);
+    }
+
     const stream = new ClientHttp2Stream(this, undefined, {});
+
     const onConnect = requestOnConnect.bind(stream, headers, options);
 
     // Close the writable side of the stream if options.endStream is set.
@@ -1535,7 +1553,8 @@ function processHeaders(headers) {
   return headers;
 }
 
-function processRespondWithFD(fd, headers, offset = 0, length = -1) {
+function processRespondWithFD(fd, headers, offset = 0, length = -1,
+                              getTrailers = false) {
   const session = this[kSession];
   const state = this[kState];
   state.headersSent = true;
@@ -1545,7 +1564,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1) {
 
   const handle = session[kHandle];
   const ret =
-    handle.submitFile(this[kID], fd, headers, offset, length);
+    handle.submitFile(this[kID], fd, headers, offset, length, getTrailers);
   let err;
   switch (ret) {
     case NGHTTP2_ERR_NOMEM:
@@ -1560,7 +1579,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1) {
   }
 }
 
-function doSendFD(session, options, fd, headers, err, stat) {
+function doSendFD(session, options, fd, headers, getTrailers, err, stat) {
   if (this.destroyed || session.destroyed) {
     abort(this);
     return;
@@ -1588,10 +1607,11 @@ function doSendFD(session, options, fd, headers, err, stat) {
 
   processRespondWithFD.call(this, fd, headersList,
                             statOptions.offset,
-                            statOptions.length);
+                            statOptions.length,
+                            getTrailers);
 }
 
-function doSendFileFD(session, options, fd, headers, err, stat) {
+function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) {
   if (this.destroyed || session.destroyed) {
     abort(this);
     return;
@@ -1633,10 +1653,11 @@ function doSendFileFD(session, options, fd, headers, err, stat) {
 
   processRespondWithFD.call(this, fd, headersList,
                             options.offset,
-                            options.length);
+                            options.length,
+                            getTrailers);
 }
 
-function afterOpen(session, options, headers, err, fd) {
+function afterOpen(session, options, headers, getTrailers, err, fd) {
   const state = this[kState];
   if (this.destroyed || session.destroyed) {
     abort(this);
@@ -1648,7 +1669,8 @@ function afterOpen(session, options, headers, err, fd) {
   }
   state.fd = fd;
 
-  fs.fstat(fd, doSendFileFD.bind(this, session, options, fd, headers));
+  fs.fstat(fd,
+           doSendFileFD.bind(this, session, options, fd, headers, getTrailers));
 }
 
 
@@ -1783,6 +1805,17 @@ class ServerHttp2Stream extends Http2Stream {
     options = Object.assign(Object.create(null), options);
     options.endStream = !!options.endStream;
 
+    let getTrailers = false;
+    if (options.getTrailers !== undefined) {
+      if (typeof options.getTrailers !== 'function') {
+        throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
+                                   'getTrailers',
+                                   options.getTrailers);
+      }
+      getTrailers = true;
+      state.getTrailers = options.getTrailers;
+    }
+
     headers = processHeaders(headers);
     const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
 
@@ -1809,7 +1842,10 @@ class ServerHttp2Stream extends Http2Stream {
 
     const handle = session[kHandle];
     const ret =
-      handle.submitResponse(this[kID], headersList, options.endStream);
+      handle.submitResponse(this[kID],
+                            headersList,
+                            options.endStream,
+                            getTrailers);
     let err;
     switch (ret) {
       case NGHTTP2_ERR_NOMEM:
@@ -1862,6 +1898,17 @@ class ServerHttp2Stream extends Http2Stream {
                                  options.statCheck);
     }
 
+    let getTrailers = false;
+    if (options.getTrailers !== undefined) {
+      if (typeof options.getTrailers !== 'function') {
+        throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
+                                   'getTrailers',
+                                   options.getTrailers);
+      }
+      getTrailers = true;
+      state.getTrailers = options.getTrailers;
+    }
+
     if (typeof fd !== 'number')
       throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
                                  'fd', 'number');
@@ -1876,7 +1923,8 @@ class ServerHttp2Stream extends Http2Stream {
     }
 
     if (options.statCheck !== undefined) {
-      fs.fstat(fd, doSendFD.bind(this, session, options, fd, headers));
+      fs.fstat(fd,
+               doSendFD.bind(this, session, options, fd, headers, getTrailers));
       return;
     }
 
@@ -1888,7 +1936,8 @@ class ServerHttp2Stream extends Http2Stream {
 
     processRespondWithFD.call(this, fd, headersList,
                               options.offset,
-                              options.length);
+                              options.length,
+                              getTrailers);
   }
 
   // Initiate a file response on this Http2Stream. The path is passed to
@@ -1930,6 +1979,17 @@ class ServerHttp2Stream extends Http2Stream {
                                  options.statCheck);
     }
 
+    let getTrailers = false;
+    if (options.getTrailers !== undefined) {
+      if (typeof options.getTrailers !== 'function') {
+        throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
+                                   'getTrailers',
+                                   options.getTrailers);
+      }
+      getTrailers = true;
+      state.getTrailers = options.getTrailers;
+    }
+
     headers = processHeaders(headers);
     const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
     // Payload/DATA frames are not permitted in these cases
@@ -1939,7 +1999,8 @@ class ServerHttp2Stream extends Http2Stream {
       throw new errors.Error('ERR_HTTP2_PAYLOAD_FORBIDDEN', statusCode);
     }
 
-    fs.open(path, 'r', afterOpen.bind(this, session, options, headers));
+    fs.open(path, 'r',
+            afterOpen.bind(this, session, options, headers, getTrailers));
   }
 
   // Sends a block of informational headers. In theory, the HTTP/2 spec

src/node_http2.cc

@@ -538,6 +538,7 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
   int32_t parent = args[2]->Int32Value(context).ToChecked();
   int32_t weight = args[3]->Int32Value(context).ToChecked();
   bool exclusive = args[4]->BooleanValue(context).ToChecked();
+  bool getTrailers = args[5]->BooleanValue(context).ToChecked();
 
   DEBUG_HTTP2("Http2Session: submitting request: headers: %d, end-stream: %d, "
               "parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
@@ -550,7 +551,8 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {
 
   int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
                                                        *list, list.length(),
-                                                       nullptr, endStream);
+                                                       nullptr, endStream,
+                                                       getTrailers);
   DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
   args.GetReturnValue().Set(ret);
 }
@@ -570,6 +572,7 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {
   int32_t id = args[0]->Int32Value(context).ToChecked();
   Local<Array> headers = args[1].As<Array>();
   bool endStream = args[2]->BooleanValue(context).ToChecked();
+  bool getTrailers = args[3]->BooleanValue(context).ToChecked();
 
   DEBUG_HTTP2("Http2Session: submitting response for stream %d: headers: %d, "
               "end-stream: %d\n", id, headers->Length(), endStream);
@@ -581,7 +584,7 @@ void Http2Session::SubmitResponse(const FunctionCallbackInfo<Value>& args) {
   Headers list(isolate, context, headers);
 
   args.GetReturnValue().Set(
-      stream->SubmitResponse(*list, list.length(), endStream));
+      stream->SubmitResponse(*list, list.length(), endStream, getTrailers));
 }
 
 void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
@@ -605,6 +608,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
 
   int64_t offset = args[3]->IntegerValue(context).ToChecked();
   int64_t length = args[4]->IntegerValue(context).ToChecked();
+  bool getTrailers = args[5]->BooleanValue(context).ToChecked();
 
   CHECK_GE(offset, 0);
 
@@ -618,7 +622,7 @@ void Http2Session::SubmitFile(const FunctionCallbackInfo<Value>& args) {
   Headers list(isolate, context, headers);
 
   args.GetReturnValue().Set(stream->SubmitFile(fd, *list, list.length(),
-                                               offset, length));
+                                               offset, length, getTrailers));
 }
 
 void Http2Session::SendHeaders(const FunctionCallbackInfo<Value>& args) {

src/node_http2_core-inl.h

@@ -274,10 +274,11 @@ inline void Nghttp2Session::RemoveStream(int32_t id) {
 inline Nghttp2Stream* Nghttp2Stream::Init(
     int32_t id,
     Nghttp2Session* session,
-    nghttp2_headers_category category) {
+    nghttp2_headers_category category,
+    bool getTrailers) {
   DEBUG_HTTP2("Nghttp2Stream %d: initializing stream\n", id);
   Nghttp2Stream* stream = stream_free_list.pop();
-  stream->ResetState(id, session, category);
+  stream->ResetState(id, session, category, getTrailers);
   session->AddStream(stream);
   return stream;
 }
@@ -287,7 +288,8 @@ inline Nghttp2Stream* Nghttp2Stream::Init(
 inline void Nghttp2Stream::ResetState(
     int32_t id,
     Nghttp2Session* session,
-    nghttp2_headers_category category) {
+    nghttp2_headers_category category,
+    bool getTrailers) {
   DEBUG_HTTP2("Nghttp2Stream %d: resetting stream state\n", id);
   session_ = session;
   queue_head_ = nullptr;
@@ -303,6 +305,7 @@ inline void Nghttp2Stream::ResetState(
   prev_local_window_size_ = 65535;
   queue_head_index_ = 0;
   queue_head_offset_ = 0;
+  getTrailers_ = getTrailers;
 }
 
 
@@ -414,9 +417,11 @@ inline int32_t Nghttp2Stream::SubmitPushPromise(
 // be sent.
 inline int Nghttp2Stream::SubmitResponse(nghttp2_nv* nva,
                                          size_t len,
-                                         bool emptyPayload) {
+                                         bool emptyPayload,
+                                         bool getTrailers) {
   CHECK_GT(len, 0);
   DEBUG_HTTP2("Nghttp2Stream %d: submitting response\n", id_);
+  getTrailers_ = getTrailers;
   nghttp2_data_provider* provider = nullptr;
   nghttp2_data_provider prov;
   prov.source.ptr = this;
@@ -432,10 +437,12 @@ inline int Nghttp2Stream::SubmitResponse(nghttp2_nv* nva,
 inline int Nghttp2Stream::SubmitFile(int fd,
                                      nghttp2_nv* nva, size_t len,
                                      int64_t offset,
-                                     int64_t length) {
+                                     int64_t length,
+                                     bool getTrailers) {
   CHECK_GT(len, 0);
   CHECK_GT(fd, 0);
   DEBUG_HTTP2("Nghttp2Stream %d: submitting file\n", id_);
+  getTrailers_ = getTrailers;
   nghttp2_data_provider prov;
   prov.source.ptr = this;
   prov.source.fd = fd;
@@ -456,7 +463,8 @@ inline int32_t Nghttp2Session::SubmitRequest(
     nghttp2_nv* nva,
     size_t len,
     Nghttp2Stream** assigned,
-    bool emptyPayload) {
+    bool emptyPayload,
+    bool getTrailers) {
   CHECK_GT(len, 0);
   DEBUG_HTTP2("Nghttp2Session: submitting request\n");
   nghttp2_data_provider* provider = nullptr;
@@ -470,7 +478,9 @@ inline int32_t Nghttp2Session::SubmitRequest(
                                        provider, nullptr);
   // Assign the Nghttp2Stream handle
   if (ret > 0) {
-    Nghttp2Stream* stream = Nghttp2Stream::Init(ret, this);
+    Nghttp2Stream* stream = Nghttp2Stream::Init(ret, this,
+                                                NGHTTP2_HCAT_HEADERS,
+                                                getTrailers);
     if (emptyPayload) stream->Shutdown();
     if (assigned != nullptr) *assigned = stream;
   }

src/node_http2_core.cc

@@ -163,6 +163,34 @@ ssize_t Nghttp2Session::OnSelectPadding(nghttp2_session* session,
   return padding;
 }
 
+void Nghttp2Session::GetTrailers(nghttp2_session* session,
+                                 Nghttp2Session* handle,
+                                 Nghttp2Stream* stream,
+                                 uint32_t* flags) {
+  if (stream->GetTrailers()) {
+    // Only when we are done sending the last chunk of data do we check for
+    // any trailing headers that are to be sent. This is the only opportunity
+    // we have to make this check. If there are trailers, then the
+    // NGHTTP2_DATA_FLAG_NO_END_STREAM flag must be set.
+    MaybeStackBuffer<nghttp2_nv> trailers;
+    handle->OnTrailers(stream, &trailers);
+    if (trailers.length() > 0) {
+      DEBUG_HTTP2("Nghttp2Session %d: sending trailers for stream %d, "
+                  "count: %d\n", handle->session_type_, id,
+                  trailers.length());
+      *flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
+      nghttp2_submit_trailer(session,
+                             stream->id(),
+                             *trailers,
+                             trailers.length());
+    }
+    for (size_t n = 0; n < trailers.length(); n++) {
+      free(trailers[n].name);
+      free(trailers[n].value);
+    }
+  }
+}
+
 // Called by nghttp2 to collect the data while a file response is sent.
 // The buf is the DATA frame buffer that needs to be filled with at most
 // length bytes. flags is used to control what nghttp2 does next.
@@ -213,7 +241,7 @@ ssize_t Nghttp2Session::OnStreamReadFD(nghttp2_session* session,
     DEBUG_HTTP2("Nghttp2Session %d: no more data for stream %d\n",
                 handle->session_type_, id);
     *flags |= NGHTTP2_DATA_FLAG_EOF;
-    // Sending trailers is not permitted with this provider.
+    GetTrailers(session, handle, stream, flags);
   }
 
   return numchars;
@@ -291,25 +319,7 @@ ssize_t Nghttp2Session::OnStreamRead(nghttp2_session* session,
                 handle->session_type_, id);
     *flags |= NGHTTP2_DATA_FLAG_EOF;
 
-    // Only when we are done sending the last chunk of data do we check for
-    // any trailing headers that are to be sent. This is the only opportunity
-    // we have to make this check. If there are trailers, then the
-    // NGHTTP2_DATA_FLAG_NO_END_STREAM flag must be set.
-    MaybeStackBuffer<nghttp2_nv> trailers;
-    handle->OnTrailers(stream, &trailers);
-    if (trailers.length() > 0) {
-      DEBUG_HTTP2("Nghttp2Session %d: sending trailers for stream %d, "
-                  "count: %d\n", handle->session_type_, id, trailers.length());
-      *flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
-      nghttp2_submit_trailer(session,
-                             stream->id(),
-                             *trailers,
-                             trailers.length());
-    }
-    for (size_t n = 0; n < trailers.length(); n++) {
-      free(trailers[n].name);
-      free(trailers[n].value);
-    }
+    GetTrailers(session, handle, stream, flags);
   }
   assert(offset <= length);
   return offset;

src/node_http2_core.h

@@ -117,7 +117,8 @@ class Nghttp2Session {
       nghttp2_nv* nva,
       size_t len,
       Nghttp2Stream** assigned = nullptr,
-      bool emptyPayload = true);
+      bool emptyPayload = true,
+      bool getTrailers = false);
 
   // Submits a notice to the connected peer that the session is in the
   // process of shutting down.
@@ -179,6 +180,11 @@ class Nghttp2Session {
   inline void HandleDataFrame(const nghttp2_frame* frame);
   inline void HandleGoawayFrame(const nghttp2_frame* frame);
 
+  static void GetTrailers(nghttp2_session* session,
+                          Nghttp2Session* handle,
+                          Nghttp2Stream* stream,
+                          uint32_t* flags);
+
   /* callbacks for nghttp2 */
 #ifdef NODE_DEBUG_HTTP2
   static int OnNghttpError(nghttp2_session* session,
@@ -259,7 +265,8 @@ class Nghttp2Stream {
   static inline Nghttp2Stream* Init(
       int32_t id,
       Nghttp2Session* session,
-      nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS);
+      nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
+      bool getTrailers = false);
 
   inline ~Nghttp2Stream() {
     CHECK_EQ(session_, nullptr);
@@ -278,7 +285,8 @@ class Nghttp2Stream {
   inline void ResetState(
       int32_t id,
       Nghttp2Session* session,
-      nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS);
+      nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
+      bool getTrailers = false);
 
   // Destroy this stream instance and free all held memory.
   // Note that this will free queued outbound and inbound
@@ -306,13 +314,15 @@ class Nghttp2Stream {
   // Initiate a response on this stream.
   inline int SubmitResponse(nghttp2_nv* nva,
                             size_t len,
-                            bool emptyPayload = false);
+                            bool emptyPayload = false,
+                            bool getTrailers = false);
 
   // Send data read from a file descriptor as the response on this stream.
   inline int SubmitFile(int fd,
                         nghttp2_nv* nva, size_t len,
                         int64_t offset,
-                        int64_t length);
+                        int64_t length,
+                        bool getTrailers = false);
 
   // Submit informational headers for this stream
   inline int SubmitInfo(nghttp2_nv* nva, size_t len);
@@ -354,6 +364,10 @@ class Nghttp2Stream {
     return flags_ & NGHTTP2_STREAM_READ_PAUSED;
   }
 
+  inline bool GetTrailers() const {
+    return getTrailers_;
+  }
+
   // Returns true if this stream is in the reading state, which occurs when
   // the NGHTTP2_STREAM_READ_START flag has been set and the
   // NGHTTP2_STREAM_READ_PAUSED flag is *not* set.
@@ -441,6 +455,9 @@ class Nghttp2Stream {
 
   int32_t prev_local_window_size_ = 65535;
 
+  // True if this stream will have outbound trailers
+  bool getTrailers_ = false;
+
   friend class Nghttp2Session;
 };
 

test/parallel/test-http2-misused-pseudoheaders.js

@@ -27,12 +27,10 @@ function onStream(stream, headers, flags) {
   stream.respond({
     'content-type': 'text/html',
     ':status': 200
-  });
-
-  // This will cause an error to be emitted on the stream because
-  // using a pseudo-header in a trailer is forbidden.
-  stream.on('fetchTrailers', (trailers) => {
+  }, {
+    getTrailers: common.mustCall((trailers) => {
       trailers[':status'] = 'bar';
+    })
   });
 
   stream.on('error', common.expectsError({

test/parallel/test-http2-trailers.js

@@ -15,12 +15,16 @@ const server = h2.createServer();
 server.on('stream', common.mustCall(onStream));
 
 function onStream(stream, headers, flags) {
+  stream.on('trailers', common.mustCall((headers) => {
+    assert.strictEqual(headers[trailerKey], trailerValue);
+  }));
   stream.respond({
     'content-type': 'text/html',
     ':status': 200
-  });
-  stream.on('fetchTrailers', function(trailers) {
+  }, {
+    getTrailers: common.mustCall((trailers) => {
       trailers[trailerKey] = trailerValue;
+    })
   });
   stream.end(body);
 }
@@ -29,16 +33,19 @@ server.listen(0);
 
 server.on('listening', common.mustCall(function() {
   const client = h2.connect(`http://localhost:${this.address().port}`);
-  const req = client.request({ ':path': '/' });
+  const req = client.request({ ':path': '/', ':method': 'POST' }, {
+    getTrailers: common.mustCall((trailers) => {
+      trailers[trailerKey] = trailerValue;
+    })
+  });
   req.on('data', common.mustCall());
   req.on('trailers', common.mustCall((headers) => {
     assert.strictEqual(headers[trailerKey], trailerValue);
-    req.end();
   }));
   req.on('end', common.mustCall(() => {
     server.close();
     client.destroy();
   }));
-  req.end();
+  req.end('data');
 
 }));