fix(ext/node): ServerResponse header array handling (denoland#24149)

Previously res.setHeader("foo", ["bar", "baz"]) added a single header
with a value of `bar,baz`. Really this should add two separate headers.
This is visible in `set-cookie` for example.

ext/node/polyfills/http.ts

@@ -1333,7 +1333,8 @@ function onError(self, error, cb) {
 export class ServerResponse extends NodeWritable {
  statusCode = 200;
  statusMessage?: string = undefined;
- #headers = new Headers({});
+ #headers: Record<string, string | string[]> = { __proto__: null };
+ #hasNonStringHeaders: boolean = false;
  #readable: ReadableStream;
  override writable = true;
  // used by `npm:on-finished`
@@ -1411,32 +1412,35 @@ export class ServerResponse extends NodeWritable {
  this.socket = socket;
  }
 
- setHeader(name: string, value: string) {
- this.#headers.set(name, value);
+ setHeader(name: string, value: string | string[]) {
+ if (Array.isArray(value)) {
+ this.#hasNonStringHeaders = true;
+ }
+ this.#headers[name] = value;
  return this;
  }
 
  getHeader(name: string) {
- return this.#headers.get(name) ?? undefined;
+ return this.#headers[name];
  }
  removeHeader(name: string) {
- return this.#headers.delete(name);
+ delete this.#headers[name];
  }
  getHeaderNames() {
- return Array.from(this.#headers.keys());
+ return Object.keys(this.#headers);
  }
  getHeaders() {
- return Object.fromEntries(this.#headers.entries());
+ return { __proto__: null, ...this.#headers };
  }
  hasHeader(name: string) {
- return this.#headers.has(name);
+ return Object.hasOwn(this.#headers, name);
  }
 
  writeHead(status: number, headers: Record<string, string> = {}) {
  this.statusCode = status;
  for (const k in headers) {
  if (Object.hasOwn(headers, k)) {
- this.#headers.set(k, headers[k]);
+ this.setHeader(k, headers[k]);
  }
  }
  return this;
@@ -1461,9 +1465,26 @@ export class ServerResponse extends NodeWritable {
  if (ServerResponse.#bodyShouldBeNull(this.statusCode)) {
  body = null;
  }
+ let headers: Record<string, string> | [string, string][] = this
+ .#headers as Record<string, string>;
+ if (this.#hasNonStringHeaders) {
+ headers = [];
+ // Guard is not needed as this is a null prototype object.
+ // deno-lint-ignore guard-for-in
+ for (const key in this.#headers) {
+ const entry = this.#headers[key];
+ if (Array.isArray(entry)) {
+ for (const value of entry) {
+ headers.push([key, value]);
+ }
+ } else {
+ headers.push([key, entry]);
+ }
+ }
+ }
  this.#resolve(
  new Response(body, {
- headers: this.#headers,
+ headers,
  status: this.statusCode,
  statusText: this.statusMessage,
  }),
@@ -1473,11 +1494,11 @@ export class ServerResponse extends NodeWritable {
  // deno-lint-ignore no-explicit-any
  override end(chunk?: any, encoding?: any, cb?: any): this {
  this.finished = true;
- if (!chunk && this.#headers.has("transfer-encoding")) {
+ if (!chunk && "transfer-encoding" in this.#headers) {
  // FIXME(bnoordhuis) Node sends a zero length chunked body instead, i.e.,
  // the trailing "0\r\n", but respondWith() just hangs when I try that.
- this.#headers.set("content-length", "0");
- this.#headers.delete("transfer-encoding");
+ this.#headers["content-length"] = "0";
+ delete this.#headers["transfer-encoding"];
  }
 
  // @ts-expect-error The signature for cb is stricter than the one implemented here

tests/unit_node/http_test.ts

@@ -182,6 +182,33 @@ Deno.test("[node/http] server can respond with 101, 204, 205, 304 status", async
  }
 });
 
+Deno.test("[node/http] multiple set-cookie headers", async () => {
+ const { promise, resolve } = Promise.withResolvers<void>();
+
+ const server = http.createServer((_req, res) => {
+ res.setHeader("Set-Cookie", ["foo=bar", "bar=foo"]);
+ assertEquals(res.getHeader("Set-Cookie"), ["foo=bar", "bar=foo"]);
+ res.end();
+ });
+
+ server.listen(async () => {
+ const res = await fetch(
+ // deno-lint-ignore no-explicit-any
+ `http:https://127.0.0.1:${(server.address() as any).port}/`,
+ );
+ assert(res.ok);
+
+ const setCookieHeaders = res.headers.getSetCookie();
+ assertEquals(setCookieHeaders, ["foo=bar", "bar=foo"]);
+
+ await res.body!.cancel();
+
+ server.close(() => resolve());
+ });
+
+ await promise;
+});
+
 Deno.test("[node/http] IncomingRequest socket has remoteAddress + remotePort", async () => {
  const { promise, resolve } = Promise.withResolvers<void>();
 
@@ -1000,8 +1027,8 @@ Deno.test("[node/http] ServerResponse getHeaders", () => {
  const res = new http.ServerResponse(req);
  res.setHeader("foo", "bar");
  res.setHeader("bar", "baz");
- assertEquals(res.getHeaderNames(), ["bar", "foo"]);
- assertEquals(res.getHeaders(), { "bar": "baz", "foo": "bar" });
+ assertEquals(res.getHeaderNames(), ["foo", "bar"]);
+ assertEquals(res.getHeaders(), { "foo": "bar", "bar": "baz" });
 });
 
 Deno.test("[node/http] ServerResponse default status code 200", () => {