From 331d437c64e1485990621af82dccb7a23fa2ea3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 31 Aug 2023 19:33:01 +0200 Subject: [PATCH 01/14] feat: respect Node.js flag --- lib/client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 7d9ec8d7c27..380cc68666a 100644 --- a/lib/client.js +++ b/lib/client.js @@ -6,6 +6,7 @@ const assert = require('assert') const net = require('net') +const http = require('http') const util = require('./core/util') const timers = require('./timers') const Request = require('./core/request') @@ -223,7 +224,7 @@ class Client extends DispatcherBase { this[kConnector] = connect this[kSocket] = null this[kPipelining] = pipelining != null ? pipelining : 1 - this[kMaxHeadersSize] = maxHeaderSize || 16384 + this[kMaxHeadersSize] = maxHeaderSize || http.maxHeaderSize || 16384 this[kKeepAliveDefaultTimeout] = keepAliveTimeout == null ? 4e3 : keepAliveTimeout this[kKeepAliveMaxTimeout] = keepAliveMaxTimeout == null ? 600e3 : keepAliveMaxTimeout this[kKeepAliveTimeoutThreshold] = keepAliveTimeoutThreshold == null ? 1e3 : keepAliveTimeoutThreshold From 26eaf6af5d8a69a9fb246f363f11ccde99b7202f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 31 Aug 2023 19:33:49 +0200 Subject: [PATCH 02/14] add test --- test/client-node-max-header-size.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 test/client-node-max-header-size.js diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js new file mode 100644 index 00000000000..9a879be366a --- /dev/null +++ b/test/client-node-max-header-size.js @@ -0,0 +1,16 @@ +const { execSync } = require("node:child_process"); +const { test } = require("tap"); + +test("Respect --max-http-header-size Node.js flag", (t) => { + const command = + 'node -e "require(`../undici-fetch`);fetch(`https://httpbin.org/get`)"'; + try { + execSync(`${command} --max-http-header-size=1}`, { cwd: __dirname }); + } catch (error) { + t.same(error.message.includes("UND_ERR_HEADERS_OVERFLOW"), true); + } + + execSync(command, { cwd: __dirname }); + + t.end(); +}); From a0eb07a0b54e5cfa88787d948a86ce7e58864675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 1 Sep 2023 13:16:42 +0200 Subject: [PATCH 03/14] cleaner test --- test/client-node-max-header-size.js | 30 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 9a879be366a..f03701c8d09 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -1,16 +1,20 @@ -const { execSync } = require("node:child_process"); -const { test } = require("tap"); +const { execSync } = require('node:child_process') +const { test } = require('tap') -test("Respect --max-http-header-size Node.js flag", (t) => { - const command = - 'node -e "require(`../undici-fetch`);fetch(`https://httpbin.org/get`)"'; - try { - execSync(`${command} --max-http-header-size=1}`, { cwd: __dirname }); - } catch (error) { - t.same(error.message.includes("UND_ERR_HEADERS_OVERFLOW"), true); - } +const command = 'node -e "require(`./index-fetch`);fetch(`https://httpbin.org/get`)"' - execSync(command, { cwd: __dirname }); +test("respect Node.js' --max-http-header-size", async (t) => { + t.throws( + () => execSync(`${command} --max-http-header-size=1`), + /UND_ERR_HEADERS_OVERFLOW/, + 'max-http-header-size=1 should throw' + ) - t.end(); -}); + t.doesNotThrow( + () => execSync(command), + /UND_ERR_HEADERS_OVERFLOW/, + 'default max-http-header-size should not throw' + ) + + t.end() +}) From f245be8764efe41d5e94b2c9fa8ff4245b6ba3f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 1 Sep 2023 13:18:21 +0200 Subject: [PATCH 04/14] fix test --- test/client-node-max-header-size.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index f03701c8d09..4dab12eac85 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -1,7 +1,7 @@ const { execSync } = require('node:child_process') const { test } = require('tap') -const command = 'node -e "require(`./index-fetch`);fetch(`https://httpbin.org/get`)"' +const command = 'node -e "require(\'./index-fetch\');fetch(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( From b222781c45d4790f65127922c86bc6d4b1b60d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 1 Sep 2023 13:20:50 +0200 Subject: [PATCH 05/14] update docs --- docs/api/Client.md | 2 +- types/client.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/Client.md b/docs/api/Client.md index fc7c5d26e8f..4e736a71b21 100644 --- a/docs/api/Client.md +++ b/docs/api/Client.md @@ -22,7 +22,7 @@ Returns: `Client` * **keepAliveMaxTimeout** `number | null` (optional) - Default: `600e3` - The maximum allowed `keepAliveTimeout` when overridden by *keep-alive* hints from the server. Defaults to 10 minutes. * **keepAliveTimeout** `number | null` (optional) - Default: `4e3` - The timeout after which a socket without active requests will time out. Monitors time between activity on a connected socket. This value may be overridden by *keep-alive* hints from the server. See [MDN: HTTP - Headers - Keep-Alive directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive#directives) for more details. Defaults to 4 seconds. * **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `1e3` - A number subtracted from server *keep-alive* hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 1 second. -* **maxHeaderSize** `number | null` (optional) - Default: `16384` - The maximum length of request headers in bytes. Defaults to 16KiB. +* **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB. * **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable. * **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections. * **connect** `ConnectOptions | Function | null` (optional) - Default: `null`. diff --git a/types/client.d.ts b/types/client.d.ts index 56074a15ae7..3c97e0bf39e 100644 --- a/types/client.d.ts +++ b/types/client.d.ts @@ -24,7 +24,7 @@ export declare namespace Client { export interface Options { /** TODO */ interceptors?: OptionsInterceptors; - /** The maximum length of request headers in bytes. Default: `16384` (16KiB). */ + /** The maximum length of request headers in bytes. Default: Node.js' `--max-http-header-size` or `16384` (16KiB). */ maxHeaderSize?: number; /** The amount of time the parser will wait to receive the complete HTTP headers (Node 14 and above only). Default: `300e3` milliseconds (300s). */ headersTimeout?: number; From 9f442d2ed14c7f6b2183eea453c7f5b8123e1c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 1 Sep 2023 13:22:44 +0200 Subject: [PATCH 06/14] revert empty space --- docs/api/Client.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/Client.md b/docs/api/Client.md index 4e736a71b21..c615fea4901 100644 --- a/docs/api/Client.md +++ b/docs/api/Client.md @@ -22,7 +22,7 @@ Returns: `Client` * **keepAliveMaxTimeout** `number | null` (optional) - Default: `600e3` - The maximum allowed `keepAliveTimeout` when overridden by *keep-alive* hints from the server. Defaults to 10 minutes. * **keepAliveTimeout** `number | null` (optional) - Default: `4e3` - The timeout after which a socket without active requests will time out. Monitors time between activity on a connected socket. This value may be overridden by *keep-alive* hints from the server. See [MDN: HTTP - Headers - Keep-Alive directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive#directives) for more details. Defaults to 4 seconds. * **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `1e3` - A number subtracted from server *keep-alive* hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 1 second. -* **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB. +* **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB. * **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable. * **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections. * **connect** `ConnectOptions | Function | null` (optional) - Default: `null`. From 26d7b427cecbb5ce9d6d2b29e40a5c19f52426c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 1 Sep 2023 13:33:41 +0200 Subject: [PATCH 07/14] remove already default value --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 380cc68666a..c04421d32a8 100644 --- a/lib/client.js +++ b/lib/client.js @@ -224,7 +224,7 @@ class Client extends DispatcherBase { this[kConnector] = connect this[kSocket] = null this[kPipelining] = pipelining != null ? pipelining : 1 - this[kMaxHeadersSize] = maxHeaderSize || http.maxHeaderSize || 16384 + this[kMaxHeadersSize] = maxHeaderSize || http.maxHeaderSize this[kKeepAliveDefaultTimeout] = keepAliveTimeout == null ? 4e3 : keepAliveTimeout this[kKeepAliveMaxTimeout] = keepAliveMaxTimeout == null ? 600e3 : keepAliveMaxTimeout this[kKeepAliveTimeoutThreshold] = keepAliveTimeoutThreshold == null ? 1e3 : keepAliveTimeoutThreshold From 295a461c2e0f6348b61b053d6e22fed2dee55975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Sat, 2 Sep 2023 11:52:22 +0200 Subject: [PATCH 08/14] add --- test/client-node-max-header-size.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 4dab12eac85..321d16b89f7 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -1,3 +1,5 @@ +'use strict' + const { execSync } = require('node:child_process') const { test } = require('tap') From cc7388aee10cc828fca278b1962325d724a195a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Sat, 2 Sep 2023 20:31:57 +0200 Subject: [PATCH 09/14] don't use `globalThis.fetch` to pass test in Node.js 16 --- test/client-node-max-header-size.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 321d16b89f7..79916905ab5 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -3,7 +3,7 @@ const { execSync } = require('node:child_process') const { test } = require('tap') -const command = 'node -e "require(\'./index-fetch\');fetch(\'https://httpbin.org/get\')"' +const command = 'node -e "require(\'./index-fetch\').fetch(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( From 417ef095ebe0f364fc2905098d430f9213821b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 5 Sep 2023 14:10:16 +0200 Subject: [PATCH 10/14] add test under `test/fetch` --- test/fetch/client-node-max-header-size.js | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 test/fetch/client-node-max-header-size.js diff --git a/test/fetch/client-node-max-header-size.js b/test/fetch/client-node-max-header-size.js new file mode 100644 index 00000000000..ece3f860048 --- /dev/null +++ b/test/fetch/client-node-max-header-size.js @@ -0,0 +1,24 @@ +'use strict' + +const { execSync } = require('node:child_process') +const { test } = require('tap') + +const command = 'node -e "require(\'.\').fetch(\'https://httpbin.org/get\')"' + +test("respect Node.js' --max-http-header-size", async (t) => { + + + t.throws( + () => execSync(`${command} --max-http-header-size=1`), + /UND_ERR_HEADERS_OVERFLOW/, + 'max-http-header-size=1 should throw' + ) + + t.doesNotThrow( + () => execSync(command), + /UND_ERR_HEADERS_OVERFLOW/, + 'default max-http-header-size should not throw' + ) + + t.end() +}) From f8472b9bb81ff7bf00a4a815f214f7620f603e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 5 Oct 2023 14:43:50 +0200 Subject: [PATCH 11/14] fix lint --- test/fetch/client-node-max-header-size.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/fetch/client-node-max-header-size.js b/test/fetch/client-node-max-header-size.js index ece3f860048..df90c8642de 100644 --- a/test/fetch/client-node-max-header-size.js +++ b/test/fetch/client-node-max-header-size.js @@ -6,8 +6,6 @@ const { test } = require('tap') const command = 'node -e "require(\'.\').fetch(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { - - t.throws( () => execSync(`${command} --max-http-header-size=1`), /UND_ERR_HEADERS_OVERFLOW/, From 772ead89b94c9b8aa9ce70f675d27d1a8cc24849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 5 Oct 2023 14:53:46 +0200 Subject: [PATCH 12/14] import from `undici-fetch` --- test/client-node-max-header-size.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 79916905ab5..9967d8ee015 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -3,7 +3,7 @@ const { execSync } = require('node:child_process') const { test } = require('tap') -const command = 'node -e "require(\'./index-fetch\').fetch(\'https://httpbin.org/get\')"' +const command = 'node -e "require(\'./undici-fetch\').fetch(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( From b56acb97b046334dfe30cd6e081c3aa6947edc43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 5 Oct 2023 15:07:41 +0200 Subject: [PATCH 13/14] use correct paths, use `request` for pure undici test --- test/client-node-max-header-size.js | 2 +- test/fetch/client-node-max-header-size.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 9967d8ee015..b4bd6280249 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -3,7 +3,7 @@ const { execSync } = require('node:child_process') const { test } = require('tap') -const command = 'node -e "require(\'./undici-fetch\').fetch(\'https://httpbin.org/get\')"' +const command = 'node -e "require(\'.\').request(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( diff --git a/test/fetch/client-node-max-header-size.js b/test/fetch/client-node-max-header-size.js index df90c8642de..3668551dc53 100644 --- a/test/fetch/client-node-max-header-size.js +++ b/test/fetch/client-node-max-header-size.js @@ -3,7 +3,7 @@ const { execSync } = require('node:child_process') const { test } = require('tap') -const command = 'node -e "require(\'.\').fetch(\'https://httpbin.org/get\')"' +const command = 'node -e "require(\'./undici-fetch.js\').fetch(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( From 2c9156b3f73c2ed1f068c7691e669a4f67dca984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 5 Oct 2023 17:14:00 +0200 Subject: [PATCH 14/14] fix Node.js 14 test --- test/client-node-max-header-size.js | 3 ++- test/fetch/client-node-max-header-size.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index b4bd6280249..b5374901644 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -7,7 +7,8 @@ const command = 'node -e "require(\'.\').request(\'https://httpbin.org/get\')"' test("respect Node.js' --max-http-header-size", async (t) => { t.throws( - () => execSync(`${command} --max-http-header-size=1`), + // TODO: Drop the `--unhandled-rejections=throw` once we drop Node.js 14 + () => execSync(`${command} --max-http-header-size=1 --unhandled-rejections=throw`), /UND_ERR_HEADERS_OVERFLOW/, 'max-http-header-size=1 should throw' ) diff --git a/test/fetch/client-node-max-header-size.js b/test/fetch/client-node-max-header-size.js index 3668551dc53..432a576b97e 100644 --- a/test/fetch/client-node-max-header-size.js +++ b/test/fetch/client-node-max-header-size.js @@ -7,7 +7,8 @@ const command = 'node -e "require(\'./undici-fetch.js\').fetch(\'https://httpbin test("respect Node.js' --max-http-header-size", async (t) => { t.throws( - () => execSync(`${command} --max-http-header-size=1`), + // TODO: Drop the `--unhandled-rejections=throw` once we drop Node.js 14 + () => execSync(`${command} --max-http-header-size=1 --unhandled-rejections=throw`), /UND_ERR_HEADERS_OVERFLOW/, 'max-http-header-size=1 should throw' )