Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { expect, it } from "vitest";

import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js";

/**
* `http.request` and `https.request` validate `options.method` with `checkIsHttpToken`
* before they build the request, so a literal outside the token set throws
* `ERR_INVALID_HTTP_TOKEN` from the call itself and no request exists.
*
* Measured on Node 24.22.1: `"GET"`, `"get"`, `"M-SEARCH"`, `"X"` and the punctuation set
* `"!#$%&'*+-.^_`|~"` all construct a request; `"BAD METHOD"`, `"G/ET"`, `"G:ET"`,
* `"G,ET"`, `"GET "`, `" "` and `"GéT"` throw. An empty string is accepted, because Node
* reads `""` as "not given" and falls back to `GET`.
*/
const requestsFor = (body: string) =>
analyzeJavaScriptSemantics(
`import http from "node:http";\nimport https from "node:https";\nfunction f() { ${body}; }`,
).requestOperations;

it.each([
[`http.request({ host: "h", path: "/", method: "BAD METHOD" })`],
[`http.request({ host: "h", path: "/", method: "G/ET" })`],
[`http.request({ host: "h", path: "/", method: "G:ET" })`],
[`http.request({ host: "h", path: "/", method: "GET " })`],
[`http.request({ host: "h", path: "/", method: " " })`],
[`http.request({ host: "h", path: "/", method: "GéT" })`],
[`https.request({ host: "h", path: "/", method: "BAD METHOD" })`],
[`http.get({ host: "h", path: "/", method: "BAD METHOD" })`],
] as const)("emits no request for a rejected method token: %s", (call) => {
expect(requestsFor(call)).toEqual([]);
});

it.each([
[`http.request({ host: "h", path: "/", method: "GET" })`],
[`http.request({ host: "h", path: "/", method: "get" })`],
[`http.request({ host: "h", path: "/", method: "M-SEARCH" })`],
[`http.request({ host: "h", path: "/", method: "X" })`],
// Node reads an empty method as "not given" and falls back to GET.
[`http.request({ host: "h", path: "/", method: "" })`],
[`http.request({ host: "h", path: "/" })`],
// `fetch` reads a different member and validates no token.
[`fetch("http://h/", { method: "BAD METHOD" })`],
// A binding is not a literal, so this file leaves the answer to the runtime.
[`const m = "BAD METHOD"; http.request({ host: "h", method: m })`],
] as const)("keeps the request for an accepted method: %s", (call) => {
expect(requestsFor(call)).toHaveLength(1);
});

it.each([
// An erased TypeScript assertion is the same object at runtime, so the method is still
// validated. Measured on Node 24.22.1, this throws ERR_INVALID_HTTP_TOKEN.
[
`http.request(({ host: "h", method: "BAD METHOD" } as any))`,
`http.request(({ host: "h", method: "BAD METHOD" }) as any)`,
],
] as const)(
"reads the options through an erased assertion: %s",
(_name, call) => {
expect(
analyzeJavaScriptSemantics(
`import http from "node:http";\nfunction f() { ${call}; }`,
).requestOperations,
).toEqual([]);
},
);

it("keeps the request when a leading spread shifts the options slot", () => {
// The spread supplies the options and the callback, so the trailing object is ignored
// and Node builds a GET request. Measured on Node 24.22.1.
const operations = analyzeJavaScriptSemantics(
`import http from "node:http";\nfunction f() { const a = [{ host: "h" }, () => {}]; http.request(...a, { method: "BAD METHOD" }); }`,
).requestOperations;
expect(operations).toHaveLength(1);
});

it.each([
// A callable first argument is the callback, so Node ignores a later options object.
[`http.request(() => {}, { method: "BAD METHOD" })`],
[`http.request(function () {}, { method: "BAD METHOD" })`],
// A getter runs before the copy and can rewrite the method.
[`http.request({ get x() { this.method = "GET"; }, method: "BAD METHOD" })`],
] as const)(
"keeps the request for a callable or accessor-written options: %s",
(call) => {
expect(requestsFor(call)).toHaveLength(1);
},
);
76 changes: 76 additions & 0 deletions src/domain/javascript/javascriptSemanticDataEffects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
javaScriptExpressionWrapperTarget,
readExactJavaScriptLiteral,
semanticStaticPropertyKey,
staticObjectProperty,
semanticStaticPropertyName,
unwrapJavaScriptExpression,
} from "./javascriptAstValues.js";
Expand Down Expand Up @@ -271,6 +272,9 @@ const collectRequests = (
!URL.canParse(endpoint)
)
return;
// `http`/`https` validate `options.method` with `checkIsHttpToken` before the request
// exists, so a literal outside the token set throws from the call itself.
if (rejectsHttpMethodLiteral(node, method)) return;
output.push({
node,
operation: {
Expand Down Expand Up @@ -579,6 +583,78 @@ const requestFields = (
});
};

/**
* The characters RFC 9110 allows in a method token, which is what Node's
* `checkIsHttpToken` accepts.
*
* Measured on Node 24.22.1, `method: "M-SEARCH"`, `"X"` and the punctuation set
* `"!#$%&'*+-.^_` | "~"` all construct a request, while `"G/ET"`, `"G:ET"`, `"G(ET"`,
* `"G,ET"`, `"G"ET"`, `"GéT"`, `" "` and `"\\t"` throw `ERR_INVALID_HTTP_TOKEN`. The set is
* a closed character class rather than a list of verbs, so `"GET "` fails on the trailing
* space and `"get"` is accepted.
*/
const HTTP_TOKEN = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]*$/u;

