undici.request: reject a method that is not a string, not an HTTP token, or CONNECT

src/js/thirdparty/undici.js

@@ -1,7 +1,9 @@
 const EventEmitter = require("node:events");
+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;
@@ -105,6 +107,29 @@ class BodyReadable extends ReadableFromWeb {
   }
 }
 
+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";
+  // 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" (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;
+}
+
 // NOT IMPLEMENTED
 // *   idempotent?: boolean;
 // *   onInfo?: (info: { statusCode: number, headers: Object<string, string | string[]> }) => void;
@@ -187,7 +212,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 +341,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 {}

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,122 @@ 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]() };
+  }
+
+  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
+  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", expectRejection);
+
+  // 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();
+
+    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"],
+  ])("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",
+    });
+  });
+});