Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions src/js/internal/http1_server_fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,11 @@ function connectionListenerHTTP1(server, socket, options) {
req.url = url;
req.method = typeof methodNum === "number" ? allMethods[methodNum] : methodNum;
req.upgrade = upgrade;
// Like Node's parserOnHeadersComplete (and the native dispatcher): _addHeaderLine reads this
// off the request when it combines duplicates of a header that is not list-valued.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (server.joinDuplicateHeaders) {
req.joinDuplicateHeaders = true;
}
req._addHeaderLines(rawHeaders, rawHeaders.length);

// Node's parserOnIncoming: upgrade only sticks for CONNECT or when an 'upgrade' listener
Expand Down
5 changes: 4 additions & 1 deletion src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6804,8 +6804,11 @@ class Http2SecureServer extends tls.Server {
this.requestTimeout = http1Options.requestTimeout ?? 300000;
this.maxHeadersCount = http1Options.maxHeadersCount ?? null;
this.maxRequestsPerSocket = http1Options.maxRequestsPerSocket ?? 0;
const joinDuplicateHeaders = http1Options.joinDuplicateHeaders;
if (joinDuplicateHeaders !== undefined) validateBoolean(joinDuplicateHeaders, "options.joinDuplicateHeaders");
this.joinDuplicateHeaders = joinDuplicateHeaders;
// connectionListenerHTTP1 reads these off the server when initializing
// the per-connection parser, matching Node's storeHTTP1Options.
// the per-connection parser, matching Node's storeHTTPOptions.
this.maxHeaderSize = http1Options.maxHeaderSize;
this.insecureHTTPParser = http1Options.insecureHTTPParser;
this.httpValidation = http1Options.httpValidation;
Expand Down
81 changes: 81 additions & 0 deletions test/js/node/http/node-http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4139,3 +4139,84 @@ it("connectionListener hands off Upgrade and CONNECT like Node", async () => {
expect(serverSide.destroyed).toBe(true);
}
});

it("connectionListener applies the server's joinDuplicateHeaders option like the native path and Node", async () => {
// Node keeps only the first value of a header it treats as single-valued (Authorization,
// Content-Type, ...) unless the server was created with joinDuplicateHeaders; Cookie and unknown
// headers are joined either way. The same raw request goes through server.emit("connection", ...)
// (the JS parser path, which has to read the option off the server itself) and through the same
// server options listening normally (the native path); both must produce Node's req.headers.
const rawRequest =
"GET / HTTP/1.1\r\nHost: example.test\r\n" +
"Authorization: one\r\nAuthorization: two\r\n" +
"Content-Type: text/plain\r\nContent-Type: text/html\r\n" +
"Cookie: a=1\r\nCookie: b=2\r\n" +
"X-Custom: first\r\nX-Custom: second\r\n" +
"Connection: close\r\n\r\n";

function serverRecordingHeaders(options: { joinDuplicateHeaders?: boolean }) {
const { promise: headers, resolve, reject } = Promise.withResolvers<http.IncomingHttpHeaders>();
const server = createServer(options, (req, res) => {
resolve({ ...req.headers });
res.end();
});
// A no-op once the handler has resolved, so the connection closing after the response is fine.
const closedEarly = () => reject(new Error("the connection closed before the request was dispatched"));
return { server, headers, reject, closedEarly };
}

async function headersSeenOverEmittedConnection(options: { joinDuplicateHeaders?: boolean }) {
const { server, headers, reject, closedEarly } = serverRecordingHeaders(options);
const [clientSide, serverSide] = duplexPair();
clientSide.on("error", reject);
serverSide.on("error", reject);
serverSide.on("close", closedEarly);
clientSide.resume();
server.emit("connection", serverSide);
clientSide.write(rawRequest);
try {
return await headers;
} finally {
clientSide.destroy();
serverSide.destroy();
}
}

async function headersSeenOverListeningServer(options: { joinDuplicateHeaders?: boolean }) {
const { server, headers, reject, closedEarly } = serverRecordingHeaders(options);
server.listen(0, "127.0.0.1");
await once(server, "listening");
const socket = connect((server.address() as AddressInfo).port, "127.0.0.1");
socket.on("error", reject);
socket.on("close", closedEarly);
socket.resume();
socket.write(rawRequest);
try {
return await headers;
} finally {
socket.destroy();
server.close();
}
}

const joined = {
host: "example.test",
authorization: "one, two",
"content-type": "text/plain, text/html",
cookie: "a=1; b=2",
"x-custom": "first, second",
connection: "close",
};
const firstValueWins = { ...joined, authorization: "one", "content-type": "text/plain" };
for (const [options, expected] of [
[{ joinDuplicateHeaders: true }, joined],
[{ joinDuplicateHeaders: false }, firstValueWins],
[{}, firstValueWins],
] as const) {
expect({
options,
emitted: await headersSeenOverEmittedConnection(options),
listening: await headersSeenOverListeningServer(options),
}).toEqual({ options, emitted: expected, listening: expected });
}
});
73 changes: 73 additions & 0 deletions test/js/node/http2/node-http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4365,6 +4365,79 @@ it("http2 allowHTTP1 fallback omits the Connection header on a close-delimited r
}
});

