From b10d6624b79b536700513c1c9f0c0efdbc7125da Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 8 Oct 2026 12:40:23 +0000 Subject: [PATCH 1/3] undici.request: reject a method that is not a string, not an HTTP token, or CONNECT request() sent GET for a method that is not a string, and for most strings that are not an HTTP token. It sent CONNECT too. undici rejects each of them before it connects, with InvalidArgumentError: - not a string: "method must be a string" - not an HTTP token: "invalid request method" - exactly "CONNECT": "invalid method" request() now makes the same checks before it calls fetch(). InvalidArgumentError gets the name, the code UND_ERR_INVALID_ARG and the default message that it has in undici. A falsy method is still the default. It is now "GET" for the guard of request() too, so a falsy method with a body rejects there and no longer in fetch(). A valid method still goes out in upper case, because fetch() sends GET for a spelling outside its method table. GET, HEAD, POST, PUT, DELETE, OPTIONS and PATCH skip the checks and the case map. --- src/js/thirdparty/undici.js | 35 ++++++- test/js/first_party/undici/undici.test.ts | 117 +++++++++++++++++++++- 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/src/js/thirdparty/undici.js b/src/js/thirdparty/undici.js index cdd08e8f239c..2a4d51f93187 100644 --- a/src/js/thirdparty/undici.js +++ b/src/js/thirdparty/undici.js @@ -1,4 +1,5 @@ const EventEmitter = require("node:events"); +const { checkIsHttpToken } = require("internal/validators"); const { _ReadableFromWeb: ReadableFromWeb } = require("internal/webstreams_adapters"); const ObjectCreate = Object.create; @@ -105,6 +106,29 @@ class BodyReadable extends ReadableFromWeb { } } +// undici checks the method before it connects: +// https://github.com/nodejs/undici/blob/v6.21.3/lib/api/api-request.js#L31-L33 +// https://github.com/nodejs/undici/blob/v6.21.3/lib/core/request.js#L59-L63 +function requestMethod(method) { + // The usual methods need no scan and no case map. + switch (method) { + case "GET": + case "HEAD": + case "POST": + case "PUT": + case "DELETE": + case "OPTIONS": + case "PATCH": + return method; + } + if (!method) return "GET"; + if (method === "CONNECT") throw new InvalidArgumentError("invalid method"); + if (typeof method !== "string") throw new InvalidArgumentError("method must be a string"); + if (!checkIsHttpToken(method)) throw new InvalidArgumentError("invalid request method"); + // fetch() sends GET for a spelling outside its method table (#42497), and the table has no mixed case. + return method.toUpperCase(); +} + // NOT IMPLEMENTED // * idempotent?: boolean; // * onInfo?: (info: { statusCode: number, headers: Object }) => void; @@ -187,7 +211,7 @@ async function request( if (typeof url === "string" && query) url = new URL(url); if (typeof url === "object" && url !== null && query) if (query) url.search = new URLSearchParams(query).toString(); - method = method && typeof method === "string" ? method.toUpperCase() : null; + method = requestMethod(method); // idempotent = idempotent === undefined ? method === "GET" || method === "HEAD" : idempotent; if (inputBody && (method === "GET" || method === "HEAD")) { @@ -316,7 +340,14 @@ class BodyTimeoutError extends UndiciError {} class RequestContentLengthMismatchError extends UndiciError {} class ConnectTimeoutError extends UndiciError {} class ResponseStatusCodeError extends UndiciError {} -class InvalidArgumentError extends UndiciError {} +class InvalidArgumentError extends UndiciError { + constructor(message) { + super(message); + this.name = "InvalidArgumentError"; + this.message = message || "Invalid Argument Error"; + this.code = "UND_ERR_INVALID_ARG"; + } +} class InvalidReturnValueError extends UndiciError {} class RequestAbortedError extends AbortError {} class ClientDestroyedError extends UndiciError {} diff --git a/test/js/first_party/undici/undici.test.ts b/test/js/first_party/undici/undici.test.ts index 59087d28e7ab..0f0a2a1ba939 100644 --- a/test/js/first_party/undici/undici.test.ts +++ b/test/js/first_party/undici/undici.test.ts @@ -1,6 +1,8 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { once } from "node:events"; +import { createServer as createNetServer, type AddressInfo } from "node:net"; import { Readable } from "node:stream"; -import { request, fetch as undiciFetch } from "undici"; +import { errors, request, fetch as undiciFetch } from "undici"; import { createServer } from "../../../http-test-server"; @@ -394,3 +396,116 @@ describe("undici.request maxRedirections", () => { } }); }); + +describe.concurrent("undici.request method", () => { + // Records the request line of every request. It is a raw socket because + // Bun.serve closes the connection for a method outside its table. + async function listen() { + const lines: string[] = []; + const server = createNetServer(socket => { + let head = ""; + socket.on("data", chunk => { + head += chunk; + if (!head.includes("\r\n\r\n")) return; + lines.push(head.slice(0, head.indexOf("\r\n"))); + socket.end("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + }); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const { port } = server.address() as AddressInfo; + return { lines, url: `http://127.0.0.1:${port}/`, [Symbol.asyncDispose]: () => server[Symbol.asyncDispose]() }; + } + + // undici makes these checks before it connects: + // https://github.com/nodejs/undici/blob/v6.21.3/lib/api/api-request.js#L31-L33 + // https://github.com/nodejs/undici/blob/v6.21.3/lib/core/request.js#L59-L63 + it.each([ + ["true", true, "method must be a string"], + ["123", 123, "method must be a string"], + ["1n", 1n, "method must be a string"], + ["a symbol", Symbol("GET"), "method must be a string"], + ["{}", {}, "method must be a string"], + ['["POST"]', ["POST"], "method must be a string"], + ['new String("POST")', new String("POST"), "method must be a string"], + ['"GET POST"', "GET POST", "invalid request method"], + ['" GET"', " GET", "invalid request method"], + ['"GET\\r\\n"', "GET\r\n", "invalid request method"], + ['"a/b"', "a/b", "invalid request method"], + ['"G\\u00e9T"', "G\u00e9T", "invalid request method"], + // toUpperCase() turns U+017F into "S". + ['"po\\u017ft"', "po\u017ft", "invalid request method"], + ['"CONNECT"', "CONNECT", "invalid method"], + ])("rejects %s and sends nothing", async (_, method, message) => { + await using server = await listen(); + + const rejected = request(server.url, { method: method as never }); + await expect(rejected).rejects.toBeInstanceOf(errors.InvalidArgumentError); + await expect(rejected).rejects.toMatchObject({ + name: "InvalidArgumentError", + code: "UND_ERR_INVALID_ARG", + message, + }); + + // The next request is the first one that the server sees. + await (await request(server.url)).body.text(); + expect(server.lines).toEqual(["GET / HTTP/1.1"]); + }); + + async function expectRequestLine(_: string, sent: string, method: unknown) { + await using server = await listen(); + + await (await request(server.url, { method: method as never })).body.text(); + expect(server.lines).toEqual([`${sent} / HTTP/1.1`]); + } + + it.each([ + // A falsy method is no method, as in undici. + ["no method", "GET", undefined], + ["null", "GET", null], + ['""', "GET", ""], + ["0", "GET", 0], + ["false", "GET", false], + ['"POST"', "POST", "POST"], + ['"PROPFIND"', "PROPFIND", "PROPFIND"], + ])("%s goes out as %s", expectRequestLine); + + // undici sends each of these as written. request() goes through fetch(), which + // sends GET for a mixed-case spelling (#42497), so it upper-cases the method. + it.each([ + ['"patch"', "PATCH", "patch"], + ['"Put"', "PUT", "Put"], + ['"Delete"', "DELETE", "Delete"], + // Only exactly "CONNECT" is rejected, as in undici. + ['"connect"', "CONNECT", "connect"], + ['"Connect"', "CONNECT", "Connect"], + ])("unlike undici, %s goes out as %s", expectRequestLine); + + // undici sends PUT for these: https://github.com/nodejs/undici/blob/v6.21.3/index.js#L99 + // request() has no such default. A falsy method is GET, and GET takes no body. + it.each([ + ["null", null], + ['""', ""], + ["0", 0], + ["false", false], + ])("unlike undici, %s with a body rejects as GET with a body and sends nothing", async (_, method) => { + await using server = await listen(); + + await expect(request(server.url, { method: method as never, body: "x" })).rejects.toMatchObject({ + message: "Body not allowed for GET or HEAD requests", + }); + + // The next request is the first one that the server sees. + await (await request(server.url)).body.text(); + expect(server.lines).toEqual(["GET / HTTP/1.1"]); + }); + + it("InvalidArgumentError has undici's name, code and default message", () => { + const error = new errors.InvalidArgumentError(); + expect(error).toBeInstanceOf(errors.UndiciError); + expect(error).toMatchObject({ + name: "InvalidArgumentError", + code: "UND_ERR_INVALID_ARG", + message: "Invalid Argument Error", + }); + }); +}); From d12d79ad4b4c56e5d1699fffa7d01b4673979f9d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:34:46 +0000 Subject: [PATCH 2/3] undici.request: reject every case of CONNECT, upper-case through a captured primordial request() puts the method in upper case, so "connect" and "Connect" went out as CONNECT, the one method that undici never sends from request(). After a 200 answer to that request, the next requests to the same origin go out on the same socket, which a server treats as a tunnel. The CONNECT check now runs on the upper-case method. undici rejects only the exact string and sends another case as written, and the tests name that difference. The case map now calls a String.prototype.toUpperCase that is captured when the module loads, so a replaced prototype method does not change the method that goes out. --- src/js/thirdparty/undici.js | 7 ++-- test/js/first_party/undici/undici.test.ts | 40 +++++++++++++---------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/js/thirdparty/undici.js b/src/js/thirdparty/undici.js index 2a4d51f93187..53ca48eab8c2 100644 --- a/src/js/thirdparty/undici.js +++ b/src/js/thirdparty/undici.js @@ -3,6 +3,7 @@ const { checkIsHttpToken } = require("internal/validators"); const { _ReadableFromWeb: ReadableFromWeb } = require("internal/webstreams_adapters"); const ObjectCreate = Object.create; +const StringPrototypeToUpperCase = String.prototype.toUpperCase; const kEmptyObject = ObjectCreate(null); var fetch = Bun.fetch; @@ -122,11 +123,13 @@ function requestMethod(method) { return method; } if (!method) return "GET"; - if (method === "CONNECT") throw new InvalidArgumentError("invalid method"); if (typeof method !== "string") throw new InvalidArgumentError("method must be a string"); if (!checkIsHttpToken(method)) throw new InvalidArgumentError("invalid request method"); // fetch() sends GET for a spelling outside its method table (#42497), and the table has no mixed case. - return method.toUpperCase(); + method = StringPrototypeToUpperCase.$call(method); + // undici rejects only "CONNECT" and sends "connect" as written. Here every case of it would go out as CONNECT. + if (method === "CONNECT") throw new InvalidArgumentError("invalid method"); + return method; } // NOT IMPLEMENTED diff --git a/test/js/first_party/undici/undici.test.ts b/test/js/first_party/undici/undici.test.ts index 0f0a2a1ba939..849bf209e80d 100644 --- a/test/js/first_party/undici/undici.test.ts +++ b/test/js/first_party/undici/undici.test.ts @@ -416,6 +416,22 @@ describe.concurrent("undici.request method", () => { return { lines, url: `http://127.0.0.1:${port}/`, [Symbol.asyncDispose]: () => server[Symbol.asyncDispose]() }; } + async function expectRejection(_: string, method: unknown, message: string) { + await using server = await listen(); + + const rejected = request(server.url, { method: method as never }); + await expect(rejected).rejects.toBeInstanceOf(errors.InvalidArgumentError); + await expect(rejected).rejects.toMatchObject({ + name: "InvalidArgumentError", + code: "UND_ERR_INVALID_ARG", + message, + }); + + // The next request is the first one that the server sees. + await (await request(server.url)).body.text(); + expect(server.lines).toEqual(["GET / HTTP/1.1"]); + } + // undici makes these checks before it connects: // https://github.com/nodejs/undici/blob/v6.21.3/lib/api/api-request.js#L31-L33 // https://github.com/nodejs/undici/blob/v6.21.3/lib/core/request.js#L59-L63 @@ -435,21 +451,14 @@ describe.concurrent("undici.request method", () => { // toUpperCase() turns U+017F into "S". ['"po\\u017ft"', "po\u017ft", "invalid request method"], ['"CONNECT"', "CONNECT", "invalid method"], - ])("rejects %s and sends nothing", async (_, method, message) => { - await using server = await listen(); - - const rejected = request(server.url, { method: method as never }); - await expect(rejected).rejects.toBeInstanceOf(errors.InvalidArgumentError); - await expect(rejected).rejects.toMatchObject({ - name: "InvalidArgumentError", - code: "UND_ERR_INVALID_ARG", - message, - }); + ])("rejects %s and sends nothing", expectRejection); - // The next request is the first one that the server sees. - await (await request(server.url)).body.text(); - expect(server.lines).toEqual(["GET / HTTP/1.1"]); - }); + // undici sends each of these as written. request() upper-cases the method, so + // it would send CONNECT, and undici never sends CONNECT from request(). + it.each([ + ['"connect"', "connect", "invalid method"], + ['"Connect"', "Connect", "invalid method"], + ])("unlike undici, rejects %s and sends nothing", expectRejection); async function expectRequestLine(_: string, sent: string, method: unknown) { await using server = await listen(); @@ -475,9 +484,6 @@ describe.concurrent("undici.request method", () => { ['"patch"', "PATCH", "patch"], ['"Put"', "PUT", "Put"], ['"Delete"', "DELETE", "Delete"], - // Only exactly "CONNECT" is rejected, as in undici. - ['"connect"', "CONNECT", "connect"], - ['"Connect"', "CONNECT", "Connect"], ])("unlike undici, %s goes out as %s", expectRequestLine); // undici sends PUT for these: https://github.com/nodejs/undici/blob/v6.21.3/index.js#L99 From d27605a643ff9ab457ee51f95ca47fbd7f9c1c95 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:39:02 +0000 Subject: [PATCH 3/3] undici.request: keep each comment of requestMethod() to one line The reference for the type check and the token check sits on the lines that it covers, and the CONNECT comment names its own reference. --- src/js/thirdparty/undici.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/js/thirdparty/undici.js b/src/js/thirdparty/undici.js index 53ca48eab8c2..c840e4ffb899 100644 --- a/src/js/thirdparty/undici.js +++ b/src/js/thirdparty/undici.js @@ -107,9 +107,6 @@ class BodyReadable extends ReadableFromWeb { } } -// undici checks the method before it connects: -// https://github.com/nodejs/undici/blob/v6.21.3/lib/api/api-request.js#L31-L33 -// https://github.com/nodejs/undici/blob/v6.21.3/lib/core/request.js#L59-L63 function requestMethod(method) { // The usual methods need no scan and no case map. switch (method) { @@ -123,11 +120,12 @@ function requestMethod(method) { return method; } if (!method) return "GET"; + // https://github.com/nodejs/undici/blob/v6.21.3/lib/core/request.js#L59-L63 if (typeof method !== "string") throw new InvalidArgumentError("method must be a string"); if (!checkIsHttpToken(method)) throw new InvalidArgumentError("invalid request method"); // fetch() sends GET for a spelling outside its method table (#42497), and the table has no mixed case. method = StringPrototypeToUpperCase.$call(method); - // undici rejects only "CONNECT" and sends "connect" as written. Here every case of it would go out as CONNECT. + // undici rejects only "CONNECT" (lib/api/api-request.js#L31) and sends "connect" as written. Here it would go out as CONNECT. if (method === "CONNECT") throw new InvalidArgumentError("invalid method"); return method; }