/**
* Whether a literal `options.method` makes `http.request` throw before any request exists.
*
* `http` and `https` validate the method with `checkIsHttpToken` before they build the
* request, so a literal outside `HTTP_TOKEN` throws `ERR_INVALID_HTTP_TOKEN` from the call
* itself. Measured on Node 24.22.1, an empty string is accepted: Node treats `""` as "not
* given" and falls back to `GET`. A non-string value throws `ERR_INVALID_ARG_TYPE` too, but
* it is left undecided here because the reported fields do not describe the type check.
*/
const rejectsHttpMethodLiteral = (
node: t.CallExpression | t.OptionalCallExpression | t.NewExpression,
method: JavaScriptSemanticRequestOperation["method"],
): boolean => {
// `fetch` and `WebSocket` read a different member and do not validate a token.
if (method !== "get" && method !== "request") return false;
// A spread before the options slot can supply several runtime arguments, so a later
// syntactic object is not necessarily the options Node reads. Measured on Node
// 24.22.1, `http.request(...[{host, path}, () => {}], {method: "BAD METHOD"})` returns
// a GET request, so the trailing object is ignored and the call must be kept.
if (node.arguments.some((argument) => t.isSpreadElement(argument)))
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
return false;
// A callable first argument is the callback, so Node replaces the options with defaults
// and never reads a later object. Measured on Node 24.22.1,
// `http.request(() => {}, {method: "BAD METHOD"})` returns a GET ClientRequest.
if (
node.arguments.some(
(argument) =>
t.isArrowFunctionExpression(argument) ||
t.isFunctionExpression(argument),
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
)
)
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
return false;
// The value is unwrapped before the slot is chosen, because an erased TypeScript
// assertion is the same object at runtime: measured on Node 24.22.1,
// `http.request({method: "BAD METHOD"} as any)` throws ERR_INVALID_HTTP_TOKEN.
const first = node.arguments[0];
const firstValue =
first === undefined ? undefined : unwrapJavaScriptExpression(first).node;
const options =
firstValue !== undefined && t.isObjectExpression(firstValue)
? firstValue
: unwrapJavaScriptExpression(node.arguments[1] ?? first!).node;
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
Comment on lines +710 to +713

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain HTTPS requests when the leading input is falsy

A falsy HTTPS head is fresh evidence beyond the handled callback overload: for https.request(null, { method: "BAD METHOD" }) (likewise false, 0, or undefined), Node's HTTPS wrapper leaves its default options object in place and ignores argument 1, so it constructs a GET ClientRequest. This ternary instead selects argument 1 and removes that real request; preserve the call unless slot 1 is proven to be the HTTP/HTTPS options slot, or carry the module-specific overload identity into this decision.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

if (!t.isObjectExpression(options)) return false;
// `ClientRequest` copies the options with `Object.assign`, so a getter runs first and can
// rewrite a later member. Measured on Node 24.22.1,
// `http.request({get x() { this.method = "GET"; }, method: "BAD METHOD"})` returns a GET
// request.
if (
options.properties.some(
(property) => t.isObjectMethod(property) && property.kind === "get",
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
)
)
return false;
const expression = options;
const lookup = staticObjectProperty(expression, "method");
Comment thread
XiaoBinGan marked this conversation as resolved.
Resolved
if (lookup.status !== "explicit") return false;
const literal = readExactJavaScriptLiteral(lookup.value);
if (!literal.found || typeof literal.value !== "string") return false;
return literal.value !== "" && !HTTP_TOKEN.test(literal.value);
};
const requestEndpoint = (
node: t.CallExpression | t.OptionalCallExpression | t.NewExpression,
): string | null =>
Expand Down
Loading