From 0ad917929e19e5da0382236b5246021d3407c7cd Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 01:51:42 -0500 Subject: [PATCH 1/2] Check http.request options as Node does --- .../src/frontend/lowering/lower-server.ts | 33 ++++++++++++++- .../http-client-request-path-check.ts.json | 6 +++ .../http-request-option-checks.cjs.json | 6 +++ packages/runtime/src/scr_http.c | 41 ++++++++++++------- .../corpus/http-client-request-path-check.ts | 30 ++++++++++++++ tests/corpus/http-request-option-checks.cjs | 34 +++++++++++++++ 6 files changed, 134 insertions(+), 16 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-client-request-path-check.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-option-checks.cjs.json create mode 100644 tests/corpus/http-client-request-path-check.ts create mode 100644 tests/corpus/http-request-option-checks.cjs diff --git a/packages/compiler/src/frontend/lowering/lower-server.ts b/packages/compiler/src/frontend/lowering/lower-server.ts index 7ba3f7191..0da4e3a05 100644 --- a/packages/compiler/src/frontend/lowering/lower-server.ts +++ b/packages/compiler/src/frontend/lowering/lower-server.ts @@ -6499,6 +6499,33 @@ function lowerHttp2ModuleCall( ); } +/** The scalar request options the literal walk lowers, with the type it + * lowers each one at. */ +const LITERAL_REQUEST_OPTION_KINDS: ReadonlyMap = new Map([ + ["hostname", "string"], + ["host", "string"], + ["port", "f64"], + ["path", "string"], + ["method", "string"], + ["timeout", "f64"], + ["maxHeaderSize", "f64"], +]); + +/** Whether a literal's scalar option holds a value of another type than the + * literal walk lowers it at, which sends the literal to the runtime walk. */ +function runtimeCheckedOption( + lowerer: Lowerer, + prop: ts.PropertyAssignment | ts.ShorthandPropertyAssignment, +): boolean { + const want = LITERAL_REQUEST_OPTION_KINDS.get( + (prop.name as ts.Identifier | ts.StringLiteral).text, + ); + if (want === undefined) return false; + const value = ts.isPropertyAssignment(prop) ? prop.initializer : prop.name; + const t = lowerer.mapTypeOf(lowerer.typeOf(value)); + return t !== null && t.kind !== want; +} + /** The checked-dynamic options value of a request whose options the * runtime reads: a non-literal options value (a RequestOptions variable — * checked-dynamic storage — or a record that converts to one), or a @@ -6528,7 +6555,11 @@ function runtimeRequestOptions( (ts.isIdentifier(prop.name) || ts.isStringLiteral(prop.name)) && (prop.name.text === "socketPath" || // a client certificate: the runtime request walk reads cert and key - (secureish && (prop.name.text === "cert" || prop.name.text === "key"))) + (secureish && (prop.name.text === "cert" || prop.name.text === "key")) || + // a scalar option whose value is not the walk's type (an untyped + // or union value, a string port): the runtime walk runs Node's + // checks on it (ERR_INVALID_ARG_TYPE with the option's name) + runtimeCheckedOption(lowerer, prop)) ); }); if (!needsRuntime) return null; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-client-request-path-check.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-client-request-path-check.ts.json new file mode 100644 index 000000000..c3c9bf7e7 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-client-request-path-check.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/http-client-request-path-check.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-option-checks.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-option-checks.cjs.json new file mode 100644 index 000000000..630d70af2 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-option-checks.cjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/http-request-option-checks.cjs" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_http.c b/packages/runtime/src/scr_http.c index 00faefdea..68e08a069 100644 --- a/packages/runtime/src/scr_http.c +++ b/packages/runtime/src/scr_http.c @@ -5158,6 +5158,24 @@ static void scr_http_client_conn_free(ScrHttpConn *conn) { /* ── http.request / http.get ─────────────────────────────────────────── */ +/* ClientRequest's path check: characters outside U+0021..U+00FF throw + * ERR_UNESCAPED_CHARACTERS. In UTF-8, U+0080..U+00FF are the C2/C3 lead + * bytes. */ +static bool scr_http_request_path_valid(const ScrStr *path /*borrowed*/) { + for (size_t i = 0; i < path->len; i++) { + unsigned char ch = (unsigned char)path->data[i]; + if (ch >= 0x21 && ch < 0x80) continue; + if (ch == 0xc2 || ch == 0xc3) { + i++; + continue; + } + static const char msg[] = "Request path contains unescaped characters"; + scr_throw_error_msg_code(SCR_ERR_TYPE, msg, sizeof msg - 1, "ERR_UNESCAPED_CHARACTERS"); + return false; + } + return true; +} + static ScrHttpClientReq *scr_http_request_impl(ScrStr *host /*borrowed*/, double port, ScrStr *path /*borrowed*/, ScrStr *method /*borrowed*/, double timeout_ms, ScrArr *header_pairs /*borrowed*/, @@ -5165,11 +5183,16 @@ static ScrHttpClientReq *scr_http_request_impl(ScrStr *host /*borrowed*/, double ScrHttpRespFn fn, int default_port, void (*wrap)(ScrNetSocket *, void *), void *wrap_ctx, ScrNetSocket *presock /*moves, nullable*/) { - /* Node's ClientRequest constructor validates the method token - * SYNCHRONOUSLY (RFC 9110 tchar, at least one): the catchable + /* Node's ClientRequest constructor validates the path, then the method + * token (RFC 9110 tchar, at least one): the catchable * ERR_INVALID_HTTP_TOKEN TypeError with Node's exact message, the raw * method text inside the quotes. Moved-in arguments release — the * request never exists. */ + if (!scr_http_request_path_valid(path)) { + if (cb) scr_closure_release(cb); + if (presock) scr_net_sock_release(presock); + return NULL; + } bool method_ok = method->len > 0; for (size_t i = 0; i < method->len && method_ok; i++) { unsigned char ch = (unsigned char)method->data[i]; @@ -7435,19 +7458,7 @@ bool scr_http_req_opts_read(const ScrDyn *opts, bool secure, const char *api, Sc scr_http_req_opts_free(o); return false; } - bool bad = false; - for (size_t i = 0; i < o->path->len && !bad; i++) { - unsigned char ch = (unsigned char)o->path->data[i]; - if (ch < 0x21) bad = true; - else if (ch >= 0x80) { - /* U+0080..U+00FF (lead byte C2/C3) pass; anything above is unescaped */ - if (ch != 0xc2 && ch != 0xc3) bad = true; - else i++; - } - } - if (bad) { - static const char msg[] = "Request path contains unescaped characters"; - scr_throw_error_msg_code(SCR_ERR_TYPE, msg, sizeof msg - 1, "ERR_UNESCAPED_CHARACTERS"); + if (!scr_http_request_path_valid(o->path)) { scr_http_req_opts_free(o); return false; } diff --git a/tests/corpus/http-client-request-path-check.ts b/tests/corpus/http-client-request-path-check.ts new file mode 100644 index 000000000..72bdb5d87 --- /dev/null +++ b/tests/corpus/http-client-request-path-check.ts @@ -0,0 +1,30 @@ +// http.request checks the path before it sends anything: characters outside +// U+0021..U+00FF throw ERR_UNESCAPED_CHARACTERS and no request is made. +// Latin-1 characters and DEL pass. +import * as http from "node:http"; + +const paths = ["/plain", "/latinéÿ", "/del\u007f", "/space here", "/tab\there", "/wideĀ", "/emoji\u{1f600}"]; + +const server = http.createServer((req, res) => { + console.log("served", JSON.stringify(req.url)); + res.end(); +}); + +server.listen(0, "127.0.0.1", () => { + const port = (server.address() as { port: number }).port; + for (const path of paths) { + try { + const req = http.request({ host: "127.0.0.1", port, path }); + req.on("error", () => {}); + req.destroy(); + console.log("accepted", JSON.stringify(path)); + } catch (error) { + const e = error as NodeJS.ErrnoException; + console.log("threw", JSON.stringify(path), e.name, e.code, e.message); + } + } + http.get({ host: "127.0.0.1", port, path: "/after" }, (res) => { + res.resume(); + res.on("end", () => server.close()); + }); +}); diff --git a/tests/corpus/http-request-option-checks.cjs b/tests/corpus/http-request-option-checks.cjs new file mode 100644 index 000000000..9ec3eb993 --- /dev/null +++ b/tests/corpus/http-request-option-checks.cjs @@ -0,0 +1,34 @@ +// http.request runs Node's option checks on untyped values in an options +// literal: a method, hostname or host that is not a string throws +// ERR_INVALID_ARG_TYPE naming the option, undefined and null take the +// defaults, and a port string that is not a port throws ERR_SOCKET_BAD_PORT. +'use strict'; +const http = require('http'); + +function attempt(label, fn) { + try { + fn(); + console.log(label, 'ok'); + } catch (e) { + console.log(label, e.name, e.code, e.message); + } +} + +for (const method of [-1, {}, true, []]) { + attempt('method', () => http.request({ method, path: '/' }, () => {})); +} +for (const hostname of [1, [], new Date(), false]) { + attempt('hostname', () => http.request({ hostname })); +} +for (const host of [NaN, {}]) { + attempt('host', () => http.request({ host })); +} +[undefined, null, 'GET', 'post'].forEach((method) => { + attempt(`method ${method}`, () => { + const req = http.request({ method, port: 1, agent: false }); + req.on('error', () => {}); + req.destroy(); + }); +}); +const port = 'x'; +attempt('port', () => http.request({ port })); From f1e431188180764b940d763145721b032bae1e67 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 06:16:58 -0500 Subject: [PATCH 2/2] Check the request path and method before a socket is opened http.request runs the ClientRequest constructor's path and method checks before it calls a createConnection option, takes a pooled socket or dials a socketPath, as Node does. Before, the createConnection callback had already run, and a socketPath server had seen a connection, when the check threw ERR_UNESCAPED_CHARACTERS or ERR_INVALID_HTTP_TOKEN. --- .../http-request-checks-before-socket.ts.json | 6 ++ packages/runtime/src/scr_http.c | 71 ++++++++++++------- .../http-request-checks-before-socket.ts | 59 +++++++++++++++ 3 files changed, 110 insertions(+), 26 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-checks-before-socket.ts.json create mode 100644 tests/corpus/http-request-checks-before-socket.ts diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-checks-before-socket.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-checks-before-socket.ts.json new file mode 100644 index 000000000..b5fdba7ad --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/http-request-checks-before-socket.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/http-request-checks-before-socket.ts" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_http.c b/packages/runtime/src/scr_http.c index 68e08a069..bbc0a71b3 100644 --- a/packages/runtime/src/scr_http.c +++ b/packages/runtime/src/scr_http.c @@ -5176,23 +5176,13 @@ static bool scr_http_request_path_valid(const ScrStr *path /*borrowed*/) { return true; } -static ScrHttpClientReq *scr_http_request_impl(ScrStr *host /*borrowed*/, double port, - ScrStr *path /*borrowed*/, ScrStr *method /*borrowed*/, - double timeout_ms, ScrArr *header_pairs /*borrowed*/, - bool auto_end, ScrClosure *cb /*moves, nullable*/, - ScrHttpRespFn fn, int default_port, - void (*wrap)(ScrNetSocket *, void *), void *wrap_ctx, - ScrNetSocket *presock /*moves, nullable*/) { - /* Node's ClientRequest constructor validates the path, then the method - * token (RFC 9110 tchar, at least one): the catchable - * ERR_INVALID_HTTP_TOKEN TypeError with Node's exact message, the raw - * method text inside the quotes. Moved-in arguments release — the - * request never exists. */ - if (!scr_http_request_path_valid(path)) { - if (cb) scr_closure_release(cb); - if (presock) scr_net_sock_release(presock); - return NULL; - } +/* The ClientRequest constructor's checks that come before any socket + * exists, in Node's order: the path (ERR_UNESCAPED_CHARACTERS), then the + * method token (RFC 9110 tchar, at least one; ERR_INVALID_HTTP_TOKEN with + * the raw method text inside the quotes). False with the exception + * pending. */ +static bool scr_http_request_checks(const ScrStr *path /*borrowed*/, const ScrStr *method /*borrowed*/) { + if (!scr_http_request_path_valid(path)) return false; bool method_ok = method->len > 0; for (size_t i = 0; i < method->len && method_ok; i++) { unsigned char ch = (unsigned char)method->data[i]; @@ -5200,17 +5190,30 @@ static ScrHttpClientReq *scr_http_request_impl(ScrStr *host /*borrowed*/, double (ch >= '0' && ch <= '9') || (ch != 0 && strchr("!#$%&'*+-.^_`|~", ch) != NULL); } - if (!method_ok) { + if (method_ok) return true; + ScrJsonBuf b; + scr_jb_init(&b); + scr_jb_puts(&b, "Method must be a valid HTTP token [\""); + scr_jb_write(&b, method->data, method->len); + scr_jb_puts(&b, "\"]"); + ScrStr *msg = scr_jb_finish(&b); + scr_throw_error_msg_code(SCR_ERR_TYPE, msg->data, msg->len, "ERR_INVALID_HTTP_TOKEN"); + scr_str_release(msg); + return false; +} + +static ScrHttpClientReq *scr_http_request_impl(ScrStr *host /*borrowed*/, double port, + ScrStr *path /*borrowed*/, ScrStr *method /*borrowed*/, + double timeout_ms, ScrArr *header_pairs /*borrowed*/, + bool auto_end, ScrClosure *cb /*moves, nullable*/, + ScrHttpRespFn fn, int default_port, + void (*wrap)(ScrNetSocket *, void *), void *wrap_ctx, + ScrNetSocket *presock /*moves, nullable*/) { + /* Node's ClientRequest constructor checks; moved-in arguments release + * when they fail — the request never exists. */ + if (!scr_http_request_checks(path, method)) { if (cb) scr_closure_release(cb); if (presock) scr_net_sock_release(presock); - ScrJsonBuf b; - scr_jb_init(&b); - scr_jb_puts(&b, "Method must be a valid HTTP token [\""); - scr_jb_write(&b, method->data, method->len); - scr_jb_puts(&b, "\"]"); - ScrStr *msg = scr_jb_finish(&b); - scr_throw_error_msg_code(SCR_ERR_TYPE, msg->data, msg->len, "ERR_INVALID_HTTP_TOKEN"); - scr_str_release(msg); return NULL; } scr_http_install(); @@ -5311,6 +5314,12 @@ ScrHttpClientReq *scr_http_request_conn(ScrClosure *conn_cb /*moves*/, ScrStr *p ScrStr *method /*borrowed*/, double timeout_ms, ScrArr *header_pairs /*borrowed*/, bool auto_end, ScrClosure *cb /*moves, nullable*/, ScrHttpRespFn fn) { + /* Node checks the path and method before it calls createConnection */ + if (!scr_http_request_checks(path, method)) { + scr_closure_release(conn_cb); + if (cb != NULL) scr_closure_release(cb); + return NULL; + } ScrNetSocket *sock = ((ScrNetSocket *(*)(ScrClosure *))conn_cb->fn)(conn_cb); /* +1 result */ scr_closure_release(conn_cb); if (scr_exc_pending() || sock == NULL) { @@ -5832,6 +5841,11 @@ static ScrHttpClientReq *scr_http_request_pooled(ScrHttpAgent *ag, ScrStr *host void (*wrap)(ScrNetSocket *, void *), void *wrap_ctx, ScrStr *pool_key /*borrowed*/, ScrStr *socket_path /*borrowed*/) { + /* the constructor's checks come before the agent takes or dials a socket */ + if (!scr_http_request_checks(path, method)) { + if (cb) scr_closure_release(cb); + return NULL; + } double p = port >= 0 ? port : ag->default_port > 0 ? ag->default_port : (double)default_port; @@ -5892,6 +5906,11 @@ ScrHttpClientReq *scr_http_request_agent_ex(ScrStr *host /*borrowed*/, double po /* agent: false — Node's one-shot Agent: exactly this client's model, * plus its Connection: close request header (unless the caller set one) */ if (agent->kind == SCR_DYN_BOOL && !agent->v.b) { + /* the constructor's checks come before a Unix socket is dialed */ + if (!scr_http_request_checks(path, method)) { + if (cb) scr_closure_release(cb); + return NULL; + } double p = port >= 0 ? port : (double)default_port; bool has_conn = false; size_t npairs = (size_t)scr_arr_len(header_pairs); diff --git a/tests/corpus/http-request-checks-before-socket.ts b/tests/corpus/http-request-checks-before-socket.ts new file mode 100644 index 000000000..868871db1 --- /dev/null +++ b/tests/corpus/http-request-checks-before-socket.ts @@ -0,0 +1,59 @@ +// http.request checks the path and the method before it opens a socket, as +// Node's ClientRequest constructor does: createConnection is not called and +// a socketPath server sees no connection when either check throws. +import * as http from "node:http"; +import * as net from "node:net"; +import * as os from "node:os"; +import * as path from "node:path"; + +function attempt(label: string, fn: () => void): void { + try { + fn(); + console.log(label, "no throw"); + } catch (error) { + const e = error as NodeJS.ErrnoException; + console.log(label, e.name, e.code); + } +} + +let connections = 0; +const socketPath = path.join(os.tmpdir(), `scriptc-checks-${process.pid}.sock`); +const server = net.createServer((socket) => { + connections++; + socket.destroy(); +}); + +server.listen(socketPath, () => { + attempt("createConnection path", () => { + http.request({ + path: "/a b", + createConnection: () => { + console.log("createConnection called"); + return net.connect(socketPath); + }, + }); + }); + attempt("createConnection method", () => { + http.request({ + path: "/", + method: "BAD METHOD", + createConnection: () => { + console.log("createConnection called"); + return net.connect(socketPath); + }, + }); + }); + attempt("socketPath path", () => { + http.request({ socketPath, path: "/a\tb" }); + }); + attempt("socketPath agent false path", () => { + http.request({ socketPath, path: "/a b", agent: false }); + }); + attempt("socketPath method", () => { + http.request({ socketPath, path: "/", method: "A(B" }); + }); + setTimeout(() => { + console.log("connections", connections); + server.close(() => console.log("closed")); + }, 200); +});