fix(ext/node): ServerResponse header array handling (#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.

1 files changed, 34 insertions, 13 deletions

diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts
index ec3fe6e0b..32e69772d 100644
--- a/ext/node/polyfills/http.ts
+++ b/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