diff --git a/doc/api/http2.md b/doc/api/http2.md index 650136ab32..adc0b0c118 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -130,9 +130,9 @@ session.on('stream', (stream, headers, flags) => { }); ``` -*Note*: User code will typically not listen for this event directly, and would -instead register a handler for the `'stream'` event emitted by the `net.Server` -or `tls.Server` instances returned by `http2.createServer()` and +On the server side, user code will typically not listen for this event directly, +and would instead register a handler for the `'stream'` event emitted by the +`net.Server` or `tls.Server` instances returned by `http2.createServer()` and `http2.createSecureServer()`, respectively, as in the example below: ```js @@ -164,7 +164,14 @@ not handled, the `'error'` event will be re-emitted on the `Http2Session`. #### Event: 'timeout' -(TODO: fill in detail) +After the `http2session.setTimeout()` method is used to set the timeout period +for this `Http2Session`, the `'timeout'` event is emitted if there is no +activity on the `Http2Session` after the configured number of milliseconds. + +```js +session.setTimeout(2000); +session.on('timeout', () => { /** .. **/ }); +``` #### http2session.destroy() @@ -182,15 +189,15 @@ longer be used, otherwise `false`. * Value: {[Settings Object][]} -An object describing the current local settings of this `Http2Session`. The -local settings are local to *this* `Http2Session` instance. +A prototype-less object describing the current local settings of this +`Http2Session`. The local settings are local to *this* `Http2Session` instance. #### http2session.remoteSettings * Value: {[Settings Object][]} -An object describing the current remote settings of this `Http2Session`. The -remote settings are set by the *connected* HTTP/2 peer. +A prototype-less object describing the current remote settings of this +`Http2Session`. The remote settings are set by the *connected* HTTP/2 peer. #### http2session.request(headers[, options]) @@ -216,41 +223,77 @@ HTTP/2 request to the connected server. This method is only available if `http2session.type` is equal to `http2.constants.NGHTTP2_SESSION_CLIENT`. -(TODO: fill in detail) +```js +const http2 = require('http2'); +const clientSession = http2.connect('https://localhost:1234'); +const { + HTTP2_HEADER_PATH, + HTTP2_HEADER_STATUS +} = http2.constants; + +const req = clientSession.request({ [HTTP2_HEADER_PATH]: '/' }); +req.on('response', (headers) => { + console.log(HTTP2_HEADER_STATUS); + req.on('data', (chunk) => { /** .. **/ }); + req.on('end', () => { /** .. **/ }); +}); +``` #### http2session.rstStream(stream, code) * stream {Http2Stream} -* code {number} +* code {number} Unsigned 32-bit integer identifying the error code. Defaults to + `http2.constant.NGHTTP2_NO_ERROR` (`0x00`) Sends an `RST_STREAM` frame to the connected HTTP/2 peer, causing the given -`Http2Stream` to be closed on both sides using error code `code`. +`Http2Stream` to be closed on both sides using [error code][] `code`. #### http2session.setTimeout(msecs, callback) * `msecs` {number} * `callback` {Function} -(TODO: fill in detail) +Used to set a callback function that is called when there is no activity on +the `Http2Session` after `msecs` milliseconds. The given `callback` is +registered as a listener on the `'timeout'` event. #### http2session.shutdown(options[, callback]) * `options` {Object} * `graceful` {boolean} `true` to attempt a polite shutdown of the `Http2Session`. - * `errorCode` {number} The HTTP/2 Error Code to return. Note that this is - *not* the same thing as an HTTP Response Status Code. + * `errorCode` {number} The HTTP/2 [error code][] to return. Note that this is + *not* the same thing as an HTTP Response Status Code. Defaults to `0x00` + (No Error). * `lastStreamID` {number} The Stream ID of the last successfully processed `Http2Stream` on this `Http2Session`. * `opaqueData` {Buffer} A `Buffer` instance containing arbitrary additional data to send to the peer upon disconnection. This is used, typically, to provide additional data for debugging failures, if necessary. -* `callback` {Function} +* `callback` {Function} A callback that is invoked after the session shutdown + has been completed. Attempts to shutdown this `Http2Session` using HTTP/2 defined procedures. If specified, the given `callback` function will be invoked once the shutdown process has completed. +Note that calling `http2session.shutdown()` does *not* destroy the session or +tear down the `Socket` connection. It merely prompts both sessions to begin +preparing to cease activity. + +During a "graceful" shutdown, the session will first send a `GOAWAY` frame to +the connected peer identifying the last processed stream as 232-1. +Then, on the next tick of the event loop, a second `GOAWAY` frame identifying +the most recently processed stream identifier is sent. This process allows the +remote peer to begin preparing for the connection to be terminated. + +```js +session.shutdown({ + graceful: true, + opaqueData: Buffer.from('add some debugging data here') +}, () => session.destroy()); +``` + #### http2session.socket * Value: {net.Socket|tls.TLSSocket} @@ -417,7 +460,9 @@ this event is emitted, the `Http2Stream` instance is no longer usable. #### Event: 'timeout' -(TODO: fill in detail) +The `'timeout'` event is emitted after no activity is received for this +`'Http2Stream'` within the number of millseconds set using +`http2stream.setTimeout()`. #### Event: 'trailers' @@ -427,7 +472,7 @@ trailing header fields is received. The listener callback is passed the ```js stream.on('trailers', (headers, flags) => { - // TODO(jasnell): Fill in example + console.log(headers); }); ``` @@ -459,36 +504,38 @@ peer. * Value: {number} -Set to the `RST_STREAM` error code when the `Http2Stream` is destroyed after -either receiving an `RST_STREAM` frame from the connected peer, calling -`http2stream.rstStream()`, or `http2stream.destroy()`. +Set to the `RST_STREAM` [error code][] reported when the `Http2Stream` is +destroyed after either receiving an `RST_STREAM` frame from the connected peer, +calling `http2stream.rstStream()`, or `http2stream.destroy()`. Will be +`undefined` if the `Http2Stream` has not been closed. #### http2stream.rstStream(code) -* `code` {number} +* code {number} Unsigned 32-bit integer identifying the error code. Defaults to + `http2.constant.NGHTTP2_NO_ERROR` (`0x00`) Sends an `RST_STREAM` frame to the connected HTTP/2 peer, causing this -`Http2Stream` to be closed on both sides using error code `code`. +`Http2Stream` to be closed on both sides using [error code][] `code`. #### http2stream.rstWithNoError() -Shortcut for `http2stream.rstStream()` using error code `NO_ERROR`. +Shortcut for `http2stream.rstStream()` using error code `0x00` (No Error). #### http2stream.rstWithProtocolError() { -Shortcut for `http2stream.rstStream()` using error code `PROTOCOL_ERROR`. +Shortcut for `http2stream.rstStream()` using error code `0x01` (Protocol Error). #### http2stream.rstWithCancel() { -Shortcut for `http2stream.rstStream()` using error code `CANCEL`. +Shortcut for `http2stream.rstStream()` using error code `0x08` (Cancel). #### http2stream.rstWithRefuse() { -Shortcut for `http2stream.rstStream()` using error code `REFUSED_STREAM`. +Shortcut for `http2stream.rstStream()` using error code `0x07` (Refused Stream). #### http2stream.rstWithInternalError() { -Shortcut for `http2stream.rstStream()` using error code `INTERNAL_ERROR`. +Shortcut for `http2stream.rstStream()` using error code `0x02` (Internal Error). #### http2stream.session @@ -502,7 +549,15 @@ value will be `undefined` after the `Http2Stream` instance is destroyed. * `msecs` {number} * `callback` {Function} -(TODO: fill in detail) +```js +const http2 = require('http2'); +const client = http2.connect('http://example.org:8000'); + +const req = client.request({':path': '/'}); + +// Cancel the stream if there's no activity after 5 seconds +req.setTimeout(5000, () => req.rstStreamWithCancel()); +``` #### http2stream.state @@ -534,7 +589,7 @@ the headers. ```js stream.on('headers', (headers, flags) => { - // TODO(jasnell): Fill in example + console.log(headers); }); ``` @@ -546,7 +601,7 @@ associated with the headers. ```js stream.on('push', (headers, flags) => { - // TODO(jasnell): Fill in example + console.log(headers); }); ``` @@ -577,18 +632,6 @@ used exclusively on HTTP/2 Servers. `Http2Stream` instances on the server provide additional methods such as `http2stream.pushStream()` and `http2stream.respond()` that are only relevant on the server. -#### Event: 'request' - -The `'request'` event is emitted when a block of headers associated with an -HTTP request is received. The listener callback is passed the [Headers Object][] -and flags associated with the headers. - -```js -stream.on('request', (headers, flags) => { - // TODO(jasnell): Fill in example -}); -``` - #### http2stream.additionalHeaders(headers) * `headers` {[Headers Object][]} @@ -608,18 +651,38 @@ Sends an additional informational `HEADERS` frame to the connected HTTP/2 peer. * `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). -* `callback` {Function} +* `callback` {Function} Callback that is called once the push stream has been + initiated. + +Initiates a push stream. The callback is invoked with the new `Htt2Stream` +instance created for the push stream. -Initiates a push stream. -(TODO: fill in detail) +```js +const http2 = require('http2'); +const server = http2.createServer(); +server.on('stream', (stream) => { + stream.respond({':status': 200}); + stream.pushStream({':path': '/'}, (pushStream) => { + pushStream.respond({':status': 200}); + pushStream.end('some pushed data'); + }); + stream.end('some data'); +}); +``` #### http2stream.respond([headers[, options]]) * `headers` {[Headers Object][]} * `options` {Object} -Initiates a response. -(TODO: fill in detail) +```js +const http2 = require('http2'); +const server = http2.createServer(); +server.on('stream', (stream) => { + stream.respond({':status': 200}); + stream.end('some data'); +}); +``` ### Class: Http2Server @@ -667,7 +730,8 @@ server.on('stream', (stream, headers, flags) => { #### Event: 'timeout' -(TODO: fill in detail) +The `'timeout'` event is emitted when there is no activity on the Server for +a given number of milliseconds set using `http2server.setTimeout()`. ### Class: Http2SecureServer @@ -717,7 +781,8 @@ server.on('stream', (stream, headers, flags) => { #### Event: 'timeout' -(TODO: fill in detail) +The `'timeout'` event is emitted when there is no activity on the Server for +a given number of milliseconds set using `http2server.setTimeout()`. ### http2.getDefaultSettings() @@ -882,11 +947,37 @@ server.listen(80); * Returns `Http2Session` Returns a HTTP/2 client `Http2Session` instance. -(TODO: fill in detail) + +```js +const http2 = require('http2'); +const client = http2.connect('https://localhost:1234'); + +/** use the client **/ + +client.destroy(); +``` ### http2.constants -(TODO: Fill in details) +#### Error Codes for RST_STREAM and GOAWAY + + +| Value | Name | Constant | +|-------|---------------------|-----------------------------------------------| +| 0x00 | No Error | `http2.constants.NGHTTP2_NO_ERROR` | +| 0x01 | Protocol Error | `http2.constants.NGHTTP2_PROTOCOL_ERROR` | +| 0x02 | Internal Error | `http2.constants.NGHTTP2_INTERNAL_ERROR` | +| 0x03 | Flow Control Error | `http2.constants.NGHTTP2_FLOW_CONTROL_ERROR` | +| 0x04 | Settings Timeout | `http2.constants.NGHTTP2_SETTINGS_TIMEOUT` | +| 0x05 | Stream Closed | `http2.constants.NGHTTP2_STREAM_CLOSED` | +| 0x06 | Frame Size Error | `http2.constants.NGHTTP2_FRAME_SIZE_ERROR` | +| 0x07 | Refused Stream | `http2.constants.NGHTTP2_REFUSED_STREAM` | +| 0x08 | Cancel | `http2.constants.NGHTTP2_CANCEL` | +| 0x09 | Compression Error | `http2.constants.NGHTTP2_COMPRESSION_ERROR` | +| 0x0a | Connect Error | `http2.constants.NGHTTP2_CONNECT_ERROR` | +| 0x0b | Enhance Your Calm | `http2.constants.NGHTTP2_ENHANCE_YOUR_CALM` | +| 0x0c | Inadequate Security | `http2.constants.NGHTTP2_INADEQUATE_SECURITY` | +| 0x0d | HTTP/1.1 Required | `http2.constants.NGHTTP2_HTTP_1_1_REQUIRED` | ### Headers Object @@ -912,7 +1003,14 @@ prototype. This means that normal JavaScript object methods such as `Object.prototype.toString()` and `Object.prototype.hasOwnProperty()` will not work. -(TODO: Fill in more detail) +```js +const http2 = require('http2'); +const server = http2.createServer(); +server.on('stream', (stream, headers) => { + console.log(headers[':path']); + console.log(headers.ABC); +}); +``` ### Settings Object @@ -1012,3 +1110,4 @@ TBD [ServerHttp2Stream]: #http2_class_serverhttp2stream [Settings Object]: #http2_settings_object [Using options.selectPadding]: #http2_using_options_selectpadding +[error code]: #error_codes diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 82ca3c8ac6..9771bcc325 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -25,7 +25,8 @@ const { getSettings, getStreamState, mapToHeaders, - NghttpError + NghttpError, + toHeaderObject } = require('internal/http2/util'); const { @@ -57,7 +58,6 @@ const { NGHTTP2_CANCEL, NGHTTP2_DEFAULT_WEIGHT, NGHTTP2_FLAG_END_STREAM, - NGHTTP2_HCAT_REQUEST, NGHTTP2_HCAT_HEADERS, NGHTTP2_HCAT_PUSH_RESPONSE, NGHTTP2_HCAT_RESPONSE, @@ -78,7 +78,6 @@ const { HTTP2_HEADER_PATH, HTTP2_HEADER_SCHEME, HTTP2_HEADER_STATUS, - HTTP2_HEADER_COOKIE, HTTP_STATUS_CONTENT_RESET, HTTP_STATUS_OK, @@ -101,18 +100,8 @@ function onSessionHeaders(id, cat, flags, headers) { const endOfStream = !!(flags & NGHTTP2_FLAG_END_STREAM); let stream = streams.get(id); - // https://tools.ietf.org/html/rfc7540#section-8.1.2.5 - // "...If there are multiple Cookie header fields after decompression, - // these MUST be concatenated into a single octet string using the - // two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ") before being - // passed into a non-HTTP/2 context." - if (Array.isArray(headers[HTTP2_HEADER_COOKIE])) - headers[HTTP2_HEADER_COOKIE] = - headers[HTTP2_HEADER_COOKIE].join('; '); - - if (headers[HTTP2_HEADER_STATUS]) { - headers[HTTP2_HEADER_STATUS] |= 0; - } + // Convert the array of header name value pairs into an object + const obj = toHeaderObject(headers); if (stream === undefined) { switch (owner.type) { @@ -128,16 +117,13 @@ function onSessionHeaders(id, cat, flags, headers) { 'report this as a bug in Node.js'); } streams.set(id, stream); - owner.emit('stream', stream, headers, flags); + owner.emit('stream', stream, obj, flags); } else { let event; let status; switch (cat) { - case NGHTTP2_HCAT_REQUEST: - event = 'request'; - break; case NGHTTP2_HCAT_RESPONSE: - status = headers[HTTP2_HEADER_STATUS]; + status = obj[HTTP2_HEADER_STATUS]; if (!endOfStream && status !== undefined && status >= 100 && @@ -151,7 +137,7 @@ function onSessionHeaders(id, cat, flags, headers) { event = 'push'; break; case NGHTTP2_HCAT_HEADERS: - status = headers[HTTP2_HEADER_STATUS]; + status = obj[HTTP2_HEADER_STATUS]; if (!endOfStream && status !== undefined && status >= 200) { event = 'response'; } else { @@ -164,7 +150,7 @@ function onSessionHeaders(id, cat, flags, headers) { 'report this as a bug in Node.js'); } debug(`emitting stream '${event}' event`); - stream.emit(event, headers, flags); + stream.emit(event, obj, flags); } } @@ -862,6 +848,10 @@ class Http2Session extends EventEmitter { debug('sending shutdown'); submitShutdown.call(this, options); } + + _onTimeout() { + this.emit('timeout'); + } } class ClientHttp2Session extends Http2Session { @@ -1094,6 +1084,10 @@ class Http2Stream extends Duplex { return `Http2Stream ${util.format(obj)}`; } + _onTimeout() { + this.emit('timeout'); + } + get rstCode() { return this._state.rst ? this._state.rstCode : undefined; } @@ -1265,6 +1259,7 @@ class Http2Stream extends Duplex { // Remove the stream from the session const session = this.session; + setImmediate(() => { if (session._handle !== undefined) session._handle.destroyStream(this.id); @@ -1529,6 +1524,7 @@ function socketDestroy(error) { // send data while we close the socket. this.session.destroy(); this.destroy = this[kDestroySocket]; + debug('destroying the socket'); this.destroy(error); } diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 52b1a43cf5..ceab1abee4 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -18,6 +18,7 @@ const { HTTP2_HEADER_CONTENT_MD5, HTTP2_HEADER_CONTENT_RANGE, HTTP2_HEADER_CONTENT_TYPE, + HTTP2_HEADER_COOKIE, HTTP2_HEADER_DATE, HTTP2_HEADER_ETAG, HTTP2_HEADER_EXPIRES, @@ -326,6 +327,35 @@ function assertWithinRange(name, value, min = 0, max = Infinity) { } } +function toHeaderObject(headers) { + const obj = Object.create(null); + for (var n = 0; n < headers.length; n = n + 2) { + var name = headers[n]; + var value = headers[n + 1]; + if (name === HTTP2_HEADER_STATUS) + value |= 0; + var existing = obj[name]; + if (existing === undefined) { + obj[name] = value; + } else if (!kSingleValueHeaders.has(name)) { + if (name === HTTP2_HEADER_COOKIE) { + // https://tools.ietf.org/html/rfc7540#section-8.1.2.5 + // "...If there are multiple Cookie header fields after decompression, + // these MUST be concatenated into a single octet string using the + // two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ") before + // being passed into a non-HTTP/2 context." + obj[name] = `${existing}; ${value}`; + } else { + if (Array.isArray(existing)) + existing.push(value); + else + obj[name] = [existing, value]; + } + } + } + return obj; +} + module.exports = { assertIsObject, assertValidPseudoHeaderResponse, @@ -336,5 +366,6 @@ module.exports = { getSettings, getStreamState, mapToHeaders, - NghttpError + NghttpError, + toHeaderObject }; diff --git a/src/node_http2.cc b/src/node_http2.cc index af04410c42..9bb0ea7776 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -7,6 +7,7 @@ namespace node { using v8::ArrayBuffer; using v8::Boolean; using v8::Context; +using v8::Function; using v8::Integer; namespace http2 { @@ -759,150 +760,6 @@ void Http2Session::OnTrailers(Nghttp2Stream* stream, } } -static inline bool CheckHeaderAllowsMultiple(nghttp2_vec* name) { - switch (name->len) { - case 3: - if (memcmp(name->base, "age", 3) == 0) - return false; - break; - case 4: - switch (name->base[3]) { - case 'e': - if (memcmp(name->base, "dat", 3) == 0) - return false; - break; - case 'g': - if (memcmp(name->base, "eta", 3) == 0) - return false; - break; - case 'm': - if (memcmp(name->base, "fro", 3) == 0) - return false; - break; - case 't': - if (memcmp(name->base, "hos", 3) == 0) - return false; - break; - } - case 5: - if (memcmp(name->base, "range", 5) == 0) - return false; - break; - case 6: - if (memcmp(name->base, "server", 6) == 0) - return false; - break; - case 7: - switch (name->base[6]) { - case 's': - if (memcmp(name->base, "expire", 6) == 0) - return false; - break; - case 'r': - if (memcmp(name->base, "refere", 6) == 0) - return false; - break; - } - break; - case 8: - switch (name->base[7]) { - case 'e': - if (memcmp(name->base, "if-rang", 7) == 0) - return false; - break; - case 'h': - if (memcmp(name->base, "if-matc", 7) == 0) - return false; - break; - case 'n': - if (memcmp(name->base, "locatio", 7) == 0) - return false; - break; - } - break; - case 10: - if (memcmp(name->base, "user-agent", 10) == 0) - return false; - break; - case 11: - switch (name->base[10]) { - case '5': - if (memcmp(name->base, "content-md", 10) == 0) - return false; - break; - case 'r': - if (memcmp(name->base, "retry-afte", 10) == 0) - return false; - break; - } - break; - case 12: - switch (name->base[11]) { - case 'e': - if (memcmp(name->base, "content-typ", 11) == 0) - return false; - break; - case 's': - if (memcmp(name->base, "max-forward", 11) == 0) - return false; - break; - } - break; - case 13: - switch (name->base[12]) { - case 'd': - if (memcmp(name->base, "last-modifie", 12) == 0) - return false; - break; - case 'h': - if (memcmp(name->base, "if-none-matc", 12) == 0) - return false; - break; - case 'n': - if (memcmp(name->base, "authorizatio", 12) == 0) - return false; - break; - } - break; - case 14: - if (memcmp(name->base, "content-length", 14) == 0) - return false; - break; - case 16: - switch (name->base[15]) { - case 'e': - if (memcmp(name->base, "content-languag", 15) == 0) - return false; - break; - case 'g': - if (memcmp(name->base, "content-encodin", 15) == 0) - return false; - break; - case 'n': - if (memcmp(name->base, "content-locatio", 15) == 0) - return false; - break; - } - break; - case 17: - if (memcmp(name->base, "if-modified-since", 17) == 0) - return false; - break; - case 19: - switch (name->base[18]) { - case 'e': - if (memcmp(name->base, "if-unmodified-sinc", 18) == 0) - return false; - break; - case 'n': - if (memcmp(name->base, "proxy-authorizatio", 18) == 0) - return false; - break; - } - } - return true; -} - void Http2Session::OnHeaders(Nghttp2Stream* stream, nghttp2_header_list* headers, nghttp2_headers_category cat, @@ -912,38 +769,42 @@ void Http2Session::OnHeaders(Nghttp2Stream* stream, Isolate* isolate = env()->isolate(); HandleScope scope(isolate); - Local holder = Object::New(isolate); - holder->SetPrototype(context, v8::Null(isolate)).ToChecked(); Local name_str; Local value_str; - Local array; - while (headers != nullptr) { - nghttp2_header_list* item = headers; - name_str = ExternalHeaderNameResource::New(isolate, item->name); - nghttp2_vec name = nghttp2_rcbuf_get_buf(item->name); - nghttp2_vec value = nghttp2_rcbuf_get_buf(item->value); - value_str = String::NewFromUtf8(isolate, - reinterpret_cast(value.base), - v8::NewStringType::kNormal, - value.len).ToLocalChecked(); - if (holder->Has(context, name_str).FromJust()) { - if (CheckHeaderAllowsMultiple(&name)) { - Local existing = holder->Get(context, name_str).ToLocalChecked(); - if (existing->IsArray()) { - array = existing.As(); - array->Set(context, array->Length(), value_str).FromJust(); - } else { - array = Array::New(isolate, 2); - array->Set(context, 0, existing).FromJust(); - array->Set(context, 1, value_str).FromJust(); - holder->Set(context, name_str, array).FromJust(); - } - } // Ignore singleton headers that appear more than once - } else { - holder->Set(context, name_str, value_str).FromJust(); + + Local holder = Array::New(isolate); + Local fn = env()->push_values_to_array_function(); + Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX * 2]; + + // The headers are passed in above as a linked list of nghttp2_header_list + // structs. The following converts that into a JS array with the structure: + // [name1, value1, name2, value2, name3, value3, name3, value4] and so on. + // That array is passed up to the JS layer and converted into an Object form + // like {name1: value1, name2: value2, name3: [value3, value4]}. We do it + // this way for performance reasons (it's faster to generate and pass an + // array than it is to generate and pass the object). + do { + size_t j = 0; + while (headers != nullptr && j < arraysize(argv) / 2) { + nghttp2_header_list* item = headers; + name_str = ExternalHeaderNameResource::New(isolate, item->name); + nghttp2_vec value = nghttp2_rcbuf_get_buf(item->value); + value_str = String::NewFromUtf8(isolate, + reinterpret_cast(value.base), + v8::NewStringType::kNormal, + value.len).ToLocalChecked(); + argv[j * 2] = name_str; + argv[j * 2 + 1] = value_str; + headers = item->next; + j++; } - headers = item->next; - } + // For performance, we pass name and value pairs to array.protototype.push + // in batches of size NODE_PUSH_VAL_TO_ARRAY_MAX * 2 until there are no + // more items to push. + if (j > 0) { + fn->Call(env()->context(), holder, j * 2, argv).ToLocalChecked(); + } + } while (headers != nullptr); if (object()->Has(context, env()->onheaders_string()).FromJust()) { Local argv[4] = { @@ -1061,7 +922,12 @@ void Http2Session::OnStreamReadImpl(ssize_t nread, return; } uv_buf_t buf[] { uv_buf_init((*bufs).base, nread) }; - session->Write(buf, 1); + ssize_t ret = session->Write(buf, 1); + if (ret < 0) { + DEBUG_HTTP2("Http2Session: fatal error receiving data: %d\n", ret); + nghttp2_session_terminate_session(session->session(), + NGHTTP2_PROTOCOL_ERROR); + } } diff --git a/src/node_http2_core-inl.h b/src/node_http2_core-inl.h index 80bf5b4605..75274485c4 100644 --- a/src/node_http2_core-inl.h +++ b/src/node_http2_core-inl.h @@ -40,6 +40,8 @@ inline void Nghttp2Session::SubmitShutdownNotice() { } // Sends a SETTINGS frame on the current session +// Note that this *should* send a SETTINGS frame even if niv == 0 and there +// are no settings entries to send. inline int Nghttp2Session::SubmitSettings(const nghttp2_settings_entry iv[], size_t niv) { DEBUG_HTTP2("Nghttp2Session %d: submitting settings, count: %d\n", @@ -59,6 +61,18 @@ inline Nghttp2Stream* Nghttp2Session::FindStream(int32_t id) { } } +inline void Nghttp2Stream::FlushDataChunks() { + while (data_chunks_head_ != nullptr) { + DEBUG_HTTP2("Nghttp2Stream %d: emitting data chunk\n", id_); + nghttp2_data_chunk_t* item = data_chunks_head_; + data_chunks_head_ = item->next; + session_->OnDataChunk(this, item); + delete[] item->buf.base; + data_chunk_free_list.push(item); + } + data_chunks_tail_ = nullptr; +} + // Passes all of the the chunks for a data frame out to the JS layer // The chunks are collected as the frame is being processed and sent out // to the JS side only when the frame is fully processed. @@ -69,17 +83,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) { Nghttp2Stream* stream = this->FindStream(id); // If the stream does not exist, something really bad happened CHECK_NE(stream, nullptr); - - while (stream->data_chunks_head_ != nullptr) { - DEBUG_HTTP2("Nghttp2Session %d: emitting data chunk for stream %d\n", - session_type_, id); - nghttp2_data_chunk_t* item = stream->data_chunks_head_; - stream->data_chunks_head_ = item->next; - OnDataChunk(stream, item); - delete[] item->buf.base; - data_chunk_free_list.push(item); - } - stream->data_chunks_tail_ = nullptr; + stream->FlushDataChunks(); } // Passes all of the collected headers for a HEADERS frame out to the JS layer. @@ -292,7 +296,7 @@ inline void Nghttp2Stream::ResetState( inline void Nghttp2Stream::Destroy() { - DEBUG_HTTP2("Nghttp2Stream %d: destroying stream\n", id_); + DEBUG_HTTP2("Nghttp2Stream %d: destroying stream\n", id_); // Do nothing if this stream instance is already destroyed if (IsDestroyed() || IsDestroying()) return; @@ -305,7 +309,7 @@ inline void Nghttp2Stream::Destroy() { session_ = nullptr; } - // Free any remaining data chunks. + // Free any remaining incoming data chunks. while (data_chunks_head_ != nullptr) { nghttp2_data_chunk_t* chunk = data_chunks_head_; data_chunks_head_ = chunk->next; @@ -314,7 +318,16 @@ inline void Nghttp2Stream::Destroy() { } data_chunks_tail_ = nullptr; - // Free any remainingin headers + // Free any remaining outgoing data chunks. + while (queue_head_ != nullptr) { + nghttp2_stream_write_queue* head = queue_head_; + queue_head_ = head->next; + head->cb(head->req, UV_ECANCELED); + delete head; + } + queue_tail_ = nullptr; + + // Free any remaining headers FreeHeaders(); // Return this stream instance to the freelist @@ -337,6 +350,7 @@ inline void Nghttp2Stream::FreeHeaders() { inline int Nghttp2Stream::SubmitInfo(nghttp2_nv* nva, size_t len) { DEBUG_HTTP2("Nghttp2Stream %d: sending informational headers, count: %d\n", id_, len); + CHECK_GT(len, 0); return nghttp2_submit_headers(session_->session(), NGHTTP2_FLAG_NONE, id_, nullptr, @@ -364,12 +378,13 @@ inline int Nghttp2Stream::SubmitRstStream(const uint32_t code) { code); } -// Submit a push promise +// Submit a push promise. inline int32_t Nghttp2Stream::SubmitPushPromise( nghttp2_nv* nva, size_t len, Nghttp2Stream** assigned, bool emptyPayload) { + CHECK_GT(len, 0); DEBUG_HTTP2("Nghttp2Stream %d: sending push promise\n", id_); int32_t ret = nghttp2_submit_push_promise(session_->session(), NGHTTP2_FLAG_NONE, @@ -390,6 +405,7 @@ inline int32_t Nghttp2Stream::SubmitPushPromise( inline int Nghttp2Stream::SubmitResponse(nghttp2_nv* nva, size_t len, bool emptyPayload) { + CHECK_GT(len, 0); DEBUG_HTTP2("Nghttp2Stream %d: submitting response\n", id_); nghttp2_data_provider* provider = nullptr; nghttp2_data_provider prov; @@ -412,6 +428,7 @@ inline int32_t Nghttp2Session::SubmitRequest( size_t len, Nghttp2Stream** assigned, bool emptyPayload) { + CHECK_GT(len, 0); DEBUG_HTTP2("Nghttp2Session: submitting request\n"); nghttp2_data_provider* provider = nullptr; nghttp2_data_provider prov; @@ -468,6 +485,9 @@ inline int Nghttp2Stream::Write(nghttp2_stream_write_t* req, } inline void Nghttp2Stream::ReadStart() { + // Has no effect if IsReading() is true. + if (IsReading()) + return; DEBUG_HTTP2("Nghttp2Stream %d: start reading\n", id_); if (IsPaused()) { // If handle->reading is less than zero, read_start had never previously @@ -482,7 +502,9 @@ inline void Nghttp2Stream::ReadStart() { } flags_ |= NGHTTP2_STREAM_READ_START; flags_ &= ~NGHTTP2_STREAM_READ_PAUSED; - // TODO(jasnell): Drain the queued data chunks... + + // Flush any queued data chunks immediately out to the JS layer + FlushDataChunks(); } inline void Nghttp2Stream::ReadStop() { @@ -524,6 +546,14 @@ Nghttp2Session::Callbacks::Callbacks(bool kHasGetPaddingCallback) { nghttp2_session_callbacks_set_on_data_chunk_recv_callback( callbacks, OnDataChunkReceived); + // nghttp2_session_callbacks_set_on_invalid_frame_recv( + // callbacks, OnInvalidFrameReceived); + +#ifdef NODE_DEBUG_HTTP2 + nghttp2_session_callbacks_set_error_callback( + callbacks, OnNghttpError); +#endif + if (kHasGetPaddingCallback) { nghttp2_session_callbacks_set_select_padding_callback( callbacks, OnSelectPadding); diff --git a/src/node_http2_core.cc b/src/node_http2_core.cc index 55460fc98f..164def1267 100644 --- a/src/node_http2_core.cc +++ b/src/node_http2_core.cc @@ -3,6 +3,18 @@ namespace node { namespace http2 { +#ifdef NODE_DEBUG_HTTP2 +int Nghttp2Session::OnNghttpError(nghttp2_session* session, + const char* message, + size_t len, + void* user_data) { + Nghttp2Session* handle = static_cast(user_data); + DEBUG_HTTP2("Nghttp2Session %d: Error '%.*s'\n", + handle->session_type_, len, message); + return 0; +} +#endif + // nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame. // We use it to ensure that an Nghttp2Stream instance is allocated to store // the state. @@ -15,6 +27,7 @@ int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session, frame->hd.stream_id; DEBUG_HTTP2("Nghttp2Session %d: beginning headers for stream %d\n", handle->session_type_, id); + Nghttp2Stream* stream = handle->FindStream(id); if (stream == nullptr) { Nghttp2Stream::Init(id, handle, frame->headers.cat); @@ -149,6 +162,9 @@ ssize_t Nghttp2Session::OnStreamRead(nghttp2_session* session, size_t remaining = length; size_t offset = 0; + // While there is data in the queue, copy data into buf until it is full. + // There may be data left over, which will be sent the next time nghttp + // calls this callback. while (stream->queue_head_ != nullptr) { DEBUG_HTTP2("Nghttp2Session %d: processing outbound data chunk\n", handle->session_type_); @@ -183,6 +199,13 @@ ssize_t Nghttp2Session::OnStreamRead(nghttp2_session* session, stream->queue_tail_ = nullptr; end: + // If we are no longer writable and there is no more data in the queue, + // then we need to set the NGHTTP2_DATA_FLAG_EOF flag. + // If we are still writable but there is not yet any data to send, set the + // NGHTTP2_ERR_DEFERRED flag. This will put the stream into a pending state + // that will wait for data to become available. + // If neither of these flags are set, then nghttp2 will call this callback + // again to get the data for the next DATA frame. int writable = stream->queue_head_ != nullptr || stream->IsWritable(); if (offset == 0 && writable && stream->queue_head_ == nullptr) { DEBUG_HTTP2("Nghttp2Session %d: deferring stream %d\n", @@ -194,6 +217,10 @@ 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 trailers; handle->OnTrailers(stream, &trailers); if (trailers.length() > 0) { diff --git a/src/node_http2_core.h b/src/node_http2_core.h index f4d0372868..cd21620696 100644 --- a/src/node_http2_core.h +++ b/src/node_http2_core.h @@ -167,6 +167,13 @@ class Nghttp2Session { inline void HandleDataFrame(const nghttp2_frame* frame); /* callbacks for nghttp2 */ +#ifdef NODE_DEBUG_HTTP2 + static int OnNghttpError(nghttp2_session* session, + const char* message, + size_t len, + void* user_data); +#endif + static int OnBeginHeadersCallback(nghttp2_session* session, const nghttp2_frame* frame, void* user_data); @@ -238,9 +245,11 @@ class Nghttp2Stream { CHECK_EQ(data_chunks_tail_, nullptr); CHECK_EQ(current_headers_head_, nullptr); CHECK_EQ(current_headers_tail_, nullptr); - DEBUG_HTTP2("Nghttp2Stream: freed\n"); + DEBUG_HTTP2("Nghttp2Stream %d: freed\n", id_); } + inline void FlushDataChunks(); + // Resets the state of the stream instance to defaults inline void ResetState( int32_t id, diff --git a/test/parallel/test-http2-timeouts.js b/test/parallel/test-http2-timeouts.js new file mode 100644 index 0000000000..2262472a41 --- /dev/null +++ b/test/parallel/test-http2-timeouts.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../common'); +const h2 = require('http2'); + +const server = h2.createServer(); + +// we use the lower-level API here +server.on('stream', common.mustCall((stream) => { + stream.setTimeout(1, common.mustCall(() => { + stream.respond({':status': 200}); + stream.end('hello world'); + })); +})); +server.listen(0); + +server.on('listening', common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); + client.setTimeout(1, common.mustCall(() => { + const req = client.request({ ':path': '/' }); + req.setTimeout(1, common.mustCall(() => { + req.on('response', common.mustCall()); + req.resume(); + req.on('end', common.mustCall(() => { + server.close(); + client.destroy(); + })); + req.end(); + })); + })); +}));