From d5aed82b933b23792578b3b61be1baa3e594174c Mon Sep 17 00:00:00 2001 From: Carlos Vinicius Date: Thu, 10 Sep 2026 13:23:52 -0300 Subject: [PATCH] test(h2): stabilize http2-connection - Ignore the client's own idle-socket teardown in the unexpected-disconnect guard. Once keepAliveTimeout elapses on an idle h2 session the client drops the socket itself and reconnects transparently on the next request, but the guard reported it as a failure. The guard now lives in test/utils/h2-disconnect-guard.js and has its own coverage. - Use the static pem instead of generating a fresh RSA-2048 key per test. The node-forge keygen dominated the runtime and is the same call that produced ERR_OSSL_ASN1_ILLEGAL_PADDING on a Windows job in #5195. - Close the client before the server and await both, so the client never has to react to a GOAWAY it did not ask for. - Bind and connect on 127.0.0.1 instead of localhost. - Assert the second response body against body2; it was checking body, so the second request's payload was never verified. The two responses are now numbered to keep that assertion honest. --- test/h2-disconnect-guard.js | 78 +++++++++++ test/http2-connection.js | 213 +++++++++++++++--------------- test/utils/h2-disconnect-guard.js | 31 +++++ 3 files changed, 216 insertions(+), 106 deletions(-) create mode 100644 test/h2-disconnect-guard.js create mode 100644 test/utils/h2-disconnect-guard.js diff --git a/test/h2-disconnect-guard.js b/test/h2-disconnect-guard.js new file mode 100644 index 00000000000..be0ff1bc3f2 --- /dev/null +++ b/test/h2-disconnect-guard.js @@ -0,0 +1,78 @@ +'use strict' + +const assert = require('node:assert') +const { test } = require('node:test') +const { createSecureServer } = require('node:http2') +const { once } = require('node:events') + +const pem = require('@metcoder95/https-pem') + +const { Client } = require('..') +const { guardAgainstUnexpectedDisconnect } = require('./utils/h2-disconnect-guard') + +async function setup (t, clientOpts = {}) { + const server = createSecureServer(pem) + const sessions = [] + + server.on('session', session => sessions.push(session)) + server.on('stream', stream => { + stream.respond({ ':status': 200 }) + stream.end('hello h2!') + }) + + let client = null + t.after(async () => { + if (client != null && !client.destroyed) { + await client.close() + } + await new Promise(resolve => server.close(resolve)) + }) + + await once(server.listen(0, '127.0.0.1'), 'listening') + + client = new Client(`https://127.0.0.1:${server.address().port}`, { + connect: { rejectUnauthorized: false }, + allowH2: true, + ...clientOpts + }) + + const failures = [] + guardAgainstUnexpectedDisconnect({ fail: message => failures.push(message) }, client) + + return { client, sessions, failures, get: () => client.request({ path: '/', method: 'GET' }) } +} + +test('ignores the client tearing down its own idle session', async t => { + // A saturated CI runner can stretch the gap between two requests past + // keepAliveTimeout; the client then drops the idle socket itself and + // reconnects transparently on the next request. That is not a disconnect the + // test did not ask for. + const { client, failures, get } = await setup(t, { keepAliveTimeout: 100 }) + + const disconnected = once(client, 'disconnect') + await (await get()).body.text() + await disconnected + + assert.deepStrictEqual(failures, []) +}) + +test('flags a disconnect forced by the peer', async t => { + const { client, sessions, failures, get } = await setup(t) + + const disconnected = once(client, 'disconnect') + await (await get()).body.text() + sessions[0].destroy() + await disconnected + + assert.strictEqual(failures.length, 1) + assert.match(failures[0], /^unexpected disconnect/) +}) + +test('stays quiet once the client is closing', async t => { + const { client, failures, get } = await setup(t) + + await (await get()).body.text() + await client.close() + + assert.deepStrictEqual(failures, []) +}) diff --git a/test/http2-connection.js b/test/http2-connection.js index 67ba62bbdb8..b29ad8a3d34 100644 --- a/test/http2-connection.js +++ b/test/http2-connection.js @@ -1,7 +1,7 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') -const { test, after } = require('node:test') +const { test } = require('node:test') const { createSecureServer } = require('node:http2') const { once } = require('node:events') const { Readable } = require('node:stream') @@ -9,20 +9,34 @@ const { Readable } = require('node:stream') const pem = require('@metcoder95/https-pem') const { Client } = require('..') +const { guardAgainstUnexpectedDisconnect } = require('./utils/h2-disconnect-guard') + +// Tears the client down before the server, so the client never has to react to +// a GOAWAY it did not ask for, and waits for both. +function teardown (t, server, getClient) { + t.after(async () => { + const client = getClient() + if (client != null && !client.destroyed) { + await client.close() + } + await new Promise(resolve => server.close(resolve)) + }) +} test('Should support H2 connection', async t => { - t = tspl(t, { plan: 9 }) + const assert = tspl(t, { plan: 9 }) const body = [] - const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + const server = createSecureServer(pem) let authority = '' + let client = null server.on('stream', (stream, headers, _flags, rawHeaders) => { - t.strictEqual(headers['x-my-header'], 'foo') - t.strictEqual(headers[':method'], 'GET') - t.strictEqual(headers[':scheme'], 'https') - t.strictEqual(headers[':path'], '/') - t.strictEqual(headers[':authority'], authority) + assert.strictEqual(headers['x-my-header'], 'foo') + assert.strictEqual(headers[':method'], 'GET') + assert.strictEqual(headers[':scheme'], 'https') + assert.strictEqual(headers[':path'], '/') + assert.strictEqual(headers[':authority'], authority) stream.respond({ 'content-type': 'text/plain; charset=utf-8', 'x-custom-h2': 'hello', @@ -31,24 +45,19 @@ test('Should support H2 connection', async t => { stream.end('hello h2!') }) - after(() => server.close()) + teardown(t, server, () => client) - await once(server.listen(0), 'listening') + await once(server.listen(0, '127.0.0.1'), 'listening') - authority = `localhost:${server.address().port}` - const client = new Client(`https://${authority}`, { + authority = `127.0.0.1:${server.address().port}` + client = new Client(`https://${authority}`, { connect: { rejectUnauthorized: false }, allowH2: true }) - after(() => client.close()) - client.on('disconnect', () => { - if (!client.closed && !client.destroyed) { - t.fail('unexpected disconnect') - } - }) + guardAgainstUnexpectedDisconnect(assert, client) const response = await client.request({ path: '/', @@ -64,27 +73,28 @@ test('Should support H2 connection', async t => { await once(response.body, 'end') - t.strictEqual(response.statusCode, 200) - t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') + assert.strictEqual(response.statusCode, 200) + assert.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') - await t.completed + await assert.completed }) test('Should support H2 connection(multiple requests)', async t => { - t = tspl(t, { plan: 21 }) + const assert = tspl(t, { plan: 21 }) - const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + const server = createSecureServer(pem) + let client = null server.on('stream', async (stream, headers, _flags, rawHeaders) => { - t.strictEqual(headers['x-my-header'], 'foo') - t.strictEqual(headers[':method'], 'POST') + assert.strictEqual(headers['x-my-header'], 'foo') + assert.strictEqual(headers[':method'], 'POST') const reqData = [] stream.on('data', chunk => reqData.push(chunk.toString())) await once(stream, 'end') const reqBody = reqData.join('') - t.strictEqual(reqBody.length > 0, true) + assert.strictEqual(reqBody.length > 0, true) stream.respond({ 'content-type': 'text/plain; charset=utf-8', 'x-custom-h2': 'hello', @@ -93,22 +103,18 @@ test('Should support H2 connection(multiple requests)', async t => { stream.end(`hello h2! ${reqBody}`) }) - after(() => server.close()) - await once(server.listen(0), 'listening') + teardown(t, server, () => client) + + await once(server.listen(0, '127.0.0.1'), 'listening') - const client = new Client(`https://localhost:${server.address().port}`, { + client = new Client(`https://127.0.0.1:${server.address().port}`, { connect: { rejectUnauthorized: false }, allowH2: true }) - after(() => client.close()) - client.on('disconnect', () => { - if (!client.closed && !client.destroyed) { - t.fail('unexpected disconnect') - } - }) + guardAgainstUnexpectedDisconnect(assert, client) for (let i = 0; i < 3; i++) { const sendBody = `seq ${i}` @@ -129,26 +135,27 @@ test('Should support H2 connection(multiple requests)', async t => { await once(response.body, 'end') - t.strictEqual(response.statusCode, 200) - t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), `hello h2! ${sendBody}`) + assert.strictEqual(response.statusCode, 200) + assert.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body).toString('utf8'), `hello h2! ${sendBody}`) } - await t.completed + await assert.completed }) test('Should support H2 connection (headers as array)', async t => { - t = tspl(t, { plan: 8 }) + const assert = tspl(t, { plan: 8 }) const body = [] - const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + const server = createSecureServer(pem) + let client = null server.on('stream', (stream, headers) => { - t.strictEqual(headers['x-my-header'], 'foo, bar') - t.strictEqual(headers['x-my-drink'], 'coffee, tea, water') - t.strictEqual(headers['x-other'], 'value') - t.strictEqual(headers[':method'], 'GET') + assert.strictEqual(headers['x-my-header'], 'foo, bar') + assert.strictEqual(headers['x-my-drink'], 'coffee, tea, water') + assert.strictEqual(headers['x-other'], 'value') + assert.strictEqual(headers[':method'], 'GET') stream.respond({ 'content-type': 'text/plain; charset=utf-8', 'x-custom-h2': 'hello', @@ -157,22 +164,18 @@ test('Should support H2 connection (headers as array)', async t => { stream.end('hello h2!') }) - after(() => server.close()) - await once(server.listen(0), 'listening') + teardown(t, server, () => client) - const client = new Client(`https://localhost:${server.address().port}`, { + await once(server.listen(0, '127.0.0.1'), 'listening') + + client = new Client(`https://127.0.0.1:${server.address().port}`, { connect: { rejectUnauthorized: false }, allowH2: true }) - after(() => client.close()) - client.on('disconnect', () => { - if (!client.closed && !client.destroyed) { - t.fail('unexpected disconnect') - } - }) + guardAgainstUnexpectedDisconnect(assert, client) const response = await client.request({ path: '/', @@ -192,52 +195,53 @@ test('Should support H2 connection (headers as array)', async t => { await once(response.body, 'end') - t.strictEqual(response.statusCode, 200) - t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') + assert.strictEqual(response.statusCode, 200) + assert.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') - await t.completed + await assert.completed }) test('Should support multiple header values with semicolon separator', async t => { - t = tspl(t, { plan: 9 * 2 }) + const assert = tspl(t, { plan: 9 * 2 }) const body = [] const body2 = [] const expectedCookieHeaders = ['a=b', 'c=d', 'e=f'] - const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + const server = createSecureServer(pem) + let client = null + // The two requests carry the same headers by design, so tell their responses + // apart to keep each assertion pinned to its own response. + let seq = 0 server.on('stream', (stream, headers) => { - t.strictEqual(headers['x-my-header'], 'foo, bar') - t.strictEqual(headers['x-my-drink'], 'coffee, tea, water') - t.strictEqual(headers['x-other'], 'value') - t.strictEqual(headers['cookie'], expectedCookieHeaders.join('; ')) - t.strictEqual(headers[':method'], 'GET') + const n = ++seq + assert.strictEqual(headers['x-my-header'], 'foo, bar') + assert.strictEqual(headers['x-my-drink'], 'coffee, tea, water') + assert.strictEqual(headers['x-other'], 'value') + assert.strictEqual(headers.cookie, expectedCookieHeaders.join('; ')) + assert.strictEqual(headers[':method'], 'GET') stream.respond({ 'content-type': 'text/plain; charset=utf-8', 'x-custom-h2': 'hello', ':status': 200 }) - stream.end('hello h2!') + stream.end(`hello h2! ${n}`) }) - after(() => server.close()) - await once(server.listen(0), 'listening') + teardown(t, server, () => client) - const client = new Client(`https://localhost:${server.address().port}`, { + await once(server.listen(0, '127.0.0.1'), 'listening') + + client = new Client(`https://127.0.0.1:${server.address().port}`, { connect: { rejectUnauthorized: false }, allowH2: true }) - after(() => client.close()) - client.on('disconnect', () => { - if (!client.closed && !client.destroyed) { - t.fail('unexpected disconnect') - } - }) + guardAgainstUnexpectedDisconnect(assert, client) const response = await client.request({ path: '/', @@ -258,10 +262,10 @@ test('Should support multiple header values with semicolon separator', async t = await once(response.body, 'end') - t.strictEqual(response.statusCode, 200) - t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') + assert.strictEqual(response.statusCode, 200) + assert.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2! 1') const response2 = await client.request({ path: '/', @@ -284,25 +288,26 @@ test('Should support multiple header values with semicolon separator', async t = await once(response2.body, 'end') - t.strictEqual(response2.statusCode, 200) - t.strictEqual(response2.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response2.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') + assert.strictEqual(response2.statusCode, 200) + assert.strictEqual(response2.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response2.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body2).toString('utf8'), 'hello h2! 2') - await t.completed + await assert.completed }) test('Should support H2 connection(POST Buffer)', async t => { - t = tspl(t, { plan: 6 }) + const assert = tspl(t, { plan: 6 }) - const server = createSecureServer({ ...await pem.generate({ opts: { keySize: 2048 } }), allowHTTP1: false }) + const server = createSecureServer({ key: pem.key, cert: pem.cert, allowHTTP1: false }) + let client = null server.on('stream', async (stream, headers, _flags, rawHeaders) => { - t.strictEqual(headers[':method'], 'POST') + assert.strictEqual(headers[':method'], 'POST') const reqData = [] stream.on('data', chunk => reqData.push(chunk.toString())) await once(stream, 'end') - t.strictEqual(reqData.join(''), 'hello!') + assert.strictEqual(reqData.join(''), 'hello!') stream.respond({ 'content-type': 'text/plain; charset=utf-8', 'x-custom-h2': 'hello', @@ -311,22 +316,18 @@ test('Should support H2 connection(POST Buffer)', async t => { stream.end('hello h2!') }) - after(() => server.close()) - await once(server.listen(0), 'listening') + teardown(t, server, () => client) + + await once(server.listen(0, '127.0.0.1'), 'listening') - const client = new Client(`https://localhost:${server.address().port}`, { + client = new Client(`https://127.0.0.1:${server.address().port}`, { connect: { rejectUnauthorized: false }, allowH2: true }) - after(() => client.close()) - client.on('disconnect', () => { - if (!client.closed && !client.destroyed) { - t.fail('unexpected disconnect') - } - }) + guardAgainstUnexpectedDisconnect(assert, client) const sendBody = 'hello!' const body = [] @@ -342,10 +343,10 @@ test('Should support H2 connection(POST Buffer)', async t => { await once(response.body, 'end') - t.strictEqual(response.statusCode, 200) - t.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') - t.strictEqual(response.headers['x-custom-h2'], 'hello') - t.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') + assert.strictEqual(response.statusCode, 200) + assert.strictEqual(response.headers['content-type'], 'text/plain; charset=utf-8') + assert.strictEqual(response.headers['x-custom-h2'], 'hello') + assert.strictEqual(Buffer.concat(body).toString('utf8'), 'hello h2!') - await t.completed + await assert.completed }) diff --git a/test/utils/h2-disconnect-guard.js b/test/utils/h2-disconnect-guard.js new file mode 100644 index 00000000000..a6d568e2ca8 --- /dev/null +++ b/test/utils/h2-disconnect-guard.js @@ -0,0 +1,31 @@ +'use strict' + +// The client drops an idle h2 socket itself once keepAliveTimeout elapses and +// reconnects transparently on the next request. A saturated CI runner can +// stretch the gap between two requests past that deadline, so this is a +// disconnect the test caused, not one the peer forced on it. +const SELF_INFLICTED_DISCONNECTS = new Set([ + 'socket idle timeout' +]) + +// Fails `t` when the client loses its connection for a reason the test did not +// ask for. Returns a function that removes the guard. +function guardAgainstUnexpectedDisconnect (t, client) { + const onDisconnect = (_url, _targets, err) => { + if (client.closed || client.destroyed) { + return + } + + if (err != null && SELF_INFLICTED_DISCONNECTS.has(err.message)) { + return + } + + t.fail(`unexpected disconnect: ${err?.message ?? 'no error'}`) + } + + client.on('disconnect', onDisconnect) + + return () => client.off('disconnect', onDisconnect) +} + +module.exports = { guardAgainstUnexpectedDisconnect }