// Sends a request with two Authorization and two Cookie lines over a TLS connection that negotiated
// http/1.1 (the allowHTTP1 fallback) and returns what the handler saw, plus the option as stored on
// the server.
async function duplicateHeadersSeenOverAllowHTTP1(options) {
const server = http2.createSecureServer({ ...TLS_CERT, allowHTTP1: true, ...options }, (req, res) => {
res.end(JSON.stringify({ authorization: req.headers.authorization, cookie: req.headers.cookie }));
});
await new Promise(resolve => server.listen(0, resolve));
try {
const { promise, resolve, reject } = Promise.withResolvers();
const socket = tls.connect(
{ host: "localhost", port: server.address().port, ca: TLS_CERT.cert, ALPNProtocols: ["http/1.1"] },
() =>
socket.write(
"GET / HTTP/1.1\r\nHost: localhost\r\n" +
"Authorization: one\r\nAuthorization: two\r\n" +
"Cookie: a=1\r\nCookie: b=2\r\n" +
"Connection: close\r\n\r\n",
),
);
const chunks = [];
socket.on("error", reject);
socket.on("data", chunk => chunks.push(chunk));
socket.on("end", () => resolve(Buffer.concat(chunks).toString()));
const raw = await promise;
expect(raw).toStartWith("HTTP/1.1 200 OK\r\n");
return { server: server.joinDuplicateHeaders, ...JSON.parse(raw.slice(raw.indexOf("\r\n\r\n") + 4)) };
} finally {
server.close();
}
}

it("http2 allowHTTP1 fallback applies joinDuplicateHeaders from options or options.http1Options like Node", async () => {
// Authorization is one of the headers Node keeps only the first value of unless the option is
// set; Cookie is joined either way. Node reads the option through
// storeHTTPOptions({ ...options, ...options.http1Options }), so http1Options takes precedence.
const joined = { authorization: "one, two", cookie: "a=1; b=2" };
const firstValueWins = { authorization: "one", cookie: "a=1; b=2" };
expect(
await Promise.all([
duplicateHeadersSeenOverAllowHTTP1({ joinDuplicateHeaders: true }),
duplicateHeadersSeenOverAllowHTTP1({ http1Options: { joinDuplicateHeaders: true } }),
duplicateHeadersSeenOverAllowHTTP1({ joinDuplicateHeaders: true, http1Options: { joinDuplicateHeaders: false } }),
duplicateHeadersSeenOverAllowHTTP1({ joinDuplicateHeaders: false }),
duplicateHeadersSeenOverAllowHTTP1({}),
]),
).toEqual([
{ server: true, ...joined },
{ server: true, ...joined },
{ server: false, ...firstValueWins },
{ server: false, ...firstValueWins },
{ server: undefined, ...firstValueWins },
]);
});

it("http2 createSecureServer type-checks joinDuplicateHeaders only when allowHTTP1 is set, like Node", () => {
for (const [options, received] of [
[{ joinDuplicateHeaders: "yes" }, "type string ('yes')"],
[{ http1Options: { joinDuplicateHeaders: 1 } }, "type number (1)"],
]) {
expect(() => {
http2.createSecureServer({ ...TLS_CERT, allowHTTP1: true, ...options });
}).toThrow(
expect.objectContaining({
code: "ERR_INVALID_ARG_TYPE",
message: `The "options.joinDuplicateHeaders" property must be of type boolean. Received ${received}`,
}),
);
// Without allowHTTP1 there is no HTTP/1 side to configure: Node neither validates nor stores it.
expect(Object.hasOwn(http2.createSecureServer({ ...TLS_CERT, ...options }), "joinDuplicateHeaders")).toBe(false);
}
});

// close() must not depend on the peer sending a SETTINGS ACK — Node's kMaybeDestroy
// waits on nghttp2_session_want_write()/want_read(), which does not track outstanding
// ACKs. A server that never ACKs a client-sent SETTINGS must not stall close().
Expand Down