From 7c152dacae12742ef876877b73db37a7c8348bfc Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:15:18 +0200 Subject: [PATCH] does not matter if TCP FIN or TCP RST --- lib/internal/http2/core.js | 34 +++++++++++++------ ...st-http2-client-connection-tunnel-reset.js | 10 ++---- ...ttp2-respond-with-file-connection-abort.js | 6 +++- .../test-http2-server-socket-destroy.js | 13 ++----- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2f00130ac8c4ce..1340b87a06100d 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3020,11 +3020,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); } } @@ -3305,15 +3306,28 @@ function socketOnClose() { const err = session.connecting ? new ERR_SOCKET_CLOSED() : null; const state = session[kState]; + // Code for premature close let code = state.goawayCode ?? state.destroyCode; - if (code === NGHTTP2_NO_ERROR || code === null) { - code = session[kType] === NGHTTP2_SESSION_SERVER ? + if (code === NGHTTP2_NO_ERROR) { + // Got NO_ERROR but still closed prematurely + code = null; + } + + const defaultCode = session[kType] === NGHTTP2_SESSION_SERVER ? NGHTTP2_CANCEL : NGHTTP2_INTERNAL_ERROR; - } - state.streams.forEach((stream) => stream.close(code)); - state.pendingStreams.forEach((stream) => stream.close(code)); + 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.close(); session[kMaybeDestroy](err); } diff --git a/test/parallel/test-http2-client-connection-tunnel-reset.js b/test/parallel/test-http2-client-connection-tunnel-reset.js index 83614525ae6256..6e19e3760aed05 100644 --- a/test/parallel/test-http2-client-connection-tunnel-reset.js +++ b/test/parallel/test-http2-client-connection-tunnel-reset.js @@ -20,18 +20,14 @@ h2Server.on('connect', (req, res) => { h2Server.listen(0, common.mustCall(() => { const proxyClient = h2.connect(`http://localhost:${h2Server.address().port}`); - proxyClient.once('error', common.expectsError({ - name: 'Error', - code: 'ECONNRESET', - message: 'read ECONNRESET' - })); + proxyClient.once('error', common.mustNotCall()); const proxyReq = proxyClient.request({ ':method': 'CONNECT', ':authority': 'example.com:443' }); proxyReq.once('error', common.expectsError({ name: 'Error', - code: 'ECONNRESET', - message: 'read ECONNRESET' + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_CONNECT_ERROR' })); })); 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 c2ad4e11a503a1..013560f0ce3d85 100644 --- a/test/parallel/test-http2-respond-with-file-connection-abort.js +++ b/test/parallel/test-http2-respond-with-file-connection-abort.js @@ -13,7 +13,11 @@ const { const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { - stream.on('error', (err) => assert.strictEqual(err.code, 'ECONNRESET')); + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_CANCEL' + })); stream.respondWithFile(process.execPath, { [HTTP2_HEADER_CONTENT_TYPE]: 'application/octet-stream' }); diff --git a/test/parallel/test-http2-server-socket-destroy.js b/test/parallel/test-http2-server-socket-destroy.js index 1d00d9c4eaf40d..6f52436fde505a 100644 --- a/test/parallel/test-http2-server-socket-destroy.js +++ b/test/parallel/test-http2-server-socket-destroy.js @@ -39,9 +39,6 @@ function onStream(stream) { message: 'Stream closed with error code NGHTTP2_CANCEL' })); - // Do not use destroy() as it sends FIN on Linux. - // On macOS, it sends RST. - // Always send RST. socket.resetAndDestroy(); } @@ -50,18 +47,14 @@ server.listen(0); server.on('listening', common.mustCall(async () => { const client = h2.connect(`http://localhost:${server.address().port}`); - client.on('error', common.expectsError({ - name: 'Error', - code: 'ECONNRESET', - message: 'read ECONNRESET' - })); + client.on('error', common.mustNotCall()); client.on('close', common.mustCall()); const req = client.request({ ':method': 'POST' }); req.on('error', common.expectsError({ name: 'Error', - code: 'ECONNRESET', - message: 'read ECONNRESET' + code: 'ERR_HTTP2_STREAM_ERROR', + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); req.on('aborted', common.mustCall());