diff --git a/benchmark/http2/headers.js b/benchmark/http2/headers.js index ab890dbbf2573f..fe4f251e4d2ab9 100644 --- a/benchmark/http2/headers.js +++ b/benchmark/http2/headers.js @@ -45,7 +45,7 @@ function main({ n, nheaders }) { } else { bench.end(n); server.close(); - client.destroy(); + client.close(); } }); } diff --git a/deps/nghttp2/lib/nghttp2_session.c b/deps/nghttp2/lib/nghttp2_session.c index 54746fb37b376d..67d8b08d36844f 100644 --- a/deps/nghttp2/lib/nghttp2_session.c +++ b/deps/nghttp2/lib/nghttp2_session.c @@ -1485,6 +1485,11 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, DEBUGF("stream: stream(%p)=%d close\n", stream, stream->stream_id); + if (error_code == NGHTTP2_NO_ERROR && + nghttp2_http_on_remote_end_stream(stream) == -1) { + error_code = NGHTTP2_PROTOCOL_ERROR; + } + /* We call on_stream_close_callback even if stream->state is NGHTTP2_STREAM_INITIAL. This will happen while sending request HEADERS, a local endpoint receives RST_STREAM for that stream. It @@ -2516,9 +2521,6 @@ static int session_prep_frame(nghttp2_session *session, return 0; } case NGHTTP2_RST_STREAM: - if (session_is_closing(session)) { - return NGHTTP2_ERR_SESSION_CLOSING; - } nghttp2_frame_pack_rst_stream(&session->aob.framebufs, &frame->rst_stream); return 0; case NGHTTP2_SETTINGS: { @@ -4181,6 +4183,11 @@ static int session_after_header_block_received(nghttp2_session *session) { return 0; } + /* on_frame might free the stream */ + if (!nghttp2_session_get_stream(session, frame->hd.stream_id)) { + return 0; + } + return session_end_stream_headers_received(session, frame, stream); } diff --git a/deps/nghttp2/patches/0000-content-length.patch b/deps/nghttp2/patches/0000-content-length.patch new file mode 100644 index 00000000000000..c898499e37d6ca --- /dev/null +++ b/deps/nghttp2/patches/0000-content-length.patch @@ -0,0 +1,29 @@ +From 92c120c0ba2ed5806960f8ec9a338fefc17b0427 Mon Sep 17 00:00:00 2001 +From: Gil Pedersen +Date: Tue, 15 Sep 2020 12:36:34 +0000 +Subject: [PATCH] Check content-length before calling stream_close_callback + with error code 0 + +--- + deps/nghttp2/lib/nghttp2_session.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/deps/nghttp2/lib/nghttp2_session.c b/deps/nghttp2/lib/nghttp2_session.c +index 004a4dff..008f9d34 100644 +--- a/deps/nghttp2/lib/nghttp2_session.c ++++ b/deps/nghttp2/lib/nghttp2_session.c +@@ -1485,6 +1485,11 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, + + DEBUGF("stream: stream(%p)=%d close\n", stream, stream->stream_id); + ++ if (error_code == NGHTTP2_NO_ERROR && ++ nghttp2_http_on_remote_end_stream(stream) == -1) { ++ error_code = NGHTTP2_PROTOCOL_ERROR; ++ } ++ + /* We call on_stream_close_callback even if stream->state is + NGHTTP2_STREAM_INITIAL. This will happen while sending request + HEADERS, a local endpoint receives RST_STREAM for that stream. It +-- +2.40.1 + diff --git a/deps/nghttp2/patches/0001-check-stream-free-after-header-received.patch b/deps/nghttp2/patches/0001-check-stream-free-after-header-received.patch new file mode 100644 index 00000000000000..737a17c03e2684 --- /dev/null +++ b/deps/nghttp2/patches/0001-check-stream-free-after-header-received.patch @@ -0,0 +1,29 @@ +From 9581880841f5afa00075eb6089389c03015334ce Mon Sep 17 00:00:00 2001 +From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> +Date: Fri, 4 Oct 2024 08:08:15 +0200 +Subject: [PATCH] Check if session_call_on_frame_received frees the stream in + session_after_header_block_received + +--- + deps/nghttp2/lib/nghttp2_session.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/deps/nghttp2/lib/nghttp2_session.c b/deps/nghttp2/lib/nghttp2_session.c +index 08efaf0f8e..8b818c30a2 100644 +--- a/deps/nghttp2/lib/nghttp2_session.c ++++ b/deps/nghttp2/lib/nghttp2_session.c +@@ -4186,6 +4186,11 @@ static int session_after_header_block_received(nghttp2_session *session) { + return 0; + } + ++ /* on_frame might free the stream */ ++ if (!nghttp2_session_get_stream(session, frame->hd.stream_id)) { ++ return 0; ++ } ++ + return session_end_stream_headers_received(session, frame, stream); + } + +-- +2.46.2 + diff --git a/deps/nghttp2/patches/0002-rst-before-goaway.patch b/deps/nghttp2/patches/0002-rst-before-goaway.patch new file mode 100644 index 00000000000000..e4c2789899456e --- /dev/null +++ b/deps/nghttp2/patches/0002-rst-before-goaway.patch @@ -0,0 +1,26 @@ +From a990ac8598241f939ca41bf7a7c53c40094663bf Mon Sep 17 00:00:00 2001 +From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> +Date: Fri, 4 Oct 2024 11:52:30 +0200 +Subject: [PATCH] Allow RST_STREAM before GOAWAY + +--- + deps/nghttp2/lib/nghttp2_session.c | 3 --- + 1 file changed, 3 deletions(-) + +diff --git a/deps/nghttp2/lib/nghttp2_session.c b/deps/nghttp2/lib/nghttp2_session.c +index 8b818c30a2..67d8b08d36 100644 +--- a/deps/nghttp2/lib/nghttp2_session.c ++++ b/deps/nghttp2/lib/nghttp2_session.c +@@ -2521,9 +2521,6 @@ static int session_prep_frame(nghttp2_session *session, + return 0; + } + case NGHTTP2_RST_STREAM: +- if (session_is_closing(session)) { +- return NGHTTP2_ERR_SESSION_CLOSING; +- } + nghttp2_frame_pack_rst_stream(&session->aob.framebufs, &frame->rst_stream); + return 0; + case NGHTTP2_SETTINGS: { +-- +2.46.2 + diff --git a/doc/api/http2.md b/doc/api/http2.md index 0bb059fc81d33b..eec71acbab3690 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -507,7 +507,7 @@ added: v8.4.0 due to an error. * `code` {number} The HTTP/2 error code to send in the final `GOAWAY` frame. If unspecified, and `error` is not undefined, the default is `INTERNAL_ERROR`, - otherwise defaults to `NO_ERROR`. + otherwise defaults to `CANCEL`. Immediately terminates the `Http2Session` and the associated `net.Socket` or `tls.TLSSocket`. diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 6818515d6ab684..b910c2348352cd 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -810,6 +810,10 @@ TLSSocket.prototype._init = function(socket, wrap) { const ssl = this._handle; this.server = options.server; + // Required for Socket.prototype._destroy check + // to emit 'close' server event + this._server = options.server; + debug('%s _init', options.isServer ? 'server' : 'client', 'handle?', !!ssl, diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 923db9ad1b5b1e..5326b2d5627a31 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -264,6 +264,7 @@ const { const { NGHTTP2_CANCEL, + NGHTTP2_CONNECT_ERROR, NGHTTP2_REFUSED_STREAM, NGHTTP2_DEFAULT_WEIGHT, NGHTTP2_FLAG_END_STREAM, @@ -546,8 +547,10 @@ function onStreamClose(code) { closeStream(stream, code, kNoRstStream); stream[kState].fd = -1; - // Defer destroy we actually emit end. - if (!stream.readable || code !== NGHTTP2_NO_ERROR) { + if (code !== NGHTTP2_NO_ERROR) { + const error = new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code); + stream.destroy(error); + } else if (!stream.readable) { // If errored or ended, we can destroy immediately. stream.destroy(); } else { @@ -607,11 +610,18 @@ function onFrameError(id, type, code) { return; debugSessionObj(session, 'error sending frame type %d on stream %d, code: %d', type, id, code); - const emitter = session[kState].streams.get(id) || session; + const stream = session[kState].streams.get(id); + const emitter = stream || session; emitter[kUpdateTimer](); - emitter.emit('frameError', type, code, id); - session[kState].streams.get(id).close(code); - session.close(); + + if (stream) { + stream.emit('frameError', type, code, id); + stream[kState].rstCode = code; + stream.destroy(new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code)); + } else { + session[kState].destroyCode = code; + session.destroy(new ERR_HTTP2_SESSION_ERROR(nameForErrorCode[code] || code)); + } } function onAltSvc(stream, origin, alt) { @@ -678,14 +688,16 @@ function onGoawayData(code, lastStreamID, buf) { // If this is a no error goaway, begin shutting down. // No new streams permitted, but existing streams may // close naturally on their own. - session.close(); + session[kState].flags |= SESSION_FLAGS_CLOSED; + session[kMaybeDestroy](); } else { - // However, if the code is not NGHTTP_NO_ERROR, destroy the - // session immediately. We destroy with an error but send a - // goaway using NGHTTP2_NO_ERROR because there was no error - // condition on this side of the session that caused the - // shutdown. - session.destroy(new ERR_HTTP2_SESSION_ERROR(code), NGHTTP2_NO_ERROR); + // Must not send any further data. + closeSession( + session, + code, + new ERR_HTTP2_SESSION_ERROR(nameForErrorCode[code] || code), + true, + ); } } @@ -848,6 +860,7 @@ function submitGoaway(code, lastStreamID, opaqueData) { if (this.destroyed) return; debugSessionObj(this, 'submitting goaway'); + this[kState].destroyCode = code; this[kUpdateTimer](); this[kHandle].goaway(code, lastStreamID, opaqueData); } @@ -1126,29 +1139,18 @@ function finishSessionClose(session, error) { socket.on('close', () => { emitClose(session, error); }); - if (session.closed) { - // If we're gracefully closing the socket, call resume() so we can detect - // the peer closing in case binding.Http2Session is already gone. - socket.resume(); - } // Always wait for writable side to finish. socket.end((err) => { debugSessionObj(session, 'finishSessionClose socket end', err, error); - // If session.destroy() was called, destroy the underlying socket. Delay - // it a bit to try to avoid ECONNRESET on Windows. - if (!session.closed) { - setImmediate(() => { - socket.destroy(error); - }); - } + socket.resetAndDestroy(error); }); } else { process.nextTick(emitClose, session, error); } } -function closeSession(session, code, error) { +function closeSession(session, code, error, closed) { debugSessionObj(session, 'start closing/destroying', error); const state = session[kState]; @@ -1173,7 +1175,7 @@ function closeSession(session, code, error) { // Destroy the handle if it exists at this point. if (handle !== undefined) { handle.ondone = finishSessionClose.bind(null, session, error); - handle.destroy(code, socket.destroyed); + handle.destroy(code, closed ?? socket.destroyed); } else { finishSessionClose(session, error); } @@ -1236,7 +1238,7 @@ class Http2Session extends EventEmitter { socket.on('close', socketOnClose); this[kState] = { - destroyCode: NGHTTP2_NO_ERROR, + destroyCode: null, flags: SESSION_FLAGS_PENDING, goawayCode: null, goawayLastStreamID: null, @@ -1436,7 +1438,7 @@ class Http2Session extends EventEmitter { // If a GOAWAY frame has been received, gives the error code specified get goawayCode() { - return this[kState].goawayCode || NGHTTP2_NO_ERROR; + return this[kState].goawayCode; } // If a GOAWAY frame has been received, gives the last stream ID reported @@ -1511,6 +1513,9 @@ class Http2Session extends EventEmitter { // is only a notification, and does not affect the usable state of the // session with the notable exception that new incoming streams will // be rejected automatically. + // FIXME(szmarczak): goaway without connection close + // for error codes other than NO_ERROR is against the spec. + // See https://datatracker.ietf.org/doc/html/rfc9113#section-5.4.1 goaway(code = NGHTTP2_NO_ERROR, lastStreamID = 0, opaqueData) { if (this.destroyed) throw new ERR_HTTP2_INVALID_SESSION(); @@ -1541,7 +1546,8 @@ class Http2Session extends EventEmitter { code = error; error = code !== NGHTTP2_NO_ERROR ? - new ERR_HTTP2_SESSION_ERROR(code) : undefined; + new ERR_HTTP2_SESSION_ERROR(nameForErrorCode[code] || code) : + undefined; } if (code === undefined && error != null) code = NGHTTP2_INTERNAL_ERROR; @@ -1566,11 +1572,16 @@ class Http2Session extends EventEmitter { if (this.closed || this.destroyed) return; debugSessionObj(this, 'marking session closed'); - this[kState].flags |= SESSION_FLAGS_CLOSED; + const state = this[kState]; + state.flags |= SESSION_FLAGS_CLOSED; if (typeof callback === 'function') this.once('close', callback); - this.goaway(); - this[kMaybeDestroy](); + + if (state.streams.size > 0 || state.pendingStreams.size > 0) { + this.goaway(); + } else { + this.destroy(); + } } [EventEmitter.captureRejectionSymbol](err, event, ...args) { @@ -1861,7 +1872,11 @@ function afterShutdown(status) { const stream = this.handle[kOwner]; if (stream) { stream.on('finish', () => { - stream[kMaybeDestroy](); + // We don't want to interrupt in the middle + // where the stream is closing with an error code. + setImmediate(() => { + stream[kMaybeDestroy](); + }); }); } // Currently this status value is unused @@ -1981,7 +1996,7 @@ class Http2Stream extends Duplex { this[kState] = { didRead: false, flags: STREAM_FLAGS_PENDING, - rstCode: NGHTTP2_NO_ERROR, + rstCode: undefined, writeQueueSize: 0, trailersReady: false, endAfterHeaders: false, @@ -2304,38 +2319,49 @@ class Http2Stream extends Duplex { // This will cause the internal resources to be released. // * Then cleans up the resources on the js side _destroy(err, callback) { + const id = this[kID]; const session = this[kSession]; const handle = this[kHandle]; - const id = this[kID]; + const hasHandle = handle !== undefined; + const sessionState = session[kState]; + const isServerSession = session[kType] === NGHTTP2_SESSION_SERVER; - debugStream(this[kID] || 'pending', session[kType], 'destroying stream'); + debugStream(id || 'pending', session[kType], 'destroying stream'); - const state = this[kState]; - const sessionState = session[kState]; - const sessionCode = sessionState.goawayCode || sessionState.destroyCode; - - // If a stream has already closed successfully, there is no error - // to report from this stream, even if the session has errored. - // This can happen if the stream was already in process of destroying - // after a successful close, but the session had a error between - // this stream's close and destroy operations. - // Previously, this always overrode a successful close operation code - // NGHTTP2_NO_ERROR (0) with sessionCode because the use of the || operator. - let code = this.closed ? this.rstCode : sessionCode; - if (err != null) { - if (sessionCode) { - code = sessionCode; - } else if (err instanceof AbortError) { - // Enables using AbortController to cancel requests with RST code 8. - code = NGHTTP2_CANCEL; - } else { - code = NGHTTP2_INTERNAL_ERROR; + if (!this.closed) { + let goawayCode = sessionState.goawayCode; + if (goawayCode === NGHTTP2_NO_ERROR) { + // Received NO_ERROR (GOAWAY) from remote peer yet still got reset. + // This needs to be null because the other side is within its rights + // to TCP RST after we send non-zero GOAWAY. + goawayCode = null; + } + + let destroyCode = sessionState.destroyCode; + if (destroyCode === NGHTTP2_NO_ERROR) { + // Code to send to remote peer + destroyCode = isServerSession ? NGHTTP2_INTERNAL_ERROR : NGHTTP2_CANCEL; + } + + const sessionCode = goawayCode ?? destroyCode; + let code = this.rstCode ?? sessionCode; + + if (err != null && code === null) { + code = err instanceof AbortError ? NGHTTP2_CANCEL : NGHTTP2_INTERNAL_ERROR; } + + const connect = this[kSentHeaders]?.[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT; + if (code === null && connect) { + code = NGHTTP2_CONNECT_ERROR; + } + + const defaultCode = isServerSession ? NGHTTP2_INTERNAL_ERROR : NGHTTP2_CANCEL; + code ??= defaultCode; + + const rstStreamStatus = hasHandle ? kForceRstStream : kNoRstStream; + closeStream(this, code, rstStreamStatus); } - const hasHandle = handle !== undefined; - if (!this.closed) - closeStream(this, code, hasHandle ? kForceRstStream : kNoRstStream); this.push(null); if (hasHandle) { @@ -2346,14 +2372,8 @@ class Http2Stream extends Duplex { } // Adjust the write queue size for accounting - sessionState.writeQueueSize -= state.writeQueueSize; - state.writeQueueSize = 0; - - // 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 == null && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL) - err = new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code); + sessionState.writeQueueSize -= this[kState].writeQueueSize; + this[kState].writeQueueSize = 0; this[kSession] = undefined; this[kHandle] = undefined; @@ -2995,11 +3015,12 @@ ObjectDefineProperty(Http2Session.prototype, 'setTimeout', setTimeoutValue); function socketOnError(error) { const session = this[kBoundSession]; if (session !== undefined) { - // We can ignore ECONNRESET after GOAWAY was received as there's nothing - // we can do and the other side is fully within its rights to do so. - if (error.code === 'ECONNRESET' && session[kState].goawayCode !== null) - return session.destroy(); - debugSessionObj(this, 'socket error [%s]', error.message); + if (error.code === 'ECONNRESET') { + // Handled by socketOnClose, it does not matter if the endpoint + // sends TCP FIN prematurely or TCP RST. + return; + } + debugSessionObj(session, 'socket error [%s]', error.message); session.destroy(error); } } @@ -3279,9 +3300,28 @@ function socketOnClose() { debugSessionObj(session, 'socket closed'); const err = session.connecting ? new ERR_SOCKET_CLOSED() : null; const state = session[kState]; - state.streams.forEach((stream) => stream.close(NGHTTP2_CANCEL)); - state.pendingStreams.forEach((stream) => stream.close(NGHTTP2_CANCEL)); - session.close(); + + // Code for premature close + let code = state.goawayCode ?? state.destroyCode; + if (code === NGHTTP2_NO_ERROR) { + // Got NO_ERROR but still closed prematurely + code = null; + } + + const defaultCode = NGHTTP2_INTERNAL_ERROR; + + const closeStream = (stream) => { + if (code === null) { + const connect = stream[kSentHeaders]?.[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT; + stream.close(connect ? NGHTTP2_CONNECT_ERROR : defaultCode); + } else { + stream.close(code ?? defaultCode); + } + }; + + state.streams.forEach(closeStream); + state.pendingStreams.forEach(closeStream); + session[kState].flags |= SESSION_FLAGS_CLOSED; session[kMaybeDestroy](err); } } diff --git a/lib/net.js b/lib/net.js index 604ba648754a2a..0c255c4945e1ac 100644 --- a/lib/net.js +++ b/lib/net.js @@ -101,7 +101,6 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_FD_TYPE, - ERR_INVALID_HANDLE_TYPE, ERR_INVALID_IP_ADDRESS, ERR_MISSING_ARGS, ERR_SERVER_ALREADY_LISTEN, @@ -730,15 +729,13 @@ Socket.prototype.end = function(data, encoding, callback) { return this; }; -Socket.prototype.resetAndDestroy = function() { +Socket.prototype.resetAndDestroy = function(error) { if (this._handle) { - if (!(this._handle instanceof TCP)) - throw new ERR_INVALID_HANDLE_TYPE(); if (this.connecting) { debug('reset wait for connection'); - this.once('connect', () => this._reset()); + this.once('connect', () => this._reset(error)); } else { - this._reset(); + this._reset(error); } } else { this.destroy(new ERR_SOCKET_CLOSED()); @@ -814,8 +811,7 @@ Socket.prototype._destroy = function(exception, cb) { this[kBytesRead] = this._handle.bytesRead; this[kBytesWritten] = this._handle.bytesWritten; - if (this.resetAndClosing) { - this.resetAndClosing = false; + if (this.resetAndClosing && this._handle instanceof TCP) { const err = this._handle.reset(() => { debug('emit close'); this.emit('close', isException); @@ -843,17 +839,29 @@ Socket.prototype._destroy = function(exception, cb) { if (this._server) { debug('has server'); - this._server._connections--; - if (this._server._emitCloseIfDrained) { - this._server._emitCloseIfDrained(); + + // This _destroy is called recursively on different parents + // till it closes all handles. + if (!this._parent) { + this._server._connections--; + + if (this._server._emitCloseIfDrained) { + this._server._emitCloseIfDrained(); + } } } }; -Socket.prototype._reset = function() { +Socket.prototype._reset = function(error) { debug('reset connection'); - this.resetAndClosing = true; - return this.destroy(); + + if (this._parentWrap) { + this._parentWrap.resetAndClosing = true; + } else { + this.resetAndClosing = true; + } + + return this.destroy(error); }; Socket.prototype._getpeername = function() { diff --git a/src/node_http2.cc b/src/node_http2.cc index f4f85b37781817..7f5b534ee662b8 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1720,7 +1720,19 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { stream_->ReadStart(); } - if (is_destroyed()) { + // If there is more incoming data queued up, consume it. + if (stream_buf_offset_ > 0) { + ConsumeHTTP2Data(); + } + + // This needs to be before is_destroyed(), + // so queued GOAWAY gets sent. + if (!is_write_scheduled()) { + // Schedule a new write if nghttp2 wants to send data. + MaybeScheduleWrite(); + } + + if (!is_write_scheduled() && is_destroyed()) { HandleScope scope(env()->isolate()); MakeCallback(env()->ondone_string(), 0, nullptr); if (stream_ != nullptr) { @@ -1730,16 +1742,6 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { } return; } - - // If there is more incoming data queued up, consume it. - if (stream_buf_offset_ > 0) { - ConsumeHTTP2Data(); - } - - if (!is_write_scheduled() && !is_destroyed()) { - // Schedule a new write if nghttp2 wants to send data. - MaybeScheduleWrite(); - } } // If the underlying nghttp2_session struct has data pending in its outbound @@ -1856,11 +1858,10 @@ void Http2Session::CopyDataIntoOutgoing(const uint8_t* src, size_t src_length) { // Returns non-zero value if a write is already in progress. uint8_t Http2Session::SendPendingData() { Debug(this, "sending pending data"); - // Do not attempt to send data on the socket if the destroying flag has - // been set. That means everything is shutting down and the socket - // will not be usable. - if (is_destroyed()) - return 0; + + // Must not early return if is_destroyed(), + // otherwise queued GOAWAY gets dropped! + set_write_scheduled(false); // SendPendingData should not be called recursively. @@ -2656,6 +2657,11 @@ ssize_t Http2Stream::Provider::Stream::OnRead(nghttp2_session* handle, if (stream->available_outbound_length_ == 0 && !stream->is_writable()) { Debug(session, "no more data for stream %d", id); *flags |= NGHTTP2_DATA_FLAG_EOF; + + if (session->has_pending_rststream(id)) { + *flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM; + } + if (stream->has_trailers()) { *flags |= NGHTTP2_DATA_FLAG_NO_END_STREAM; stream->OnTrailers(); diff --git a/test/parallel/test-h2-large-header-cause-client-to-hangup.js b/test/parallel/test-h2-large-header-cause-client-to-hangup.js index eb18d9e9a20ae7..d74de85bd54890 100644 --- a/test/parallel/test-h2-large-header-cause-client-to-hangup.js +++ b/test/parallel/test-h2-large-header-cause-client-to-hangup.js @@ -30,6 +30,7 @@ server.listen(0, common.mustCall(() => { stream.on('close', common.mustCall(() => { assert.strictEqual(stream.rstCode, NGHTTP2_FRAME_SIZE_ERROR); clearTimeout(timer); + clientSession.close(); server.close(); })); diff --git a/test/parallel/test-http2-cancel-while-client-reading.js b/test/parallel/test-http2-cancel-while-client-reading.js index 189e128e6667ef..f62d768396a126 100644 --- a/test/parallel/test-http2-cancel-while-client-reading.js +++ b/test/parallel/test-http2-cancel-while-client-reading.js @@ -28,6 +28,11 @@ server.listen(0, function() { { rejectUnauthorized: false } ); client_stream = client.request({ ':method': 'POST' }); + client_stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); client_stream.on('close', common.mustCall(() => { client.close(); server.close(); diff --git a/test/parallel/test-http2-client-connection-tunnel-reset.js b/test/parallel/test-http2-client-connection-tunnel-reset.js new file mode 100644 index 00000000000000..6e19e3760aed05 --- /dev/null +++ b/test/parallel/test-http2-client-connection-tunnel-reset.js @@ -0,0 +1,33 @@ +// Flags: --expose-internals + +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const h2 = require('http2'); +const { kSocket } = require('internal/http2/util'); + +const h2Server = h2.createServer(() => {}); + +h2Server.on('connect', (req, res) => { + res.writeHead(200, {}); + + res.stream.session[kSocket].resetAndDestroy(); + + h2Server.close(); +}); + +h2Server.listen(0, common.mustCall(() => { + const proxyClient = h2.connect(`http://localhost:${h2Server.address().port}`); + proxyClient.once('error', common.mustNotCall()); + const proxyReq = proxyClient.request({ + ':method': 'CONNECT', + ':authority': 'example.com:443' + }); + proxyReq.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CONNECT_ERROR' + })); +})); diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js index d7609fc33391ac..89d123d0f6df7c 100644 --- a/test/parallel/test-http2-client-destroy.js +++ b/test/parallel/test-http2-client-destroy.js @@ -102,7 +102,7 @@ const { getEventListeners } = require('events'); })); } -// Test destroy before goaway +// Test server session destroy { const server = h2.createServer(); server.on('stream', common.mustCall((stream) => { @@ -114,15 +114,43 @@ const { getEventListeners } = require('events'); client.on('close', () => { server.close(); - // Calling destroy in here should not matter - client.destroy(); + }); + + const req = client.request(); + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + })); +} + +// Test client session destroy +{ + let client; + const server = h2.createServer(); + server.on('stream', common.mustCall((stream) => { + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); + stream.once('aborted', common.mustCall()); + client.destroy(); + })); + + server.listen(0, common.mustCall(() => { + client = h2.connect(`http://localhost:${server.address().port}`); + + client.on('close', () => { + server.close(); }); client.request(); })); } -// Test destroy before connect +// Test destroy before connect (endStream) { const server = h2.createServer(); server.on('stream', common.mustNotCall()); @@ -140,6 +168,24 @@ const { getEventListeners } = require('events'); })); } +// Test destroy before connect (no endStream) +{ + const server = h2.createServer(); + server.on('stream', common.mustNotCall()); + + server.listen(0, common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); + + server.on('connection', common.mustCall(() => { + server.close(); + client.close(); + })); + + const req = client.request({}, { endStream: false }); + req.destroy(); + })); +} + // Test close before connect { const server = h2.createServer(); @@ -292,7 +338,11 @@ const { getEventListeners } = require('events'); const controller = new AbortController(); server.on('stream', common.mustCall((stream) => { - stream.on('error', common.mustNotCall()); + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); stream.on('close', common.mustCall(() => { assert.strictEqual(stream.rstCode, h2.constants.NGHTTP2_CANCEL); server.close(); diff --git a/test/parallel/test-http2-client-jsstream-destroy.js b/test/parallel/test-http2-client-jsstream-destroy.js index 7e44241e985a64..ebca22737ce953 100644 --- a/test/parallel/test-http2-client-jsstream-destroy.js +++ b/test/parallel/test-http2-client-jsstream-destroy.js @@ -50,6 +50,12 @@ server.listen(0, common.mustCall(function() { socket.destroy(); }); + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + req.on('close', common.mustCall(() => { client.close(); server.close(); diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js index bc0cb5ff619dc0..1ff56cf71ff440 100644 --- a/test/parallel/test-http2-client-rststream-before-connect.js +++ b/test/parallel/test-http2-client-rststream-before-connect.js @@ -8,6 +8,11 @@ const h2 = require('http2'); const server = h2.createServer(); server.on('stream', (stream) => { + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' + })); stream.on('close', common.mustCall()); stream.respond(); stream.end('ok'); diff --git a/test/parallel/test-http2-client-socket-destroy.js b/test/parallel/test-http2-client-socket-destroy.js index 2cc6ef1e4ea4a8..52cccad787e563 100644 --- a/test/parallel/test-http2-client-socket-destroy.js +++ b/test/parallel/test-http2-client-socket-destroy.js @@ -32,8 +32,13 @@ server.listen(0, common.mustCall(function() { })); req.resume(); - req.on('end', common.mustCall()); + req.on('end', common.mustNotCall()); req.on('close', common.mustCall(() => server.close())); + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); // On the client, the close event must call client.on('close', common.mustCall()); diff --git a/test/parallel/test-http2-close-while-writing.js b/test/parallel/test-http2-close-while-writing.js index d8537c31b00eb6..42a4b365e4f88b 100644 --- a/test/parallel/test-http2-close-while-writing.js +++ b/test/parallel/test-http2-close-while-writing.js @@ -23,6 +23,11 @@ let client_stream; server.on('session', common.mustCall(function(session) { session.on('stream', common.mustCall(function(stream) { + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); stream.resume(); stream.on('data', function() { this.write(Buffer.alloc(1)); diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index 1154b69df41548..5d5f78e49b40b7 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -44,8 +44,12 @@ server.listen(0, common.mustCall(() => { { const req = client.request(); req.on('response', common.mustNotCall()); - req.on('error', common.mustNotCall()); - req.on('end', common.mustCall()); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + req.on('end', common.mustNotCall()); req.on('close', common.mustCall(() => countdown.dec())); req.resume(); } diff --git a/test/parallel/test-http2-compat-serverresponse-end-after-statuses-without-body.js b/test/parallel/test-http2-compat-serverresponse-end-after-statuses-without-body.js index ce8cbe600c8bd8..7279273605fc45 100644 --- a/test/parallel/test-http2-compat-serverresponse-end-after-statuses-without-body.js +++ b/test/parallel/test-http2-compat-serverresponse-end-after-statuses-without-body.js @@ -33,7 +33,7 @@ server.listen(0, common.mustCall(function() { let responseCount = 0; const closeAfterResponse = () => { if (STATUS_CODES_COUNT === ++responseCount) { - client.destroy(); + client.close(); server.close(); } }; diff --git a/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js index fc97a70f42d956..20d065b9485465 100644 --- a/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js @@ -38,9 +38,15 @@ server.listen(0, common.mustCall(function() { ':authority': `localhost:${port}` }; const request = client.request(headers); - request.on('end', common.mustCall(function() { + request.once('error', common.mustCall(function(error) { + common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })(error); client.close(); })); + request.on('end', common.mustNotCall()); request.end(); request.resume(); })); diff --git a/test/parallel/test-http2-compat-serverresponse-write.js b/test/parallel/test-http2-compat-serverresponse-write.js index 64b37e8a132e1a..815ad714151b26 100644 --- a/test/parallel/test-http2-compat-serverresponse-write.js +++ b/test/parallel/test-http2-compat-serverresponse-write.js @@ -3,6 +3,7 @@ const { mustCall, mustNotCall, + expectsError, hasCrypto, skip } = require('../common'); @@ -38,7 +39,7 @@ const assert = require('assert'); // response.write() with cb returns falsy value assert(!response.write('muahaha', mustCall())); - client.destroy(); + client.close(); server.close(); })); })); @@ -65,7 +66,7 @@ const assert = require('assert'); response.end(); response.write('asd', mustCall((err) => { assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); - client.destroy(); + client.close(); server.close(); })); })); @@ -78,13 +79,17 @@ const assert = require('assert'); const port = server.address().port; const url = `http://localhost:${port}`; const client = connect(url, mustCall(() => { - client.request(); + client.request().once('error', expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); })); server.once('request', mustCall((request, response) => { response.destroy(); assert.strictEqual(response.write('asd', mustNotCall()), false); - client.destroy(); + client.close(); server.close(); })); })); diff --git a/test/parallel/test-http2-compat-socket-destroy-delayed.js b/test/parallel/test-http2-compat-socket-destroy-delayed.js index 62405047d8266e..6c8174c840fe52 100644 --- a/test/parallel/test-http2-compat-socket-destroy-delayed.js +++ b/test/parallel/test-http2-compat-socket-destroy-delayed.js @@ -32,7 +32,13 @@ app.listen(0, mustCall(() => { request.once('response', mustCall((headers, flags) => { let data = ''; request.on('data', (chunk) => { data += chunk; }); - request.on('end', mustCall(() => { + request.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + request.on('end', common.mustNotCall()); + request.on('close', mustCall(() => { assert.strictEqual(data, 'hello'); session.close(); app.close(); diff --git a/test/parallel/test-http2-compat-socket-set.js b/test/parallel/test-http2-compat-socket-set.js index e8b804858d65bd..34edc4c5459e83 100644 --- a/test/parallel/test-http2-compat-socket-set.js +++ b/test/parallel/test-http2-compat-socket-set.js @@ -87,7 +87,9 @@ server.listen(0, common.mustCall(function() { ':authority': `localhost:${port}` }; const request = client.request(headers); - request.on('end', common.mustCall(() => { + request.on('error', common.mustCall()); + request.on('end', common.mustNotCall()); + request.on('close', common.mustCall(() => { client.close(); server.close(); })); diff --git a/test/parallel/test-http2-compat-socket.js b/test/parallel/test-http2-compat-socket.js index 95bc42180ef8e9..1246f4347691a0 100644 --- a/test/parallel/test-http2-compat-socket.js +++ b/test/parallel/test-http2-compat-socket.js @@ -84,9 +84,10 @@ server.listen(0, common.mustCall(function() { ':authority': `localhost:${port}` }; const request = client.request(headers); - request.on('end', common.mustCall(() => { + request.once('error', common.mustCall(() => { client.close(); })); + request.on('end', common.mustNotCall()); request.end(); request.resume(); })); diff --git a/test/parallel/test-http2-compat-write-head-destroyed.js b/test/parallel/test-http2-compat-write-head-destroyed.js index 842bf0e9abffb0..40ae686a36fd18 100644 --- a/test/parallel/test-http2-compat-write-head-destroyed.js +++ b/test/parallel/test-http2-compat-write-head-destroyed.js @@ -20,6 +20,11 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request(); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.on('response', common.mustNotCall()); req.on('close', common.mustCall((arg) => { client.close(); diff --git a/test/parallel/test-http2-destroy-after-write.js b/test/parallel/test-http2-destroy-after-write.js index 399df015b8879e..d1b6b57da86e41 100644 --- a/test/parallel/test-http2-destroy-after-write.js +++ b/test/parallel/test-http2-destroy-after-write.js @@ -25,6 +25,11 @@ server.on('session', common.mustCall(function(session) { server.listen(0, function() { const client = http2.connect(`http://localhost:${server.address().port}`); const stream = client.request({ ':method': 'POST' }); + stream.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); stream.on('response', common.mustCall(function(headers) { assert.strictEqual(headers[':status'], 200); })); diff --git a/test/parallel/test-http2-exceeds-server-trailer-size.js b/test/parallel/test-http2-exceeds-server-trailer-size.js index ae2bbc1dca08bf..fae0c25096d8dc 100644 --- a/test/parallel/test-http2-exceeds-server-trailer-size.js +++ b/test/parallel/test-http2-exceeds-server-trailer-size.js @@ -42,7 +42,9 @@ server.listen(0, () => { const clientStream = clientSession.request(); - clientStream.on('close', common.mustCall()); + clientStream.on('close', () => { + clientSession.close(); + }); clientStream.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', name: 'Error', diff --git a/test/parallel/test-http2-large-write-destroy.js b/test/parallel/test-http2-large-write-destroy.js index b59c66bb04755b..6a0bfdf8f49efe 100644 --- a/test/parallel/test-http2-large-write-destroy.js +++ b/test/parallel/test-http2-large-write-destroy.js @@ -31,6 +31,11 @@ server.listen(0, common.mustCall(() => { { rejectUnauthorized: false }); const req = client.request({ ':path': '/' }); + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.end(); req.resume(); // Otherwise close won't be emitted if there's pending data. diff --git a/test/parallel/test-http2-multistream-destroy-on-read-tls.js b/test/parallel/test-http2-multistream-destroy-on-read-tls.js index 91cbec6b2dc975..c7ce1b19c96dab 100644 --- a/test/parallel/test-http2-multistream-destroy-on-read-tls.js +++ b/test/parallel/test-http2-multistream-destroy-on-read-tls.js @@ -17,6 +17,12 @@ const server = http2.createSecureServer({ const filenames = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j']; server.on('stream', common.mustCall((stream) => { + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); + function write() { stream.write('a'.repeat(10240)); stream.once('drain', write); diff --git a/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js b/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js index df3aefff11e7ad..a2cfdafded2cf6 100644 --- a/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js +++ b/test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js @@ -22,12 +22,6 @@ server.listen(0, common.mustCall(() => { const client = h2.connect(`http://localhost:${server.address().port}`, options); - client.on('error', common.expectsError({ - code: 'ERR_HTTP2_SESSION_ERROR', - name: 'Error', - message: 'Session closed with error code 9' - })); - const req = client.request({ // Greater than 65536 bytes 'test-header': 'A'.repeat(90000) @@ -40,9 +34,9 @@ server.listen(0, common.mustCall(() => { })); req.on('error', common.expectsError({ - code: 'ERR_HTTP2_SESSION_ERROR', + code: 'ERR_HTTP2_STREAM_ERROR', name: 'Error', - message: 'Session closed with error code 9' + message: 'Stream closed with error code NGHTTP2_COMPRESSION_ERROR' })); req.end(); })); @@ -78,7 +72,7 @@ server.listen(0, common.mustCall(() => { common.mustCall((err, session) => { assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); assert.strictEqual(err.name, 'Error'); - assert.strictEqual(err.message, 'Session closed with error code 9'); + assert.strictEqual(err.message, 'Session closed with error code NGHTTP2_COMPRESSION_ERROR'); assert.strictEqual(session instanceof ServerHttp2Session, true); server.close(); }), @@ -92,7 +86,11 @@ server.listen(0, common.mustCall(() => { const req = client.request(); req.on('response', common.mustNotCall()); - req.on('error', common.mustNotCall()); + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_COMPRESSION_ERROR' + })); req.end(); }), ); diff --git a/test/parallel/test-http2-options-max-reserved-streams.js b/test/parallel/test-http2-options-max-reserved-streams.js index 993bb1b7b3dfed..71106d54a6b633 100644 --- a/test/parallel/test-http2-options-max-reserved-streams.js +++ b/test/parallel/test-http2-options-max-reserved-streams.js @@ -34,7 +34,11 @@ server.on('stream', common.mustCall((stream) => { }, common.mustSucceed((pushedStream) => { pushedStream.respond(); pushedStream.on('aborted', common.mustCall()); - pushedStream.on('error', common.mustNotCall()); + pushedStream.on('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); pushedStream.on('close', common.mustCall(() => { assert.strictEqual(pushedStream.rstCode, 8); countdown.dec(); diff --git a/test/parallel/test-http2-options-server-request.js b/test/parallel/test-http2-options-server-request.js index 2143d379823d51..ce84eab9ab4c57 100644 --- a/test/parallel/test-http2-options-server-request.js +++ b/test/parallel/test-http2-options-server-request.js @@ -35,6 +35,6 @@ server.on('listening', common.mustCall(() => { req.resume(); req.on('end', common.mustCall(() => { server.close(); - client.destroy(); + client.close(); })); })); diff --git a/test/parallel/test-http2-options-server-response.js b/test/parallel/test-http2-options-server-response.js index 6f1ae1881d22d8..8a85faa16961eb 100644 --- a/test/parallel/test-http2-options-server-response.js +++ b/test/parallel/test-http2-options-server-response.js @@ -29,6 +29,6 @@ server.on('listening', common.mustCall(() => { req.resume(); req.on('end', common.mustCall(() => { server.close(); - client.destroy(); + client.close(); })); })); diff --git a/test/parallel/test-http2-propagate-session-destroy-code.js b/test/parallel/test-http2-propagate-session-destroy-code.js index c54e52e085af4b..3e61738d9e7b9f 100644 --- a/test/parallel/test-http2-propagate-session-destroy-code.js +++ b/test/parallel/test-http2-propagate-session-destroy-code.js @@ -7,7 +7,8 @@ if (!common.hasCrypto) const assert = require('assert'); const http2 = require('http2'); const server = http2.createServer(); -const errRegEx = /Session closed with error code 7/; +const sessionErrorMessage = 'Session closed with error code NGHTTP2_REFUSED_STREAM'; +const streamErrorMessage = 'Stream closed with error code NGHTTP2_REFUSED_STREAM'; const destroyCode = http2.constants.NGHTTP2_REFUSED_STREAM; server.on('error', common.mustNotCall()); @@ -15,14 +16,14 @@ server.on('error', common.mustNotCall()); server.on('session', (session) => { session.on('close', common.mustCall()); session.on('error', common.mustCall((err) => { - assert.match(err.message, errRegEx); + assert.strictEqual(err.message, sessionErrorMessage); assert.strictEqual(session.closed, false); assert.strictEqual(session.destroyed, true); })); session.on('stream', common.mustCall((stream) => { stream.on('error', common.mustCall((err) => { - assert.match(err.message, errRegEx); + assert.strict(err.message, streamErrorMessage); assert.strictEqual(session.closed, false); assert.strictEqual(session.destroyed, true); assert.strictEqual(stream.rstCode, destroyCode); @@ -36,7 +37,7 @@ server.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server.address().port}`); session.on('error', common.mustCall((err) => { - assert.match(err.message, errRegEx); + assert.strictEqual(err.message, sessionErrorMessage); assert.strictEqual(session.closed, false); assert.strictEqual(session.destroyed, true); })); @@ -44,7 +45,7 @@ server.listen(0, common.mustCall(() => { const stream = session.request({ [http2.constants.HTTP2_HEADER_PATH]: '/' }); stream.on('error', common.mustCall((err) => { - assert.match(err.message, errRegEx); + assert.strictEqual(err.message, streamErrorMessage); assert.strictEqual(stream.rstCode, destroyCode); })); diff --git a/test/parallel/test-http2-respond-errors.js b/test/parallel/test-http2-respond-errors.js index cc733b6994a3bd..b90b2d7b89f4d8 100644 --- a/test/parallel/test-http2-respond-errors.js +++ b/test/parallel/test-http2-respond-errors.js @@ -13,7 +13,6 @@ const server = http2.createServer(); Http2Stream.prototype.respond = () => 1; server.on('stream', common.mustCall((stream) => { - // Send headers stream.respond({ 'content-type': 'text/plain' }); @@ -43,7 +42,13 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request(); - req.on('end', common.mustCall(() => { + req.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + req.on('end', common.mustNotCall()); + req.on('close', common.mustCall(() => { client.close(); server.close(); })); diff --git a/test/parallel/test-http2-respond-file-errors.js b/test/parallel/test-http2-respond-file-errors.js index 5c3424f2bc3484..c63017f8335ad9 100644 --- a/test/parallel/test-http2-respond-file-errors.js +++ b/test/parallel/test-http2-respond-file-errors.js @@ -29,7 +29,6 @@ const fname = fixtures.path('elipses.txt'); const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { - // Check for all possible TypeError triggers on options Object.keys(optionsWithTypeError).forEach((option) => { Object.keys(types).forEach((type) => { @@ -94,6 +93,11 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request(); + req.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.on('close', common.mustCall(() => { client.close(); server.close(); diff --git a/test/parallel/test-http2-respond-file-fd-errors.js b/test/parallel/test-http2-respond-file-fd-errors.js index 5f7e57ea4e45d2..43395b9bd985eb 100644 --- a/test/parallel/test-http2-respond-file-fd-errors.js +++ b/test/parallel/test-http2-respond-file-fd-errors.js @@ -117,6 +117,11 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request(); + req.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.on('close', common.mustCall(() => { client.close(); server.close(); diff --git a/test/parallel/test-http2-respond-with-file-connection-abort.js b/test/parallel/test-http2-respond-with-file-connection-abort.js index d5ed3645708d95..0220b7cc6ca70f 100644 --- a/test/parallel/test-http2-respond-with-file-connection-abort.js +++ b/test/parallel/test-http2-respond-with-file-connection-abort.js @@ -3,9 +3,16 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); -const assert = require('assert'); +const { writeFile } = require('fs').promises; const http2 = require('http2'); const net = require('net'); +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const file = tmpdir.resolve('large-file'); + +const windowSize = 2 ** 16 - 1; +const largeBuffer = new Uint8Array(windowSize * 2); const { HTTP2_HEADER_CONTENT_TYPE @@ -13,20 +20,35 @@ const { const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { - stream.on('error', (err) => assert.strictEqual(err.code, 'ECONNRESET')); - stream.respondWithFile(process.execPath, { + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + // Do not use process.execPath because on shared builds it can be less than window size! + stream.respondWithFile(file, { [HTTP2_HEADER_CONTENT_TYPE]: 'application/octet-stream' }); })); -server.listen(0, common.mustCall(() => { - const client = http2.connect(`http://localhost:${server.address().port}`); - const req = client.request(); +(async () => { + await writeFile(file, largeBuffer); - req.on('response', common.mustCall()); - req.once('data', common.mustCall(() => { - net.Socket.prototype.destroy.call(client.socket); - server.close(); + server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + req.on('response', common.mustCall()); + req.once('data', common.mustCall(() => { + net.Socket.prototype.destroy.call(client.socket); + server.close(); + })); + req.once('end', common.mustNotCall()); + req.end(); })); - req.end(); -})); +})().then(common.mustCall()); diff --git a/test/parallel/test-http2-server-rst-stream.js b/test/parallel/test-http2-server-rst-stream.js index 9f37ce71472353..09fbc91294a94d 100644 --- a/test/parallel/test-http2-server-rst-stream.js +++ b/test/parallel/test-http2-server-rst-stream.js @@ -19,7 +19,7 @@ const tests = [ [NGHTTP2_NO_ERROR, false], [NGHTTP2_NO_ERROR, false], [NGHTTP2_PROTOCOL_ERROR, true, 'NGHTTP2_PROTOCOL_ERROR'], - [NGHTTP2_CANCEL, false], + [NGHTTP2_CANCEL, true, 'NGHTTP2_CANCEL'], [NGHTTP2_REFUSED_STREAM, true, 'NGHTTP2_REFUSED_STREAM'], [NGHTTP2_INTERNAL_ERROR, true, 'NGHTTP2_INTERNAL_ERROR'], ]; diff --git a/test/parallel/test-http2-server-session-destroy.js b/test/parallel/test-http2-server-session-destroy.js index afa3dd3398dba8..10ef88856a387a 100644 --- a/test/parallel/test-http2-server-session-destroy.js +++ b/test/parallel/test-http2-server-session-destroy.js @@ -6,6 +6,14 @@ if (!common.hasCrypto) const h2 = require('http2'); const server = h2.createServer(); +server.once('stream', (stream) => { + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); +}); + server.listen(0, common.localhostIPv4, common.mustCall(() => { const afterConnect = common.mustCall((session) => { session.request({ ':method': 'POST' }).end(common.mustCall(() => { diff --git a/test/parallel/test-http2-server-sessionerror.js b/test/parallel/test-http2-server-sessionerror.js index b71b9df80f083f..89af3e4c1826e4 100644 --- a/test/parallel/test-http2-server-sessionerror.js +++ b/test/parallel/test-http2-server-sessionerror.js @@ -45,19 +45,15 @@ server.listen(0, common.mustCall(() => { const url = `http://localhost:${server.address().port}`; http2.connect(url) .on('error', common.mustCall((err) => { - if (err.code !== 'ECONNRESET') { - assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); - assert.strictEqual(err.message, 'Session closed with error code 2'); - } + assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); + assert.strictEqual(err.message, 'Session closed with error code NGHTTP2_INTERNAL_ERROR'); })) .on('close', () => { server.removeAllListeners('error'); http2.connect(url) .on('error', common.mustCall((err) => { - if (err.code !== 'ECONNRESET') { - assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); - assert.strictEqual(err.message, 'Session closed with error code 2'); - } + assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); + assert.strictEqual(err.message, 'Session closed with error code NGHTTP2_INTERNAL_ERROR'); })) .on('close', () => server.close()); }); diff --git a/test/parallel/test-http2-server-shutdown-options-errors.js b/test/parallel/test-http2-server-shutdown-options-errors.js index 5a2ca62a6c8e31..c3f53e55e05d07 100644 --- a/test/parallel/test-http2-server-shutdown-options-errors.js +++ b/test/parallel/test-http2-server-shutdown-options-errors.js @@ -59,6 +59,11 @@ server.listen( const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request(); req.resume(); + req.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.on('close', common.mustCall(() => { client.close(); server.close(); diff --git a/test/parallel/test-http2-server-shutdown-redundant.js b/test/parallel/test-http2-server-shutdown-redundant.js index 0f199bd68a1837..c8e7359c4396e1 100644 --- a/test/parallel/test-http2-server-shutdown-redundant.js +++ b/test/parallel/test-http2-server-shutdown-redundant.js @@ -9,6 +9,12 @@ const http2 = require('http2'); const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + const session = stream.session; session.goaway(1); session.goaway(2); diff --git a/test/parallel/test-http2-server-socket-destroy.js b/test/parallel/test-http2-server-socket-destroy.js index 4e9a3f5ab375c0..6b0ff60818143d 100644 --- a/test/parallel/test-http2-server-socket-destroy.js +++ b/test/parallel/test-http2-server-socket-destroy.js @@ -32,30 +32,30 @@ function onStream(stream) { stream.on('aborted', common.mustCall()); assert.notStrictEqual(stream.session, undefined); - socket.destroy(); + + stream.once('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + + // Always send RST. + socket.resetAndDestroy(); } server.listen(0); server.on('listening', common.mustCall(async () => { const client = h2.connect(`http://localhost:${server.address().port}`); - // The client may have an ECONNRESET error here depending on the operating - // system, due mainly to differences in the timing of socket closing. Do - // not wrap this in a common mustCall. - client.on('error', (err) => { - if (err.code !== 'ECONNRESET') - throw err; - }); + client.on('error', common.mustNotCall()); client.on('close', common.mustCall()); const req = client.request({ ':method': 'POST' }); - // The client may have an ECONNRESET error here depending on the operating - // system, due mainly to differences in the timing of socket closing. Do - // not wrap this in a common mustCall. - req.on('error', (err) => { - if (err.code !== 'ECONNRESET') - throw err; - }); + req.on('error', common.expectsError({ + name: 'Error', + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); req.on('aborted', common.mustCall()); req.resume(); diff --git a/test/parallel/test-http2-server-stream-session-destroy.js b/test/parallel/test-http2-server-stream-session-destroy.js index 34b22fdfbd1334..5a7053b37f1d97 100644 --- a/test/parallel/test-http2-server-stream-session-destroy.js +++ b/test/parallel/test-http2-server-stream-session-destroy.js @@ -36,7 +36,6 @@ server.on('stream', common.mustCall((stream) => { ); // When session is destroyed all streams are destroyed and no further // error should be emitted. - stream.on('error', common.mustNotCall()); assert.strictEqual(stream.write('data', common.expectsError({ name: 'Error', code: 'ERR_STREAM_WRITE_AFTER_END', @@ -48,6 +47,11 @@ server.listen(0, common.mustCall(() => { const client = h2.connect(`http://localhost:${server.address().port}`); const req = client.request(); req.resume(); - req.on('end', common.mustCall()); + req.once('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' + })); + req.on('end', common.mustNotCall()); req.on('close', common.mustCall(() => server.close(common.mustCall()))); })); diff --git a/test/parallel/test-http2-too-many-settings.js b/test/parallel/test-http2-too-many-settings.js index da5d5865792a83..8094debc6c7867 100644 --- a/test/parallel/test-http2-too-many-settings.js +++ b/test/parallel/test-http2-too-many-settings.js @@ -30,10 +30,8 @@ function doTest(session) { server.listen(0, common.mustCall(() => { const client = h2.connect(`http://localhost:${server.address().port}`); client.on('error', common.mustCall((err) => { - if (err.code !== 'ECONNRESET') { - assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); - assert.strictEqual(err.message, 'Session closed with error code 2'); - } + assert.strictEqual(err.code, 'ERR_HTTP2_SESSION_ERROR'); + assert.strictEqual(err.message, 'Session closed with error code NGHTTP2_INTERNAL_ERROR'); })); client.on('close', common.mustCall(() => server.close())); })); diff --git a/test/parallel/test-http2-zero-length-header.js b/test/parallel/test-http2-zero-length-header.js index 2e7876858aaace..c48dc202b8f831 100644 --- a/test/parallel/test-http2-zero-length-header.js +++ b/test/parallel/test-http2-zero-length-header.js @@ -7,7 +7,7 @@ const assert = require('assert'); const http2 = require('http2'); const server = http2.createServer(); -server.on('stream', (stream, headers) => { +server.on('stream', common.mustCall((stream, headers) => { assert.deepStrictEqual(headers, { ':scheme': 'http', ':authority': `localhost:${server.address().port}`, @@ -17,10 +17,15 @@ server.on('stream', (stream, headers) => { '__proto__': null, [http2.sensitiveHeaders]: [] }); - stream.session.destroy(); + stream.respond(); + stream.end(); server.close(); -}); -server.listen(0, common.mustCall(() => { - const client = http2.connect(`http://localhost:${server.address().port}/`); - client.request({ ':path': '/', '': 'foo', 'bar': '' }).end(); })); +server.listen(0, () => { + const client = http2.connect(`http://localhost:${server.address().port}/`); + const req = client.request({ ':path': '/', '': 'foo', 'bar': '' }); + req.end(); + req.on('close', () => { + client.close(); + }); +}); diff --git a/test/sequential/test-http2-max-session-memory.js b/test/sequential/test-http2-max-session-memory.js index 9b77a45c3227fd..c007c0ac41ec2c 100644 --- a/test/sequential/test-http2-max-session-memory.js +++ b/test/sequential/test-http2-max-session-memory.js @@ -13,10 +13,6 @@ const largeBuffer = Buffer.alloc(2e6); const server = http2.createServer({ maxSessionMemory: 1 }); server.on('stream', common.mustCall((stream) => { - stream.on('error', (err) => { - if (err.code !== 'ECONNRESET') - throw err; - }); stream.respond(); stream.end(largeBuffer); })); @@ -38,7 +34,7 @@ server.listen(0, common.mustCall(() => { })); req.on('close', common.mustCall(() => { server.close(); - client.destroy(); + client.close(); })); });