From 0822f0ddcbbbd8997daea7ff473803074c7d8c86 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Sat, 12 Oct 2024 09:48:19 +0200 Subject: [PATCH 1/5] fix: restructure client-h2 --- lib/dispatcher/client-h2.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index e8e78f99100..43c34a9d0b2 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -93,6 +93,7 @@ async function connectH2 (client, socket) { session[kOpenStreams] = 0 session[kClient] = client session[kSocket] = socket + session[kHTTP2Session] = null util.addListener(session, 'error', onHttp2SessionError) util.addListener(session, 'frameError', onHttp2FrameError) @@ -140,7 +141,7 @@ async function connectH2 (client, socket) { client[kSocket] = null - if (this[kHTTP2Session] != null) { + if (this[kHTTP2Session] !== null) { this[kHTTP2Session].destroy(err) } @@ -223,16 +224,19 @@ function onHttp2SessionEnd () { * This is the root cause of #3011 * We need to handle GOAWAY frames properly, and trigger the session close * along with the socket right away + * + * @this {import('http2').ClientHttp2Session} + * @param {number} errorCode */ -function onHTTP2GoAway (code) { +function onHTTP2GoAway (errorCode) { // We cannot recover, so best to close the session and the socket - const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${code}`, util.getSocketInfo(this)) + const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, util.getSocketInfo(this[kSocket])) const client = this[kClient] client[kSocket] = null client[kHTTPContext] = null - if (this[kHTTP2Session] != null) { + if (this[kHTTP2Session] !== null) { this[kHTTP2Session].destroy(err) this[kHTTP2Session] = null } From 4296882ae5856d4dc37b5f9f84b12fb1e6b62c1a Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Sat, 12 Oct 2024 09:52:47 +0200 Subject: [PATCH 2/5] extract onHttp2SessionClose and rename onHTTP2GoAway to onHttp2SesssionGoAway --- lib/dispatcher/client-h2.js | 44 +++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 43c34a9d0b2..6ba82743f2a 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -98,26 +98,8 @@ async function connectH2 (client, socket) { util.addListener(session, 'error', onHttp2SessionError) util.addListener(session, 'frameError', onHttp2FrameError) util.addListener(session, 'end', onHttp2SessionEnd) - util.addListener(session, 'goaway', onHTTP2GoAway) - util.addListener(session, 'close', function () { - const { [kClient]: client } = this - const { [kSocket]: socket } = client - - const err = this[kSocket][kError] || this[kError] || new SocketError('closed', util.getSocketInfo(socket)) - - client[kHTTP2Session] = null - - if (client.destroyed) { - assert(client[kPending] === 0) - - // Fail entire queue. - const requests = client[kQueue].splice(client[kRunningIdx]) - for (let i = 0; i < requests.length; i++) { - const request = requests[i] - util.errorRequest(client, request, err) - } - } - }) + util.addListener(session, 'goaway', onHttp2SessionGoAway) + util.addListener(session, 'close', onHttp2SessionClose) session.unref() @@ -228,7 +210,7 @@ function onHttp2SessionEnd () { * @this {import('http2').ClientHttp2Session} * @param {number} errorCode */ -function onHTTP2GoAway (errorCode) { +function onHttp2SessionGoAway (errorCode) { // We cannot recover, so best to close the session and the socket const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, util.getSocketInfo(this[kSocket])) const client = this[kClient] @@ -257,6 +239,26 @@ function onHTTP2GoAway (errorCode) { client[kResume]() } +function onHttp2SessionClose () { + const { [kClient]: client } = this + const { [kSocket]: socket } = client + + const err = this[kSocket][kError] || this[kError] || new SocketError('closed', util.getSocketInfo(socket)) + + client[kHTTP2Session] = null + + if (client.destroyed) { + assert(client[kPending] === 0) + + // Fail entire queue. + const requests = client[kQueue].splice(client[kRunningIdx]) + for (let i = 0; i < requests.length; i++) { + const request = requests[i] + util.errorRequest(client, request, err) + } + } +} + // https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 function shouldSendContentLength (method) { return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT' From f292da88b5045c0ccd8683e7db9f1dccd3d80c50 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Sat, 12 Oct 2024 09:54:18 +0200 Subject: [PATCH 3/5] fix function header --- lib/dispatcher/client-h2.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 6ba82743f2a..c8d1a2a6d95 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -144,8 +144,8 @@ async function connectH2 (client, socket) { return { version: 'h2', defaultPipelining: Infinity, - write (...args) { - return writeH2(client, ...args) + write (request) { + return writeH2(client, request) }, resume () { resumeH2(client) From f6695f2850d7f877aa27a224d9495d7918fd6393 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Sat, 12 Oct 2024 10:02:35 +0200 Subject: [PATCH 4/5] extract functions --- lib/dispatcher/client-h2.js | 64 ++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index c8d1a2a6d95..b82453fbac3 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -106,35 +106,9 @@ async function connectH2 (client, socket) { client[kHTTP2Session] = session socket[kHTTP2Session] = session - util.addListener(socket, 'error', function (err) { - assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') - - this[kError] = err - - this[kClient][kOnError](err) - }) - - util.addListener(socket, 'end', function () { - util.destroy(this, new SocketError('other side closed', util.getSocketInfo(this))) - }) - - util.addListener(socket, 'close', function () { - const err = this[kError] || new SocketError('closed', util.getSocketInfo(this)) - - client[kSocket] = null - - if (this[kHTTP2Session] !== null) { - this[kHTTP2Session].destroy(err) - } - - client[kPendingIdx] = client[kRunningIdx] - - assert(client[kRunning] === 0) - - client.emit('disconnect', client[kUrl], [client], err) - - client[kResume]() - }) + util.addListener(socket, 'error', onHttp2SocketError) + util.addListener(socket, 'end', onHttp2SocketEnd) + util.addListener(socket, 'close', onHttp2SocketClose) let closed = false socket.on('close', () => { @@ -259,6 +233,38 @@ function onHttp2SessionClose () { } } +function onHttp2SocketClose () { + const err = this[kError] || new SocketError('closed', util.getSocketInfo(this)) + + const client = this[kHTTP2Session][kClient] + + client[kSocket] = null + + if (this[kHTTP2Session] !== null) { + this[kHTTP2Session].destroy(err) + } + + client[kPendingIdx] = client[kRunningIdx] + + assert(client[kRunning] === 0) + + client.emit('disconnect', client[kUrl], [client], err) + + client[kResume]() +} + +function onHttp2SocketError (err) { + assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID') + + this[kError] = err + + this[kClient][kOnError](err) +} + +function onHttp2SocketEnd () { + util.destroy(this, new SocketError('other side closed', util.getSocketInfo(this))) +} + // https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 function shouldSendContentLength (method) { return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT' From d59ec63afbf7d9e40e886bcd0d95d216fa2c16bb Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Sat, 12 Oct 2024 10:19:10 +0200 Subject: [PATCH 5/5] extract onSocketClose --- lib/dispatcher/client-h2.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index b82453fbac3..71cf1276a2b 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -26,7 +26,8 @@ const { kHTTP2Session, kResume, kSize, - kHTTPContext + kHTTPContext, + kClosed } = require('../core/symbols.js') const kOpenStreams = Symbol('open streams') @@ -110,10 +111,8 @@ async function connectH2 (client, socket) { util.addListener(socket, 'end', onHttp2SocketEnd) util.addListener(socket, 'close', onHttp2SocketClose) - let closed = false - socket.on('close', () => { - closed = true - }) + socket[kClosed] = false + socket.on('close', onSocketClose) return { version: 'h2', @@ -125,7 +124,7 @@ async function connectH2 (client, socket) { resumeH2(client) }, destroy (err, callback) { - if (closed) { + if (socket[kClosed]) { queueMicrotask(callback) } else { // Destroying the socket will trigger the session close @@ -265,6 +264,10 @@ function onHttp2SocketEnd () { util.destroy(this, new SocketError('other side closed', util.getSocketInfo(this))) } +function onSocketClose () { + this[kClosed] = true +} + // https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 function shouldSendContentLength (method) { return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT'