diff options
21 files changed, 863 insertions, 179 deletions
diff --git a/doc/api/errors.md b/doc/api/errors.md index f217103e36..1b78f2c337 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -802,6 +802,12 @@ SETTINGS. By default, a maximum number of un-acknowledged `SETTINGS` frame may be sent at any given time. This error code is used when that limit has been reached. +<a id="ERR_HTTP2_NO_SOCKET_MANIPULATION"></a> +### ERR_HTTP2_NO_SOCKET_MANIPULATION + +Used when attempting to read, write, pause, and/or resume a socket attached to +an `Http2Session`. + <a id="ERR_HTTP2_OUT_OF_STREAMS"></a> ### ERR_HTTP2_OUT_OF_STREAMS diff --git a/doc/api/http2.md b/doc/api/http2.md index d307f6d9d6..79536acd4f 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -2046,7 +2046,7 @@ console.log(request.headers); See [Headers Object][]. -### request.httpVersion +#### request.httpVersion <!-- YAML added: v8.4.0 --> @@ -2117,7 +2117,14 @@ added: v8.4.0 * `msecs` {number} * `callback` {Function} -Calls `request.connection.setTimeout(msecs, callback)`. +Sets the [`Http2Stream`]()'s timeout value to `msecs`. If a callback is +provided, then it is added as a listener on the `'timeout'` event on +the response object. + +If no `'timeout'` listener is added to the request, the response, or +the server, then [`Http2Stream`]()s are destroyed when they time out. If a +handler is assigned to the request, the response, or the server's `'timeout'` +events, timed out sockets must be handled explicitly. Returns `request`. @@ -2128,13 +2135,24 @@ added: v8.4.0 * {net.Socket} -The [`net.Socket`][] object associated with the connection. +Returns a Proxy object that acts as a `net.Socket` but applies getters, +setters and methods based on HTTP/2 logic. + +`destroyed`, `readable`, and `writable` properties will be retrieved from and +set on `request.stream`. -With TLS support, use [`request.socket.getPeerCertificate()`][] to obtain the -client's authentication details. +`destroy`, `emit`, `end`, `on` and `once` methods will be called on +`request.stream`. -*Note*: do not use this socket object to send or receive any data. All -data transfers are managed by HTTP/2 and data might be lost. +`setTimeout` method will be called on `request.stream.session`. + +`pause`, `read`, `resume`, and `write` will throw an error with code +`ERR_HTTP2_NO_SOCKET_MANIPULATION`. See [`Http2Session and Sockets`][] for +more information. + +All other interactions will be routed directly to the socket. With TLS support, +use [`request.socket.getPeerCertificate()`][] to obtain the client's +authentication details. #### request.stream <!-- YAML @@ -2232,7 +2250,7 @@ passed as the second parameter to the [`'request'`][] event. The response implements, but does not inherit from, the [Writable Stream][] interface. This is an [`EventEmitter`][] with the following events: -### Event: 'close' +#### Event: 'close' <!-- YAML added: v8.4.0 --> @@ -2240,7 +2258,7 @@ added: v8.4.0 Indicates that the underlying [`Http2Stream`]() was terminated before [`response.end()`][] was called or able to flush. -### Event: 'finish' +#### Event: 'finish' <!-- YAML added: v8.4.0 --> @@ -2252,7 +2270,7 @@ does not imply that the client has received anything yet. After this event, no more events will be emitted on the response object. -### response.addTrailers(headers) +#### response.addTrailers(headers) <!-- YAML added: v8.4.0 --> @@ -2265,7 +2283,7 @@ message) to the response. Attempting to set a header field name or value that contains invalid characters will result in a [`TypeError`][] being thrown. -### response.connection +#### response.connection <!-- YAML added: v8.4.0 --> @@ -2274,7 +2292,7 @@ added: v8.4.0 See [`response.socket`][]. -### response.end([data][, encoding][, callback]) +#### response.end([data][, encoding][, callback]) <!-- YAML added: v8.4.0 --> @@ -2293,7 +2311,7 @@ If `data` is specified, it is equivalent to calling If `callback` is specified, it will be called when the response stream is finished. -### response.finished +#### response.finished <!-- YAML added: v8.4.0 --> @@ -2303,7 +2321,7 @@ added: v8.4.0 Boolean value that indicates whether the response has completed. Starts as `false`. After [`response.end()`][] executes, the value will be `true`. -### response.getHeader(name) +#### response.getHeader(name) <!-- YAML added: v8.4.0 --> @@ -2320,7 +2338,7 @@ Example: const contentType = response.getHeader('content-type'); ``` -### response.getHeaderNames() +#### response.getHeaderNames() <!-- YAML added: v8.4.0 --> @@ -2340,7 +2358,7 @@ const headerNames = response.getHeaderNames(); // headerNames === ['foo', 'set-cookie'] ``` -### response.getHeaders() +#### response.getHeaders() <!-- YAML added: v8.4.0 --> @@ -2368,7 +2386,7 @@ const headers = response.getHeaders(); // headers === { foo: 'bar', 'set-cookie': ['foo=bar', 'bar=baz'] } ``` -### response.hasHeader(name) +#### response.hasHeader(name) <!-- YAML added: v8.4.0 --> @@ -2385,7 +2403,7 @@ Example: const hasContentType = response.hasHeader('content-type'); ``` -### response.headersSent +#### response.headersSent <!-- YAML added: v8.4.0 --> @@ -2394,7 +2412,7 @@ added: v8.4.0 Boolean (read-only). True if headers were sent, false otherwise. -### response.removeHeader(name) +#### response.removeHeader(name) <!-- YAML added: v8.4.0 --> @@ -2409,7 +2427,7 @@ Example: response.removeHeader('Content-Encoding'); ``` -### response.sendDate +#### response.sendDate <!-- YAML added: v8.4.0 --> @@ -2422,7 +2440,7 @@ the response if it is not already present in the headers. Defaults to true. This should only be disabled for testing; HTTP requires the Date header in responses. -### response.setHeader(name, value) +#### response.setHeader(name, value) <!-- YAML added: v8.4.0 --> @@ -2463,7 +2481,7 @@ const server = http2.createServer((req, res) => { }); ``` -### response.setTimeout(msecs[, callback]) +#### response.setTimeout(msecs[, callback]) <!-- YAML added: v8.4.0 --> @@ -2482,18 +2500,29 @@ events, timed out sockets must be handled explicitly. Returns `response`. -### response.socket +#### response.socket <!-- YAML added: v8.4.0 --> * {net.Socket} -Reference to the underlying socket. Usually users will not want to access -this property. In particular, the socket will not emit `'readable'` events -because of how the protocol parser attaches to the socket. After -`response.end()`, the property is nulled. The `socket` may also be accessed -via `response.connection`. +Returns a Proxy object that acts as a `net.Socket` but applies getters, +setters and methods based on HTTP/2 logic. + +`destroyed`, `readable`, and `writable` properties will be retrieved from and +set on `response.stream`. + +`destroy`, `emit`, `end`, `on` and `once` methods will be called on +`response.stream`. + +`setTimeout` method will be called on `response.stream.session`. + +`pause`, `read`, `resume`, and `write` will throw an error with code +`ERR_HTTP2_NO_SOCKET_MANIPULATION`. See [`Http2Session and Sockets`][] for +more information. + +All other interactions will be routed directly to the socket. Example: @@ -2506,7 +2535,7 @@ const server = http2.createServer((req, res) => { }).listen(3000); ``` -### response.statusCode +#### response.statusCode <!-- YAML added: v8.4.0 --> @@ -2526,7 +2555,7 @@ response.statusCode = 404; After response header was sent to the client, this property indicates the status code which was sent out. -### response.statusMessage +#### response.statusMessage <!-- YAML added: v8.4.0 --> @@ -2545,7 +2574,7 @@ added: v8.4.0 The [`Http2Stream`][] object backing the response. -### response.write(chunk[, encoding][, callback]) +#### response.write(chunk[, encoding][, callback]) <!-- YAML added: v8.4.0 --> @@ -2583,7 +2612,7 @@ Returns `true` if the entire data was flushed successfully to the kernel buffer. Returns `false` if all or part of the data was queued in user memory. `'drain'` will be emitted when the buffer is free again. -### response.writeContinue() +#### response.writeContinue() <!-- YAML added: v8.4.0 --> @@ -2592,7 +2621,7 @@ Sends a status `100 Continue` to the client, indicating that the request body should be sent. See the [`'checkContinue'`][] event on `Http2Server` and `Http2SecureServer`. -### response.writeHead(statusCode[, statusMessage][, headers]) +#### response.writeHead(statusCode[, statusMessage][, headers]) <!-- YAML added: v8.4.0 --> @@ -2648,7 +2677,7 @@ const server = http2.createServer((req, res) => { Attempting to set a header field name or value that contains invalid characters will result in a [`TypeError`][] being thrown. -### response.createPushResponse(headers, callback) +#### response.createPushResponse(headers, callback) <!-- YAML added: v8.4.0 --> diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 6b9c31a528..f49bb2e542 100755 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -206,6 +206,9 @@ E('ERR_HTTP2_INVALID_SETTING_VALUE', E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed'); E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', (max) => `Maximum number of pending settings acknowledgements (${max})`); +E('ERR_HTTP2_NO_SOCKET_MANIPULATION', + 'HTTP/2 sockets should not be directly read from, written to, ' + + 'paused and/or resumed.'); E('ERR_HTTP2_OUT_OF_STREAMS', 'No stream ID is available because maximum stream ID has been reached'); E('ERR_HTTP2_PAYLOAD_FORBIDDEN', diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 462b0c51b8..84f15b2ed8 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -16,10 +16,10 @@ const kHeaders = Symbol('headers'); const kRawHeaders = Symbol('rawHeaders'); const kTrailers = Symbol('trailers'); const kRawTrailers = Symbol('rawTrailers'); +const kProxySocket = Symbol('proxySocket'); +const kSetHeader = Symbol('setHeader'); const { - NGHTTP2_NO_ERROR, - HTTP2_HEADER_AUTHORITY, HTTP2_HEADER_METHOD, HTTP2_HEADER_PATH, @@ -39,7 +39,7 @@ let statusMessageWarned = false; // close as possible to the current require('http') API function assertValidHeader(name, value) { - if (name === '') + if (name === '' || typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name); if (isPseudoHeader(name)) throw new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED'); @@ -71,19 +71,24 @@ function statusMessageWarn() { } function onStreamData(chunk) { - if (!this[kRequest].push(chunk)) + const request = this[kRequest]; + if (request !== undefined && !request.push(chunk)) this.pause(); } function onStreamTrailers(trailers, flags, rawTrailers) { const request = this[kRequest]; - Object.assign(request[kTrailers], trailers); - request[kRawTrailers].push(...rawTrailers); + if (request !== undefined) { + Object.assign(request[kTrailers], trailers); + request[kRawTrailers].push(...rawTrailers); + } } function onStreamEnd() { // Cause the request stream to end as well. - this[kRequest].push(null); + const request = this[kRequest]; + if (request !== undefined) + this[kRequest].push(null); } function onStreamError(error) { @@ -97,62 +102,136 @@ function onStreamError(error) { } function onRequestPause() { - const stream = this[kStream]; - if (stream) - stream.pause(); + this[kStream].pause(); } function onRequestResume() { - const stream = this[kStream]; - if (stream) - stream.resume(); + this[kStream].resume(); } function onStreamDrain() { - this[kResponse].emit('drain'); + const response = this[kResponse]; + if (response !== undefined) + response.emit('drain'); } // TODO Http2Stream does not emit 'close' function onStreamClosedRequest() { - this[kRequest].push(null); + const request = this[kRequest]; + if (request !== undefined) + request.push(null); } // TODO Http2Stream does not emit 'close' function onStreamClosedResponse() { - this[kResponse].emit('finish'); + const response = this[kResponse]; + if (response !== undefined) + response.emit('finish'); } -function onStreamAbortedRequest(hadError, code) { +function onStreamAbortedRequest() { const request = this[kRequest]; - if (request[kState].closed === false) { - request.emit('aborted', hadError, code); + if (request !== undefined && request[kState].closed === false) { + request.emit('aborted'); request.emit('close'); } } function onStreamAbortedResponse() { const response = this[kResponse]; - if (response[kState].closed === false) { + if (response !== undefined && response[kState].closed === false) response.emit('close'); - } } function resumeStream(stream) { stream.resume(); } +const proxySocketHandler = { + get(stream, prop) { + switch (prop) { + case 'on': + case 'once': + case 'end': + case 'emit': + case 'destroy': + return stream[prop].bind(stream); + case 'writable': + case 'destroyed': + return stream[prop]; + case 'readable': + if (stream.destroyed) + return false; + const request = stream[kRequest]; + return request ? request.readable : stream.readable; + case 'setTimeout': + const session = stream.session; + if (session !== undefined) + return session.setTimeout.bind(session); + return stream.setTimeout.bind(stream); + case 'write': + case 'read': + case 'pause': + case 'resume': + throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION'); + default: + const ref = stream.session !== undefined ? + stream.session.socket : stream; + const value = ref[prop]; + return typeof value === 'function' ? value.bind(ref) : value; + } + }, + getPrototypeOf(stream) { + if (stream.session !== undefined) + return stream.session.socket.constructor.prototype; + return stream.prototype; + }, + set(stream, prop, value) { + switch (prop) { + case 'writable': + case 'readable': + case 'destroyed': + case 'on': + case 'once': + case 'end': + case 'emit': + case 'destroy': + stream[prop] = value; + return true; + case 'setTimeout': + const session = stream.session; + if (session !== undefined) + session[prop] = value; + else + stream[prop] = value; + return true; + case 'write': + case 'read': + case 'pause': + case 'resume': + throw new errors.Error('ERR_HTTP2_NO_SOCKET_MANIPULATION'); + default: + const ref = stream.session !== undefined ? + stream.session.socket : stream; + ref[prop] = value; + return true; + } + } +}; + class Http2ServerRequest extends Readable { constructor(stream, headers, options, rawHeaders) { super(options); this[kState] = { closed: false, - closedCode: NGHTTP2_NO_ERROR + didRead: false, }; this[kHeaders] = headers; this[kRawHeaders] = rawHeaders; this[kTrailers] = {}; this[kRawTrailers] = []; this[kStream] = stream; + stream[kProxySocket] = null; stream[kRequest] = this; // Pause the stream.. @@ -170,12 +249,10 @@ class Http2ServerRequest extends Readable { this.on('resume', onRequestResume); } - get closed() { - return this[kState].closed; - } - - get code() { - return this[kState].closedCode; + get complete() { + return this._readableState.ended || + this[kState].closed || + this[kStream].destroyed; } get stream() { @@ -212,9 +289,10 @@ class Http2ServerRequest extends Readable { get socket() { const stream = this[kStream]; - if (stream === undefined) - return; - return stream.session.socket; + const proxySocket = stream[kProxySocket]; + if (proxySocket === null) + return stream[kProxySocket] = new Proxy(stream, proxySocketHandler); + return proxySocket; } get connection() { @@ -222,9 +300,10 @@ class Http2ServerRequest extends Readable { } _read(nread) { - const stream = this[kStream]; - if (stream !== undefined) { - process.nextTick(resumeStream, stream); + const state = this[kState]; + if (!state.closed) { + state.didRead = true; + process.nextTick(resumeStream, this[kStream]); } else { this.emit('error', new errors.Error('ERR_HTTP2_STREAM_CLOSED')); } @@ -234,6 +313,13 @@ class Http2ServerRequest extends Readable { return this[kHeaders][HTTP2_HEADER_METHOD]; } + set method(method) { + if (typeof method !== 'string' || method.trim() === '') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'method', 'string'); + + this[kHeaders][HTTP2_HEADER_METHOD] = method; + } + get authority() { return this[kHeaders][HTTP2_HEADER_AUTHORITY]; } @@ -256,15 +342,17 @@ class Http2ServerRequest extends Readable { this[kStream].setTimeout(msecs, callback); } - [kFinish](code) { + [kFinish]() { const state = this[kState]; if (state.closed) return; - if (code !== undefined) - state.closedCode = Number(code); state.closed = true; this.push(null); - process.nextTick(() => (this[kStream] = undefined)); + this[kStream][kRequest] = undefined; + // if the user didn't interact with incoming data and didn't pipe it, + // dump it for compatibility with http1 + if (!state.didRead && !this._readableState.resumeScheduled) + this.resume(); } } @@ -272,14 +360,16 @@ class Http2ServerResponse extends Stream { constructor(stream, options) { super(options); this[kState] = { + closed: false, + ending: false, + headRequest: false, sendDate: true, statusCode: HTTP_STATUS_OK, - closed: false, - closedCode: NGHTTP2_NO_ERROR }; this[kHeaders] = Object.create(null); this[kTrailers] = Object.create(null); this[kStream] = stream; + stream[kProxySocket] = null; stream[kResponse] = this; this.writable = true; stream.on('drain', onStreamDrain); @@ -290,17 +380,35 @@ class Http2ServerResponse extends Stream { stream.on('finish', onfinish); } + // User land modules such as finalhandler just check truthiness of this + // but if someone is actually trying to use this for more than that + // then we simply can't support such use cases + get _header() { + return this.headersSent; + } + get finished() { const stream = this[kStream]; - return stream === undefined || stream._writableState.ended; + return stream.destroyed || + stream._writableState.ended || + this[kState].closed; } - get closed() { - return this[kState].closed; + get socket() { + // this is compatible with http1 which removes socket reference + // only from ServerResponse but not IncomingMessage + if (this[kState].closed) + return; + + const stream = this[kStream]; + const proxySocket = stream[kProxySocket]; + if (proxySocket === null) + return stream[kProxySocket] = new Proxy(stream, proxySocketHandler); + return proxySocket; } - get code() { - return this[kState].closedCode; + get connection() { + return this.socket; } get stream() { @@ -308,8 +416,7 @@ class Http2ServerResponse extends Stream { } get headersSent() { - const stream = this[kStream]; - return stream !== undefined ? stream.headersSent : this[kState].headersSent; + return this[kStream].headersSent; } get sendDate() { @@ -339,7 +446,7 @@ class Http2ServerResponse extends Stream { name = name.trim().toLowerCase(); assertValidHeader(name, value); - this[kTrailers][name] = String(value); + this[kTrailers][name] = value; } addTrailers(headers) { @@ -379,6 +486,9 @@ class Http2ServerResponse extends Stream { if (typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + if (this[kStream].headersSent) + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + name = name.trim().toLowerCase(); delete this[kHeaders][name]; } @@ -387,9 +497,16 @@ class Http2ServerResponse extends Stream { if (typeof name !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'name', 'string'); + if (this[kStream].headersSent) + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + + this[kSetHeader](name, value); + } + + [kSetHeader](name, value) { name = name.trim().toLowerCase(); assertValidHeader(name, value); - this[kHeaders][name] = String(value); + this[kHeaders][name] = value; } get statusMessage() { @@ -403,50 +520,45 @@ class Http2ServerResponse extends Stream { } flushHeaders() { - const stream = this[kStream]; - if (stream !== undefined && stream.headersSent === false) - this[kBeginSend](); + const state = this[kState]; + if (!state.closed && !this[kStream].headersSent) + this.writeHead(state.statusCode); } writeHead(statusCode, statusMessage, headers) { - if (typeof statusMessage === 'string') { + const state = this[kState]; + + if (state.closed) + throw new errors.Error('ERR_HTTP2_STREAM_CLOSED'); + if (this[kStream].headersSent) + throw new errors.Error('ERR_HTTP2_HEADERS_SENT'); + + if (typeof statusMessage === 'string') statusMessageWarn(); - } - if (headers === undefined && typeof statusMessage === 'object') { + if (headers === undefined && typeof statusMessage === 'object') headers = statusMessage; - } - - const stream = this[kStream]; - if (stream === undefined) { - throw new errors.Error('ERR_HTTP2_STREAM_CLOSED'); - } - if (stream.headersSent === true) { - throw new errors.Error('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND'); - } if (typeof headers === 'object') { const keys = Object.keys(headers); let key = ''; for (var i = 0; i < keys.length; i++) { key = keys[i]; - this.setHeader(key, headers[key]); + this[kSetHeader](key, headers[key]); } } - this.statusCode = statusCode; + state.statusCode = statusCode; this[kBeginSend](); } write(chunk, encoding, cb) { - const stream = this[kStream]; - if (typeof encoding === 'function') { cb = encoding; encoding = 'utf8'; } - if (stream === undefined) { + if (this[kState].closed) { const err = new errors.Error('ERR_HTTP2_STREAM_CLOSED'); if (typeof cb === 'function') process.nextTick(cb, err); @@ -454,12 +566,21 @@ class Http2ServerResponse extends Stream { throw err; return; } - this[kBeginSend](); + + const stream = this[kStream]; + if (!stream.headersSent) + this.writeHead(this[kState].statusCode); return stream.write(chunk, encoding, cb); } end(chunk, encoding, cb) { const stream = this[kStream]; + const state = this[kState]; + + if ((state.closed || state.ending) && + state.headRequest === stream.headRequest) { + return false; + } if (typeof chunk === 'function') { cb = chunk; @@ -468,19 +589,28 @@ class Http2ServerResponse extends Stream { cb = encoding; encoding = 'utf8'; } - if (this.finished === true) { - return false; - } - if (chunk !== null && chunk !== undefined) { + + if (chunk !== null && chunk !== undefined) this.write(chunk, encoding); - } + + const isFinished = this.finished; + state.headRequest = stream.headRequest; + state.ending = true; if (typeof cb === 'function') { - stream.once('finish', cb); + if (isFinished) + this.once('finish', cb); + else + stream.once('finish', cb); } - this[kBeginSend]({ endStream: true }); - stream.end(); + if (!stream.headersSent) + this.writeHead(this[kState].statusCode); + + if (isFinished) + this[kFinish](); + else + stream.end(); } destroy(err) { @@ -490,63 +620,52 @@ class Http2ServerResponse extends Stream { } setTimeout(msecs, callback) { - const stream = this[kStream]; if (this[kState].closed) return; - stream.setTimeout(msecs, callback); + this[kStream].setTimeout(msecs, callback); } createPushResponse(headers, callback) { if (typeof callback !== 'function') throw new errors.TypeError('ERR_INVALID_CALLBACK'); - const stream = this[kStream]; - if (stream === undefined) { + if (this[kState].closed) { process.nextTick(callback, new errors.Error('ERR_HTTP2_STREAM_CLOSED')); return; } - stream.pushStream(headers, {}, function(stream, headers, options) { + this[kStream].pushStream(headers, {}, function(stream, headers, options) { const response = new Http2ServerResponse(stream); callback(null, response); }); } - [kBeginSend](options) { - const stream = this[kStream]; - if (stream !== undefined && - stream.destroyed === false && - stream.headersSent === false) { - const headers = this[kHeaders]; - headers[HTTP2_HEADER_STATUS] = this[kState].statusCode; - options = options || Object.create(null); - options.getTrailers = (trailers) => { - Object.assign(trailers, this[kTrailers]); - }; - stream.respond(headers, options); - } + [kBeginSend]() { + const state = this[kState]; + const headers = this[kHeaders]; + headers[HTTP2_HEADER_STATUS] = state.statusCode; + const options = { + endStream: state.ending, + getTrailers: (trailers) => Object.assign(trailers, this[kTrailers]) + }; + this[kStream].respond(headers, options); } - [kFinish](code) { + [kFinish]() { + const stream = this[kStream]; const state = this[kState]; - if (state.closed) + if (state.closed || stream.headRequest !== state.headRequest) return; - if (code !== undefined) - state.closedCode = Number(code); state.closed = true; - state.headersSent = this[kStream].headersSent; - this.end(); - process.nextTick(() => (this[kStream] = undefined)); + this[kProxySocket] = null; + stream[kResponse] = undefined; this.emit('finish'); } // TODO doesn't support callbacks writeContinue() { const stream = this[kStream]; - if (stream === undefined || - stream.headersSent === true || - stream.destroyed === true) { + if (stream.headersSent || this[kState].closed) return false; - } - this[kStream].additionalHeaders({ + stream.additionalHeaders({ [HTTP2_HEADER_STATUS]: HTTP_STATUS_CONTINUE }); return true; diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index c88d9f5231..aa9a7b774d 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -166,8 +166,7 @@ function onSessionHeaders(id, cat, flags, headers) { // For head requests, there must not be a body... // end the writable side immediately. stream.end(); - const state = stream[kState]; - state.headRequest = true; + stream[kState].headRequest = true; } } else { stream = new ClientHttp2Stream(owner, id, { readable: !endOfStream }); @@ -1277,6 +1276,7 @@ class Http2Stream extends Duplex { rst: false, rstCode: NGHTTP2_NO_ERROR, headersSent: false, + headRequest: false, aborted: false, closeHandler: onSessionClose.bind(this) }; @@ -1333,6 +1333,11 @@ class Http2Stream extends Duplex { return this[kState].aborted; } + // true if dealing with a HEAD request + get headRequest() { + return this[kState].headRequest; + } + // The error code reported when this Http2Stream was closed. get rstCode() { return this[kState].rst ? this[kState].rstCode : undefined; @@ -1533,12 +1538,15 @@ function continueStreamDestroy(self, err, callback) { // All done const rst = state.rst; const code = rst ? state.rstCode : NGHTTP2_NO_ERROR; - if (!err && code !== NGHTTP2_NO_ERROR) { + // RST code 8 not emitted as an error as its used by clients to signify + // abort and is already covered by aborted event, also allows more + // seamless compatibility with http1 + if (!err && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL) { err = new errors.Error('ERR_HTTP2_STREAM_ERROR', code); } + callback(err); process.nextTick(emit, self, 'streamClosed', code); debug(`[${sessionName(session[kType])}] stream ${self[kID]} destroyed`); - callback(err); } function finishStreamDestroy(self, handle) { diff --git a/test/parallel/test-http2-compat-serverrequest-end.js b/test/parallel/test-http2-compat-serverrequest-end.js index 60a4876482..b6bfd04089 100644 --- a/test/parallel/test-http2-compat-serverrequest-end.js +++ b/test/parallel/test-http2-compat-serverrequest-end.js @@ -3,6 +3,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const assert = require('assert'); const h2 = require('http2'); // Http2ServerRequest should always end readable stream @@ -12,9 +13,17 @@ const server = h2.createServer(); server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { + assert.strictEqual(request.complete, false); request.on('data', () => {}); request.on('end', common.mustCall(() => { + assert.strictEqual(request.complete, true); response.on('finish', common.mustCall(function() { + // the following tests edge cases on request socket + // right after finished fires but before backing + // Http2Stream is destroyed + assert.strictEqual(request.socket.readable, request.stream.readable); + assert.strictEqual(request.socket.readable, false); + server.close(); })); response.end(); diff --git a/test/parallel/test-http2-compat-serverrequest-headers.js b/test/parallel/test-http2-compat-serverrequest-headers.js index 05e645a362..58cc52c64f 100644 --- a/test/parallel/test-http2-compat-serverrequest-headers.js +++ b/test/parallel/test-http2-compat-serverrequest-headers.js @@ -41,6 +41,27 @@ server.listen(0, common.mustCall(function() { request.url = '/one'; assert.strictEqual(request.url, '/one'); + // third-party plugins for packages like express use query params to + // change the request method + request.method = 'POST'; + assert.strictEqual(request.method, 'POST'); + common.expectsError( + () => request.method = ' ', + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "method" argument must be of type string' + } + ); + common.expectsError( + () => request.method = true, + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "method" argument must be of type string' + } + ); + response.on('finish', common.mustCall(function() { server.close(); })); diff --git a/test/parallel/test-http2-compat-serverrequest-pipe.js b/test/parallel/test-http2-compat-serverrequest-pipe.js index a8ef688fd7..24cee1c460 100644 --- a/test/parallel/test-http2-compat-serverrequest-pipe.js +++ b/test/parallel/test-http2-compat-serverrequest-pipe.js @@ -19,6 +19,7 @@ const server = http2.createServer(); server.on('request', common.mustCall((req, res) => { const dest = req.pipe(fs.createWriteStream(fn)); dest.on('finish', common.mustCall(() => { + assert.strictEqual(req.complete, true); assert.deepStrictEqual(fs.readFileSync(loc), fs.readFileSync(fn)); fs.unlinkSync(fn); res.end(); diff --git a/test/parallel/test-http2-compat-serverrequest.js b/test/parallel/test-http2-compat-serverrequest.js index 31cc5ff4ef..edcd7a8f8c 100644 --- a/test/parallel/test-http2-compat-serverrequest.js +++ b/test/parallel/test-http2-compat-serverrequest.js @@ -19,9 +19,6 @@ server.listen(0, common.mustCall(function() { httpVersionMinor: 0 }; - assert.strictEqual(request.closed, false); - assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); - assert.strictEqual(request.httpVersion, expected.version); assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor); assert.strictEqual(request.httpVersionMinor, expected.httpVersionMinor); @@ -31,10 +28,8 @@ server.listen(0, common.mustCall(function() { assert.strictEqual(request.socket, request.connection); response.on('finish', common.mustCall(function() { - assert.strictEqual(request.closed, true); - assert.strictEqual(request.code, h2.constants.NGHTTP2_NO_ERROR); process.nextTick(() => { - assert.strictEqual(request.socket, undefined); + assert.ok(request.socket); server.close(); }); })); diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index e8f648bf29..77e761b622 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -22,7 +22,6 @@ const server = http2.createServer(common.mustCall((req, res) => { res.on('finish', common.mustCall(() => { assert.doesNotThrow(() => res.destroy(nextError)); - assert.strictEqual(res.closed, true); process.nextTick(() => { assert.doesNotThrow(() => res.destroy(nextError)); }); diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 76cb692938..fafb3ea76d 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -1,6 +1,12 @@ 'use strict'; -const { mustCall, mustNotCall, hasCrypto, skip } = require('../common'); +const { + mustCall, + mustNotCall, + hasCrypto, + platformTimeout, + skip +} = require('../common'); if (!hasCrypto) skip('missing crypto'); const { strictEqual } = require('assert'); @@ -18,15 +24,16 @@ const { // It may be invoked repeatedly without throwing errors // but callback will only be called once const server = createServer(mustCall((request, response) => { - strictEqual(response.closed, false); response.end('end', 'utf8', mustCall(() => { - strictEqual(response.closed, true); response.end(mustNotCall()); process.nextTick(() => { response.end(mustNotCall()); server.close(); }); })); + response.on('finish', mustCall(() => { + response.end(mustNotCall()); + })); response.end(mustNotCall()); })); server.listen(0, mustCall(() => { @@ -111,12 +118,77 @@ const { } { - // Http2ServerResponse.end is not necessary on HEAD requests since the stream - // is already closed. Headers, however, can still be sent to the client. + // Http2ServerResponse.end is necessary on HEAD requests in compat + // for http1 compatibility const server = createServer(mustCall((request, response) => { strictEqual(response.finished, true); response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); - response.end(mustNotCall()); + response.end('data', mustCall()); + })); + 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(); + })); + })); +} + +{ + // .end should trigger 'end' event on request if user did not attempt + // to read from the request + const server = createServer(mustCall((request, response) => { + request.on('end', mustCall()); + response.end(); + })); + 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('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} + + +{ + // Should be able to call .end with cb from stream 'streamClosed' + const server = createServer(mustCall((request, response) => { + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.stream.on('streamClosed', mustCall(() => { + response.end(mustCall()); + })); })); server.listen(0, mustCall(() => { const { port } = server.address(); @@ -144,3 +216,110 @@ const { })); })); } + +{ + // Should be able to respond to HEAD request after timeout + const server = createServer(mustCall((request, response) => { + setTimeout(mustCall(() => { + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.end('data', mustCall()); + }), platformTimeout(10)); + })); + 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(); + })); + })); +} + +{ + // finish should only trigger after 'end' is called + const server = createServer(mustCall((request, response) => { + let finished = false; + response.writeHead(HTTP_STATUS_OK, { foo: 'bar' }); + response.on('finish', mustCall(() => { + finished = false; + })); + response.end('data', mustCall(() => { + strictEqual(finished, false); + response.end('data', 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(); + })); + })); +} + +{ + // Should be able to respond to HEAD with just .end + const server = createServer(mustCall((request, response) => { + response.end('data', mustCall()); + 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 + })); + request.on('data', mustNotCall()); + request.on('end', mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); + })); +} diff --git a/test/parallel/test-http2-compat-serverresponse-finished.js b/test/parallel/test-http2-compat-serverresponse-finished.js index cd17ff492b..b816b92220 100644 --- a/test/parallel/test-http2-compat-serverresponse-finished.js +++ b/test/parallel/test-http2-compat-serverresponse-finished.js @@ -5,19 +5,23 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); const h2 = require('http2'); +const net = require('net'); // Http2ServerResponse.finished const server = h2.createServer(); server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { + assert.ok(response.socket instanceof net.Socket); + assert.ok(response.connection instanceof net.Socket); + assert.strictEqual(response.socket, response.connection); + response.on('finish', common.mustCall(function() { - assert.ok(request.stream !== undefined); - assert.ok(response.stream !== undefined); - server.close(); + assert.strictEqual(response.socket, undefined); + assert.strictEqual(response.connection, undefined); process.nextTick(common.mustCall(() => { - assert.strictEqual(request.stream, undefined); - assert.strictEqual(response.stream, undefined); + assert.ok(response.stream); + server.close(); })); })); assert.strictEqual(response.finished, false); diff --git a/test/parallel/test-http2-compat-serverresponse-flushheaders.js b/test/parallel/test-http2-compat-serverresponse-flushheaders.js index 48bf4dd2d4..68d4789f69 100644 --- a/test/parallel/test-http2-compat-serverresponse-flushheaders.js +++ b/test/parallel/test-http2-compat-serverresponse-flushheaders.js @@ -15,18 +15,23 @@ server.listen(0, common.mustCall(function() { const port = server.address().port; server.once('request', common.mustCall(function(request, response) { assert.strictEqual(response.headersSent, false); + assert.strictEqual(response._header, false); // alias for headersSent response.flushHeaders(); assert.strictEqual(response.headersSent, true); + assert.strictEqual(response._header, true); response.flushHeaders(); // Idempotent common.expectsError(() => { response.writeHead(400, { 'foo-bar': 'abc123' }); }, { - code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND' + code: 'ERR_HTTP2_HEADERS_SENT' }); response.on('finish', common.mustCall(function() { server.close(); + process.nextTick(() => { + response.flushHeaders(); // Idempotent + }); })); serverResponse = response; })); diff --git a/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js new file mode 100644 index 0000000000..fb1e369f78 --- /dev/null +++ b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js @@ -0,0 +1,47 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); + +// makes sure that Http2ServerResponse setHeader & removeHeader, do not throw +// any errors if the stream was destroyed before headers were sent + +const server = h2.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + server.once('request', common.mustCall(function(request, response) { + response.destroy(); + + response.on('finish', common.mustCall(() => { + assert.strictEqual(response.headersSent, false); + assert.doesNotThrow(() => response.setHeader('test', 'value')); + assert.doesNotThrow(() => response.removeHeader('test', 'value')); + + process.nextTick(() => { + assert.doesNotThrow(() => response.setHeader('test', 'value')); + assert.doesNotThrow(() => response.removeHeader('test', 'value')); + + server.close(); + }); + })); + })); + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(function() { + client.destroy(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-serverresponse-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js index 25ec4fc11b..aae1d96c55 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers.js +++ b/test/parallel/test-http2-compat-serverresponse-headers.js @@ -124,14 +124,44 @@ server.listen(0, common.mustCall(function() { response.sendDate = false; assert.strictEqual(response.sendDate, false); - assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); - response.on('finish', common.mustCall(function() { - assert.strictEqual(response.code, h2.constants.NGHTTP2_NO_ERROR); assert.strictEqual(response.headersSent, true); + + common.expectsError( + () => response.setHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + common.expectsError( + () => response.removeHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + process.nextTick(() => { - // can access headersSent after stream is undefined - assert.strictEqual(response.stream, undefined); + common.expectsError( + () => response.setHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + common.expectsError( + () => response.removeHeader(real, expectedValue), + { + code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, + message: 'Response has already been initiated.' + } + ); + assert.strictEqual(response.headersSent, true); server.close(); }); diff --git a/test/parallel/test-http2-compat-serverresponse-writehead.js b/test/parallel/test-http2-compat-serverresponse-writehead.js index f6d74630fd..704f199ca2 100644 --- a/test/parallel/test-http2-compat-serverresponse-writehead.js +++ b/test/parallel/test-http2-compat-serverresponse-writehead.js @@ -16,7 +16,7 @@ server.listen(0, common.mustCall(function() { response.writeHead(418, { 'foo-bar': 'abc123' }); // Override common.expectsError(() => { response.writeHead(300); }, { - code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND' + code: 'ERR_HTTP2_HEADERS_SENT' }); response.on('finish', common.mustCall(function() { diff --git a/test/parallel/test-http2-compat-socket-set.js b/test/parallel/test-http2-compat-socket-set.js new file mode 100644 index 0000000000..7411fef602 --- /dev/null +++ b/test/parallel/test-http2-compat-socket-set.js @@ -0,0 +1,107 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); + +// Tests behaviour of the proxied socket in Http2ServerRequest +// & Http2ServerResponse - specifically property setters + +const errMsg = { + code: 'ERR_HTTP2_NO_SOCKET_MANIPULATION', + type: Error, + message: 'HTTP/2 sockets should not be directly read from, written to, ' + + 'paused and/or resumed.' +}; + +const server = h2.createServer(); + +server.on('request', common.mustCall(function(request, response) { + const noop = () => {}; + + assert.strictEqual(request.stream.destroyed, false); + request.socket.destroyed = true; + assert.strictEqual(request.stream.destroyed, true); + request.socket.destroyed = false; + + assert.strictEqual(request.stream.readable, false); + request.socket.readable = true; + assert.strictEqual(request.stream.readable, true); + + assert.strictEqual(request.stream.writable, true); + request.socket.writable = false; + assert.strictEqual(request.stream.writable, false); + + const realOn = request.stream.on; + request.socket.on = noop; + assert.strictEqual(request.stream.on, noop); + request.stream.on = realOn; + + const realOnce = request.stream.once; + request.socket.once = noop; + assert.strictEqual(request.stream.once, noop); + request.stream.once = realOnce; + + const realEnd = request.stream.end; + request.socket.end = noop; + assert.strictEqual(request.stream.end, noop); + request.socket.end = common.mustCall(); + request.socket.end(); + request.stream.end = realEnd; + + const realEmit = request.stream.emit; + request.socket.emit = noop; + assert.strictEqual(request.stream.emit, noop); + request.stream.emit = realEmit; + + const realDestroy = request.stream.destroy; + request.socket.destroy = noop; + assert.strictEqual(request.stream.destroy, noop); + request.stream.destroy = realDestroy; + + request.socket.setTimeout = noop; + assert.strictEqual(request.stream.session.setTimeout, noop); + + assert.strictEqual(request.stream.session.socket._isProcessing, undefined); + request.socket._isProcessing = true; + assert.strictEqual(request.stream.session.socket._isProcessing, true); + + common.expectsError(() => request.socket.read = noop, errMsg); + common.expectsError(() => request.socket.write = noop, errMsg); + common.expectsError(() => request.socket.pause = noop, errMsg); + common.expectsError(() => request.socket.resume = noop, errMsg); + + request.stream.on('finish', common.mustCall(() => { + setImmediate(() => { + request.socket.setTimeout = noop; + assert.strictEqual(request.stream.setTimeout, noop); + + assert.strictEqual(request.stream._isProcessing, undefined); + request.socket._isProcessing = true; + assert.strictEqual(request.stream._isProcessing, true); + }); + })); + response.stream.destroy(); +})); + +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(() => { + client.destroy(); + server.close(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-compat-socket.js b/test/parallel/test-http2-compat-socket.js new file mode 100644 index 0000000000..8292232f74 --- /dev/null +++ b/test/parallel/test-http2-compat-socket.js @@ -0,0 +1,89 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); +const net = require('net'); + +// Tests behaviour of the proxied socket in Http2ServerRequest +// & Http2ServerResponse - this proxy socket should mimic the +// behaviour of http1 but against the http2 api & model + +const errMsg = { + code: 'ERR_HTTP2_NO_SOCKET_MANIPULATION', + type: Error, + message: 'HTTP/2 sockets should not be directly read from, written to, ' + + 'paused and/or resumed.' +}; + +const server = h2.createServer(); + +server.on('request', common.mustCall(function(request, response) { + assert.ok(request.socket instanceof net.Socket); + assert.ok(response.socket instanceof net.Socket); + assert.strictEqual(request.socket, response.socket); + + assert.ok(request.socket.readable); + request.resume(); + assert.ok(request.socket.writable); + assert.strictEqual(request.socket.destroyed, false); + + request.socket.setTimeout(987); + assert.strictEqual(request.stream.session._idleTimeout, 987); + request.socket.setTimeout(0); + + common.expectsError(() => request.socket.read(), errMsg); + common.expectsError(() => request.socket.write(), errMsg); + common.expectsError(() => request.socket.pause(), errMsg); + common.expectsError(() => request.socket.resume(), errMsg); + + // should have correct this context for socket methods & getters + assert.ok(request.socket.address() != null); + assert.ok(request.socket.remotePort); + + request.on('end', common.mustCall(() => { + assert.strictEqual(request.socket.readable, false); + assert.doesNotThrow(() => response.socket.destroy()); + })); + response.on('finish', common.mustCall(() => { + assert.ok(request.socket); + assert.strictEqual(response.socket, undefined); + assert.ok(request.socket.destroyed); + assert.strictEqual(request.socket.readable, false); + process.nextTick(() => { + assert.strictEqual(request.socket.writable, false); + server.close(); + }); + })); + + // properties that do not exist on the proxy are retrieved from the socket + assert.ok(request.socket._server); + assert.strictEqual(request.socket.connecting, false); + + // socket events are bound and emitted on Http2Stream + request.socket.on('streamClosed', common.mustCall()); + request.socket.once('streamClosed', common.mustCall()); + request.socket.on('testEvent', common.mustCall()); + request.socket.emit('testEvent'); +})); + +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(() => { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('end', common.mustCall(() => { + client.destroy(); + })); + request.end(); + request.resume(); + })); +})); diff --git a/test/parallel/test-http2-options-max-reserved-streams.js b/test/parallel/test-http2-options-max-reserved-streams.js index 5e61907770..17009a4c11 100644 --- a/test/parallel/test-http2-options-max-reserved-streams.js +++ b/test/parallel/test-http2-options-max-reserved-streams.js @@ -3,6 +3,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const assert = require('assert'); const h2 = require('http2'); const server = h2.createServer(); @@ -32,11 +33,9 @@ server.on('stream', common.mustCall((stream) => { }, common.mustCall((pushedStream) => { pushedStream.respond({ ':status': 200 }); pushedStream.on('aborted', common.mustCall()); - pushedStream.on('error', common.mustCall(common.expectsError({ - code: 'ERR_HTTP2_STREAM_ERROR', - type: Error, - message: 'Stream closed with error code 8' - }))); + pushedStream.on('error', common.mustNotCall()); + pushedStream.on('streamClosed', + common.mustCall((code) => assert.strictEqual(code, 8))); })); stream.end('hello world'); diff --git a/test/parallel/test-http2-server-rst-stream.js b/test/parallel/test-http2-server-rst-stream.js index edaaed35c6..b92217dc99 100644 --- a/test/parallel/test-http2-server-rst-stream.js +++ b/test/parallel/test-http2-server-rst-stream.js @@ -17,7 +17,7 @@ const { NGHTTP2_INTERNAL_ERROR } = http2.constants; -const errCheck = common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR' }, 8); +const errCheck = common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR' }, 6); function checkRstCode(rstMethod, expectRstCode) { const server = http2.createServer(); @@ -32,8 +32,11 @@ function checkRstCode(rstMethod, expectRstCode) { else stream[rstMethod](); - if (expectRstCode > NGHTTP2_NO_ERROR) { + if (expectRstCode !== NGHTTP2_NO_ERROR && + expectRstCode !== NGHTTP2_CANCEL) { stream.on('error', common.mustCall(errCheck)); + } else { + stream.on('error', common.mustNotCall()); } }); @@ -58,8 +61,11 @@ function checkRstCode(rstMethod, expectRstCode) { req.on('aborted', common.mustCall()); req.on('end', common.mustCall()); - if (expectRstCode > NGHTTP2_NO_ERROR) { + if (expectRstCode !== NGHTTP2_NO_ERROR && + expectRstCode !== NGHTTP2_CANCEL) { req.on('error', common.mustCall(errCheck)); + } else { + req.on('error', common.mustNotCall()); } })); diff --git a/test/parallel/test-http2-stream-destroy-event-order.js b/test/parallel/test-http2-stream-destroy-event-order.js new file mode 100644 index 0000000000..16bb5e6a62 --- /dev/null +++ b/test/parallel/test-http2-stream-destroy-event-order.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +let client; +let req; +const server = http2.createServer(); +server.on('stream', common.mustCall((stream) => { + stream.on('error', common.mustCall(() => { + stream.on('streamClosed', common.mustCall((code) => { + assert.strictEqual(code, 2); + client.destroy(); + server.close(); + })); + })); + + req.rstStream(2); +})); +server.listen(0, common.mustCall(() => { + client = http2.connect(`http://localhost:${server.address().port}`); + req = client.request(); + req.resume(); + req.on('error', common.mustCall()); +})); |