From b18831fad25ff71c1ef763a92839370a0040b979 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 8 Nov 2024 11:10:42 +0000 Subject: [PATCH 1/3] feat!(H2): enable H2 by default --- docs/docs/api/Client.md | 2 - lib/core/connect.js | 2 +- lib/dispatcher/client-h2.js | 10 ----- test/fetch/http2.js | 37 +++++++----------- test/http2-alpn.js | 6 +-- test/http2.js | 77 ++++++++++++------------------------- 6 files changed, 42 insertions(+), 92 deletions(-) diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md index 0be99625d2e..463cde0a247 100644 --- a/docs/docs/api/Client.md +++ b/docs/docs/api/Client.md @@ -17,8 +17,6 @@ Returns: `Client` ### Parameter: `ClientOptions` -> ⚠️ Warning: The `H2` support is experimental. - * **bodyTimeout** `number | null` (optional) - Default: `300e3` - The timeout after which a request will time out, in milliseconds. Monitors time between receiving body data. Use `0` to disable it entirely. Defaults to 300 seconds. Please note the `timeout` will be reset if you keep writing data to the socket everytime. * **headersTimeout** `number | null` (optional) - Default: `300e3` - The amount of time, in milliseconds, the parser will wait to receive the complete HTTP headers while not sending the request. Defaults to 300 seconds. * **keepAliveMaxTimeout** `number | null` (optional) - Default: `600e3` - The maximum allowed `keepAliveTimeout`, in milliseconds, when overridden by *keep-alive* hints from the server. Defaults to 10 minutes. diff --git a/lib/core/connect.js b/lib/core/connect.js index 8ab21fcd5fc..f252a156834 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -84,7 +84,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess const options = { path: socketPath, ...opts } const sessionCache = new SessionCache(maxCachedSessions == null ? 100 : maxCachedSessions) timeout = timeout == null ? 10e3 : timeout - allowH2 = allowH2 != null ? allowH2 : false + allowH2 = allowH2 != null ? allowH2 : true return function connect ({ hostname, host, protocol, port, servername, localAddress, httpSocket }, callback) { let socket if (protocol === 'https:') { diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index ef6d47a0c9c..770083aa935 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -32,9 +32,6 @@ const { const kOpenStreams = Symbol('open streams') -// Experimental -let h2ExperimentalWarned = false - /** @type {import('http2')} */ let http2 try { @@ -79,13 +76,6 @@ function parseH2Headers (headers) { async function connectH2 (client, socket) { client[kSocket] = socket - if (!h2ExperimentalWarned) { - h2ExperimentalWarned = true - process.emitWarning('H2 support is experimental, expect them to change at any time.', { - code: 'UNDICI-H2' - }) - } - const session = http2.connect(client[kUrl], { createConnection: () => socket, peerMaxConcurrentStreams: client[kMaxConcurrentStreams] diff --git a/test/fetch/http2.js b/test/fetch/http2.js index f64756de788..eb44a24b2b4 100644 --- a/test/fetch/http2.js +++ b/test/fetch/http2.js @@ -42,8 +42,7 @@ test('[Fetch] Issue#2311', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) const response = await fetch( @@ -91,8 +90,7 @@ test('[Fetch] Simple GET with h2', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) const response = await fetch( @@ -150,8 +148,7 @@ test('[Fetch] Should handle h2 request with body (string or buffer)', async (t) const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) const response = await fetch( @@ -208,12 +205,11 @@ test( server.listen(0) await once(server, 'listening') - const client = new Client(`https://localhost:${server.address().port}`, { - connect: { - rejectUnauthorized: false - }, - allowH2: true - }) + const client = new Client(`https://localhost:${server.address().port}`, { + connect: { + rejectUnauthorized: false + } + }) t.after(closeClientAndServerAsPromise(client, server)) @@ -273,8 +269,7 @@ test('Should handle h2 request with body (Blob)', { skip: !Blob }, async (t) => const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t.after(closeClientAndServerAsPromise(client, server)) @@ -305,7 +300,7 @@ test('Should handle h2 request with body (Blob)', { skip: !Blob }, async (t) => test( 'Should handle h2 request with body (Blob:ArrayBuffer)', { skip: !Blob }, - async (t) => { + async t => { const server = createSecureServer(pem) const expectedBody = 'hello' const requestChunks = [] @@ -339,8 +334,7 @@ test( const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t.after(closeClientAndServerAsPromise(client, server)) @@ -386,8 +380,7 @@ test('Issue#2415', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) const response = await fetch( @@ -438,8 +431,7 @@ test('Issue #2386', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t.after(closeClientAndServerAsPromise(client, server)) @@ -488,8 +480,7 @@ test('Issue #3046', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t.after(closeClientAndServerAsPromise(client, server)) diff --git a/test/http2-alpn.js b/test/http2-alpn.js index 590f65e128e..74a095fc199 100644 --- a/test/http2-alpn.js +++ b/test/http2-alpn.js @@ -58,8 +58,7 @@ test('Should upgrade to HTTP/2 when HTTPS/1 is available for GET', async (t) => connect: { ca, servername: 'agent1' - }, - allowH2: true + } }) // close the client on teardown @@ -205,8 +204,7 @@ test('Should upgrade to HTTP/2 when HTTPS/1 is available for POST', async (t) => connect: { ca, servername: 'agent1' - }, - allowH2: true + } }) // close the client on teardown diff --git a/test/http2.js b/test/http2.js index 83ea5f62cbf..a88bb4f83c9 100644 --- a/test/http2.js +++ b/test/http2.js @@ -35,8 +35,7 @@ test('Should support H2 connection', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 6 }) @@ -87,8 +86,7 @@ test('Should support H2 connection(multiple requests)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 21 }) @@ -142,8 +140,7 @@ test('Should support H2 connection (headers as array)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 7 }) @@ -190,8 +187,7 @@ test('Should support H2 connection(POST Buffer)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 6 }) @@ -237,7 +233,6 @@ test('Should throw if bad maxConcurrentStreams has been passed', async t => { try { // eslint-disable-next-line new Client('https://localhost:1000', { - allowH2: true, maxConcurrentStreams: {} }) t.fail() @@ -251,7 +246,6 @@ test('Should throw if bad maxConcurrentStreams has been passed', async t => { try { // eslint-disable-next-line new Client('https://localhost:1000', { - allowH2: true, maxConcurrentStreams: -1 }) t.fail() @@ -375,8 +369,7 @@ test('Should handle h2 continue', async t => { connect: { rejectUnauthorized: false }, - expectContinue: true, - allowH2: true + expectContinue: true }) after(() => server.close()) @@ -425,8 +418,7 @@ test('Dispatcher#Stream', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -476,8 +468,7 @@ test('Dispatcher#Pipeline', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -537,8 +528,7 @@ test('Dispatcher#Connect', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -586,8 +576,7 @@ test('Dispatcher#Upgrade', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -617,8 +606,7 @@ test('Dispatcher#destroy', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 4 }) @@ -706,8 +694,7 @@ test('Should handle h2 request without body', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -757,8 +744,7 @@ test('Should handle h2 request with body (string or buffer) - dispatch', async t const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -844,8 +830,7 @@ test('Should handle h2 request with body (stream)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -911,8 +896,7 @@ test('Should handle h2 request with body (iterable)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -973,8 +957,7 @@ test('Should handle h2 request with body (Blob)', { skip: !Blob }, async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -1039,8 +1022,7 @@ test( const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -1090,8 +1072,7 @@ test('Agent should support H2 connection', async t => { const client = new Agent({ connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 6 }) @@ -1147,8 +1128,7 @@ test('Should provide pseudo-headers in proper order', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) after(() => server.close()) @@ -1182,8 +1162,7 @@ test('The h2 pseudo-headers is not included in the headers', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 2 }) @@ -1214,8 +1193,7 @@ test('Should throw informational error on half-closed streams (remote)', async t const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 4 }) @@ -1264,8 +1242,7 @@ test('#2364 - Concurrent aborts', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 10 }) @@ -1382,8 +1359,7 @@ test('#2364 - Concurrent aborts (2nd variant)', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 10 }) @@ -1488,8 +1464,7 @@ test('#3046 - GOAWAY Frame', async t => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 7 }) @@ -1542,8 +1517,7 @@ test('#3671 - Graceful close', async (t) => { const client = new Client(`https://localhost:${server.address().port}`, { connect: { rejectUnauthorized: false - }, - allowH2: true + } }) t = tspl(t, { plan: 5 }) @@ -1605,8 +1579,7 @@ test('#3753 - Handle GOAWAY Gracefully', async (t) => { connect: { rejectUnauthorized: false }, - pipelining: 2, - allowH2: true + pipelining: 2 }) t = tspl(t, { plan: 30 }) From e3c9dfd2eea2cef81497e962d2de3f20d57962fd Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 8 Nov 2024 11:14:31 +0000 Subject: [PATCH 2/3] fix: lint --- test/fetch/http2.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/fetch/http2.js b/test/fetch/http2.js index eb44a24b2b4..ef026eaec3b 100644 --- a/test/fetch/http2.js +++ b/test/fetch/http2.js @@ -205,11 +205,11 @@ test( server.listen(0) await once(server, 'listening') - const client = new Client(`https://localhost:${server.address().port}`, { - connect: { - rejectUnauthorized: false - } - }) + const client = new Client(`https://localhost:${server.address().port}`, { + connect: { + rejectUnauthorized: false + } + }) t.after(closeClientAndServerAsPromise(client, server)) From 4a439ba36854d1fb33f60064e2330df72525f907 Mon Sep 17 00:00:00 2001 From: Carlos Fuentes Date: Fri, 22 Nov 2024 10:57:00 +0100 Subject: [PATCH 3/3] docs: adjust documentation --- docs/docs/api/Client.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md index 0fb3519045a..cd7bf14037d 100644 --- a/docs/docs/api/Client.md +++ b/docs/docs/api/Client.md @@ -29,9 +29,20 @@ Returns: `Client` * **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body. * **autoSelectFamily**: `boolean` (optional) - Default: depends on local Node version, on Node 18.13.0 and above is `false`. Enables a family autodetection algorithm that loosely implements section 5 of [RFC 8305](https://tools.ietf.org/html/rfc8305#section-5). See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. This option is ignored if not supported by the current Node version. * **autoSelectFamilyAttemptTimeout**: `number` - Default: depends on local Node version, on Node 18.13.0 and above is `250`. The amount of time in milliseconds to wait for a connection attempt to finish before trying the next address when using the `autoSelectFamily` option. See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. -* **allowH2**: `boolean` - Default: `false`. Enables support for H2 if the server has assigned bigger priority to it through ALPN negotiation. +* **allowH2**: `boolean` - Default: `true`. Enables support for H2 if the server has assigned bigger priority to it through ALPN negotiation. * **maxConcurrentStreams**: `number` - Default: `100`. Dictates the maximum number of concurrent streams for a single H2 session. It can be overridden by a SETTINGS remote frame. +> **Notes about HTTP/2** +> - It only works under TLS connections. h2c is not supported. +> - The server must support HTTP/2 and choose it as the protocol during the ALPN negotiation. +> - The server must not have a bigger priority for HTTP/1.1 than HTTP/2. +> - Pseudo headers are automatically attached to the request. If you try to set them, they will be overwritten. +> - The `:path` header is automatically set to the request path. +> - The `:method` header is automatically set to the request method. +> - The `:scheme` header is automatically set to the request scheme. +> - The `:authority` header is automatically set to the request `host[:port]`. +> - `PUSH` frames are yet not supported. + #### Parameter: `ConnectOptions` Every Tls option, see [here](https://nodejs.org/api/tls.html#tls_tls_connect_options_callback).