Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6809,6 +6809,9 @@ class Http2SecureServer extends tls.Server {
this.maxHeaderSize = http1Options.maxHeaderSize;
this.insecureHTTPParser = http1Options.insecureHTTPParser;
this.httpValidation = http1Options.httpValidation;
const joinDuplicateHeaders = http1Options.joinDuplicateHeaders;
if (joinDuplicateHeaders !== undefined) validateBoolean(joinDuplicateHeaders, "options.joinDuplicateHeaders");
this.joinDuplicateHeaders = joinDuplicateHeaders;
}
if (typeof onRequestHandler === "function") {
this.on("request", onRequestHandler);
Expand Down
51 changes: 51 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,54 @@ it("connectionListener hands off Upgrade and CONNECT like Node", async () => {
expect(serverSide.destroyed).toBe(true);
}
});

it("connectionListener applies the server's joinDuplicateHeaders option like 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. A socket fed in through server.emit("connection", ...) is served
// by the JS parser path, which has to read the option off the server like the native path does.
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";

async function headersSeenOverEmittedConnection(options: { joinDuplicateHeaders?: boolean }) {
const { promise: seen, resolve, reject } = Promise.withResolvers<Record<string, string | undefined>>();
const server = createServer(options, (req, res) => {
const { authorization, "content-type": contentType, cookie, "x-custom": xCustom } = req.headers;
resolve({ authorization, "content-type": contentType, cookie, "x-custom": xCustom as string });
res.end();
});
const [clientSide, serverSide] = duplexPair();
clientSide.on("error", reject);
serverSide.on("error", reject);
serverSide.on("close", () => reject(new Error("the server closed the connection without dispatching the request")));
clientSide.resume();
server.emit("connection", serverSide);
clientSide.write(rawRequest);
try {
return await seen;
} finally {
clientSide.destroy();
serverSide.destroy();
}
}

const joined = {
authorization: "one, two",
"content-type": "text/plain, text/html",
cookie: "a=1; b=2",
"x-custom": "first, second",
};
const firstValueWins = { ...joined, authorization: "one", "content-type": "text/plain" };
expect(
await Promise.all([
headersSeenOverEmittedConnection({ joinDuplicateHeaders: true }),
headersSeenOverEmittedConnection({ joinDuplicateHeaders: false }),
headersSeenOverEmittedConnection({}),
]),
).toEqual([joined, firstValueWins, firstValueWins]);
});
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