From 667032e4d33a1331181b8a99f9c6ee9e8f0b0958 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Thu, 2 Jul 2026 13:24:34 +0000 Subject: [PATCH 1/2] wip --- src/adapters/_node/headers.ts | 105 +++++++++++++++++++++++++++++++-- src/adapters/_node/request.ts | 16 ++++- src/adapters/_node/response.ts | 22 +++---- src/adapters/_node/send.ts | 67 +++++++++++++++++---- src/adapters/node.ts | 8 ++- 5 files changed, 184 insertions(+), 34 deletions(-) diff --git a/src/adapters/_node/headers.ts b/src/adapters/_node/headers.ts index 50965ebb..eb446155 100644 --- a/src/adapters/_node/headers.ts +++ b/src/adapters/_node/headers.ts @@ -3,6 +3,51 @@ import { lazyInherit } from "../../_inherit.ts"; // https://github.com/nodejs/node/blob/main/lib/_http_incoming.js +/** + * Header names Node.js treats as single-value: repeats keep only the FIRST + * occurrence in `req.headers`, diverging from WHATWG Headers ", " join + * semantics. (https://nodejs.org/api/http.html#messageheaders — `set-cookie` + * stays an array and `cookie` repeats join with "; " in both Node and the + * Fetch spec, so neither needs a fallback.) + */ +const _nonJoinedHeaders = /* @__PURE__ */ new Set([ + "age", + "authorization", + "content-length", + "content-type", + "etag", + "expires", + "from", + "host", + "if-modified-since", + "if-unmodified-since", + "last-modified", + "location", + "max-forwards", + "proxy-authorization", + "referer", + "retry-after", + "server", + "user-agent", +]); + +// WHATWG header name token (RFC 9110 field-name) +const _validHeaderNameRE = /^[!#$%&'*+\-.^_`|~\dA-Za-z]+$/; + +function _isRepeated(rawHeaders: string[], lowerName: string): boolean { + let seen = false; + for (let i = 0; i < rawHeaders.length; i += 2) { + const key = rawHeaders[i]; + if (key.length === lowerName.length && key.toLowerCase() === lowerName) { + if (seen) { + return true; + } + seen = true; + } + } + return false; +} + export type NodeRequestHeaders = InstanceType; export const NodeRequestHeaders: { @@ -41,18 +86,66 @@ export const NodeRequestHeaders: { } get(name: string): string | null { - // Always read from the rawHeaders-materialized Headers: Node collapses - // headers it treats as single-value (authorization, content-type, …) to - // their first occurrence in `req.headers`, which diverges from WHATWG. - return this._headers.get(name); + if (this.#headers) { + return this.#headers.get(name); + } + const lower = name.toLowerCase(); + if (lower.charCodeAt(0) === 58 /* : */) { + // HTTP/2 pseudo-header: invalid WHATWG name → native TypeError + return this._headers.get(name); + } + const value = this.#req.headers[lower]; + if (typeof value === "string") { + // Node collapses repeated single-value headers (authorization, + // content-type, …) to their first occurrence in `req.headers`, + // diverging from WHATWG ", " join semantics. Deopt to the + // rawHeaders-materialized Headers only when such a header actually + // repeats. + return _nonJoinedHeaders.has(lower) && _isRepeated(this.#req.rawHeaders, lower) + ? this._headers.get(name) + : value; + } + if (Array.isArray(value)) { + // Only set-cookie is array-valued in `req.headers` + return value.join(", "); + } + // Absent, or a non-string artifact from `req.headers`'s prototype + // (`toString`, …). A real `__proto__` header never lands as an own key + // (the prototype accessor swallows Node's assignment) and invalid names + // need native error semantics — both read from the materialized Headers. + return lower !== "__proto__" && _validHeaderNameRE.test(name) + ? null + : this._headers.get(name); } has(name: string): boolean { - return this._headers.has(name); + if (this.#headers) { + return this.#headers.has(name); + } + const lower = name.toLowerCase(); + if (lower.charCodeAt(0) === 58 /* : */) { + // HTTP/2 pseudo-header: invalid WHATWG name → native TypeError + return this._headers.has(name); + } + // Presence is unaffected by Node's duplicate collapsing/joining. + // `hasOwn` guards against `req.headers` prototype hits (`toString`, …). + if (Object.hasOwn(this.#req.headers, lower)) { + return true; + } + // `__proto__` never lands as an own key (see get()); invalid names need + // native error semantics. + return lower !== "__proto__" && _validHeaderNameRE.test(name) + ? false + : this._headers.has(name); } getSetCookie(): string[] { - return this._headers.getSetCookie(); + if (this.#headers) { + return this.#headers.getSetCookie(); + } + // Node always materializes set-cookie as an array of every occurrence. + const value = this.#req.headers["set-cookie"]; + return Array.isArray(value) ? value.slice() : value ? [value] : []; } entries(): HeadersIterator<[string, string]> { diff --git a/src/adapters/_node/request.ts b/src/adapters/_node/request.ts index 3613bd53..6d431653 100644 --- a/src/adapters/_node/request.ts +++ b/src/adapters/_node/request.ts @@ -24,6 +24,10 @@ export const NodeRequest: { class Request implements Partial { runtime: ServerRequest["runtime"]; + // Declared so the post-construction `request.waitUntil = ...` assignment + // in the adapters doesn't add a property (hidden-class transition) per + // request. + waitUntil?: ServerRequest["waitUntil"]; #req: NodeServerRequest; #url?: URL; @@ -148,7 +152,14 @@ export const NodeRequest: { if (this.#request) { return this.#request.json(); } - return this.text().then((text) => JSON.parse(text)); + if (this.#bodyStream !== undefined) { + return this.text().then((text) => JSON.parse(text)); + } + // Parse in a single continuation (readBody -> parse) instead of going + // through text() — one less promise + microtask hop per body read. + return readBody(this.#req, this.#maxRequestBodySize).then((buf) => + JSON.parse(buf.toString()), + ); } get _request(): globalThis.Request { @@ -257,7 +268,8 @@ function readBody(req: NodeServerRequest, maxRequestBodySize?: number): Promise< }; const onEnd = () => { cleanup(); - resolve(Buffer.concat(chunks)); + // Single-chunk bodies (the common case) skip Buffer.concat's alloc+copy + resolve(chunks.length === 1 ? chunks[0] : Buffer.concat(chunks)); }; req.on("data", onData).once("end", onEnd).once("error", onError); }); diff --git a/src/adapters/_node/response.ts b/src/adapters/_node/response.ts index f0c27b2a..50a7388a 100644 --- a/src/adapters/_node/response.ts +++ b/src/adapters/_node/response.ts @@ -8,7 +8,8 @@ export type PreparedNodeResponseBody = string | Buffer | Uint8Array | DataView | export interface PreparedNodeResponse { status: number; statusText: string; - headers: [string, string][]; + /** Flat rawHeaders-style list: `[name1, value1, name2, value2, …]` */ + headers: string[]; body: PreparedNodeResponseBody; } @@ -145,8 +146,8 @@ export const NodeResponse: { } } - // Headers - const headers: [string, string][] = []; + // Headers (flat rawHeaders-style list — avoids a per-response flatten in writeHead) + const headers: string[] = []; const initHeaders = this.#init?.headers; const headerEntries = this.#response?.headers || @@ -156,21 +157,22 @@ export const NodeResponse: { ? initHeaders : initHeaders?.entries ? (initHeaders as Headers).entries() - : // prettier-ignore - Object.entries(initHeaders).map(([k, v]) => [k.toLowerCase(), v]) + : Object.entries(initHeaders) : undefined); let hasContentTypeHeader: boolean | undefined; let hasContentLength: boolean | undefined; if (headerEntries) { for (const [key, value] of headerEntries) { + // Normalize names once: enables case-insensitive content-type / + // content-length dedup and matches native Response header casing. + const lowerKey = typeof key === "string" ? key.toLowerCase() : String(key); if (Array.isArray(value)) { for (const v of value) { - headers.push([key, v]); + headers.push(lowerKey, v); } } else { - headers.push([key, value]); + headers.push(lowerKey, value); } - const lowerKey = typeof key === "string" ? key.toLowerCase() : key; if (lowerKey === "content-type") { hasContentTypeHeader = true; } else if (lowerKey === "content-length") { @@ -179,10 +181,10 @@ export const NodeResponse: { } } if (contentType && !hasContentTypeHeader) { - headers.push(["content-type", contentType]); + headers.push("content-type", contentType); } if (contentLength && !hasContentLength) { - headers.push(["content-length", String(contentLength)]); + headers.push("content-length", String(contentLength)); } // Free up memory diff --git a/src/adapters/_node/send.ts b/src/adapters/_node/send.ts index c3acea64..2ffed757 100644 --- a/src/adapters/_node/send.ts +++ b/src/adapters/_node/send.ts @@ -4,13 +4,47 @@ import type NodeHttp from "node:http"; import type { NodeServerResponse } from "../../types.ts"; import type { NodeResponse } from "./response.ts"; -export async function sendNodeResponse( +/** + * Sends a web `Response` to a Node.js `ServerResponse`. + * + * The returned promise resolves once the response has been fully sent + * (kept for `toNodeHandler` consumers that await completion). + */ +export function sendNodeResponse( nodeRes: NodeServerResponse, webRes: Response | NodeResponse, ): Promise { + try { + return _sendNodeResponse(nodeRes, webRes, false) || Promise.resolve(); + } catch (error) { + return Promise.reject(error); + } +} + +/** + * Fire-and-forget variant for the internal `serve()` path: node:http ignores + * the request listener's return value, so tracking `end()` completion with a + * per-response Promise (and the microtask hops to settle it) is pure overhead + * there. Streaming bodies still return their tracking promise (it drives + * their own cleanup). + * + * @internal + */ +export function sendNodeResponseDetached( + nodeRes: NodeServerResponse, + webRes: Response | NodeResponse, +): Promise | void { + return _sendNodeResponse(nodeRes, webRes, true); +} + +function _sendNodeResponse( + nodeRes: NodeServerResponse, + webRes: Response | NodeResponse, + detached: boolean, +): Promise | void { if (!webRes) { nodeRes.statusCode = 500; - return endNodeResponse(nodeRes); + return endNodeResponse(nodeRes, detached); } // Fast path for NodeResponse @@ -31,39 +65,46 @@ export async function sendNodeResponse( } else { writeHead(nodeRes, res.status, res.statusText, res.headers); } - return endNodeResponse(nodeRes); + return endNodeResponse(nodeRes, detached); } - const rawHeaders = [...webRes.headers]; + const rawHeaders: string[] = []; + for (const [key, value] of webRes.headers) { + rawHeaders.push(key, value); + } writeHead(nodeRes, webRes.status, webRes.statusText, rawHeaders); - return webRes.body ? streamBody(webRes.body, nodeRes) : endNodeResponse(nodeRes); + return webRes.body ? streamBody(webRes.body, nodeRes) : endNodeResponse(nodeRes, detached); } function writeHead( nodeRes: NodeServerResponse, status: number, statusText: string, - rawHeaders: [string, string][], -): void { // Node.js writeHead accepts a raw array of [key, value, key, value] or [[key, value], [key, value]] // https://github.com/nodejs/node/blob/v22.14.0/lib/_http_server.js#L376 // https://github.com/nodejs/node/blob/v24.10.0/lib/_http_outgoing.js#L417 // But it has an inconsistency in slow-path that does not unflattens!! // https://github.com/h3js/srvx/pull/40 - const writeHeaders = rawHeaders.flat(); + // We always pass the (safe) flat form, pre-built to avoid a per-response flatten. + rawHeaders: string[], +): void { if (!nodeRes.headersSent) { if (nodeRes.req?.httpVersion === "2.0") { // @ts-expect-error - nodeRes.writeHead(status, writeHeaders); + nodeRes.writeHead(status, rawHeaders); } else { // @ts-expect-error - nodeRes.writeHead(status, statusText, writeHeaders); + nodeRes.writeHead(status, statusText, rawHeaders); } } } -function endNodeResponse(nodeRes: NodeServerResponse) { +function endNodeResponse(nodeRes: NodeServerResponse, detached?: boolean): Promise | void { + if (detached) { + nodeRes.end(); + return; + } return new Promise((resolve) => nodeRes.end(resolve)); } @@ -72,7 +113,7 @@ function pipeBody( nodeRes: NodeServerResponse, status: number, statusText: string, - headers: [string, string][], + headers: string[], ): Promise | void { if (nodeRes.destroyed) { stream.destroy?.(); @@ -103,7 +144,7 @@ function pipeBody( stream.off("readable", onReadable); stream.destroy(); writeHead(nodeRes, 500, "Internal Server Error", []); - endNodeResponse(nodeRes).then(resolve); + (endNodeResponse(nodeRes) as Promise).then(resolve); } function onReadable() { stream.off("error", onEarlyError); diff --git a/src/adapters/node.ts b/src/adapters/node.ts index f3172e63..55004a94 100644 --- a/src/adapters/node.ts +++ b/src/adapters/node.ts @@ -1,4 +1,4 @@ -import { sendNodeResponse } from "./_node/send.ts"; +import { sendNodeResponseDetached } from "./_node/send.ts"; import { NodeRequest } from "./_node/request.ts"; import { fmtURL, @@ -82,9 +82,11 @@ class NodeServer implements Server { }); request.waitUntil = this.#wait?.waitUntil; const res = fetchHandler(request); + // node:http ignores the listener's return value — use the detached + // variant to skip the per-response end-tracking Promise. return res instanceof Promise - ? res.then((resolvedRes) => sendNodeResponse(nodeRes, resolvedRes)) - : sendNodeResponse(nodeRes, res); + ? res.then((resolvedRes) => sendNodeResponseDetached(nodeRes, resolvedRes)) + : sendNodeResponseDetached(nodeRes, res); }; this.node = { handler, server: undefined }; From 7af2b9a65eda2ca30dd913f6093c3d65bc229a19 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Thu, 2 Jul 2026 13:38:53 +0000 Subject: [PATCH 2/2] fix(node): guard detached send path against sync serialization throws The internal serve() path switched from the async `sendNodeResponse` to the synchronous `sendNodeResponseDetached`. The async variant implicitly turned a synchronous throw during serialization (e.g. an invalid header value in `writeHead`) into a swallowed rejected promise; the detached variant let it escape the node:http request listener as an `uncaughtException`, crashing the process. Guard `sendNodeResponseDetached` so a serialization throw fails the single response (500 if not yet committed, otherwise destroy the socket) instead of taking the server down. Also dedupe the buffered-body read shared by `text()`/`json()` behind a `#readBuffered()` helper, preserving json()'s single-continuation parse. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/adapters/_node/request.ts | 13 +++++++++---- src/adapters/_node/send.ts | 21 ++++++++++++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/adapters/_node/request.ts b/src/adapters/_node/request.ts index 6d431653..b51b9469 100644 --- a/src/adapters/_node/request.ts +++ b/src/adapters/_node/request.ts @@ -138,6 +138,13 @@ export const NodeRequest: { return this.#bodyStream; } + // Buffer the raw request body once; consumers add their own single + // continuation (`.toString()` / `JSON.parse`) so no extra promise or + // microtask hop is introduced vs. inlining the read. + #readBuffered() { + return readBody(this.#req, this.#maxRequestBodySize); + } + text() { if (this.#request) { return this.#request.text(); @@ -145,7 +152,7 @@ export const NodeRequest: { if (this.#bodyStream !== undefined) { return this.#bodyStream ? new Response(this.#bodyStream).text() : Promise.resolve(""); } - return readBody(this.#req, this.#maxRequestBodySize).then((buf) => buf.toString()); + return this.#readBuffered().then((buf) => buf.toString()); } json() { @@ -157,9 +164,7 @@ export const NodeRequest: { } // Parse in a single continuation (readBody -> parse) instead of going // through text() — one less promise + microtask hop per body read. - return readBody(this.#req, this.#maxRequestBodySize).then((buf) => - JSON.parse(buf.toString()), - ); + return this.#readBuffered().then((buf) => JSON.parse(buf.toString())); } get _request(): globalThis.Request { diff --git a/src/adapters/_node/send.ts b/src/adapters/_node/send.ts index 2ffed757..24b5601d 100644 --- a/src/adapters/_node/send.ts +++ b/src/adapters/_node/send.ts @@ -28,13 +28,32 @@ export function sendNodeResponse( * there. Streaming bodies still return their tracking promise (it drives * their own cleanup). * + * A synchronous throw during serialization (e.g. an invalid header value in + * `writeHead`) must not escape the request listener — that would surface as an + * `uncaughtException` and take the process down. Guard it here and fail the + * single response instead. + * * @internal */ export function sendNodeResponseDetached( nodeRes: NodeServerResponse, webRes: Response | NodeResponse, ): Promise | void { - return _sendNodeResponse(nodeRes, webRes, true); + try { + return _sendNodeResponse(nodeRes, webRes, true); + } catch (error) { + handleSendError(nodeRes, error); + } +} + +function handleSendError(nodeRes: NodeServerResponse, _error: unknown): void { + if (nodeRes.headersSent) { + // Response already committed — the only recovery is to tear down the socket. + nodeRes.destroy(); + } else { + nodeRes.statusCode = 500; + nodeRes.end(); + } } function _sendNodeResponse(