Merge branch 'main' into pfg/node-3

src/bun.js/bindings/NodeHTTP.cpp

@@ -1369,7 +1369,8 @@ JSC_DEFINE_HOST_FUNCTION(jsHTTPGetHeader, (JSGlobalObject * globalObject, CallFr
     return JSValue::encode(jsUndefined());
 }
 
-JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFrame* callFrame))
+template<bool append>
+EncodedJSValue jsHTTPSetOrAppendHeader(JSGlobalObject* globalObject, CallFrame* callFrame)
 {
     auto& vm = JSC::getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
@@ -1397,7 +1398,11 @@ JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFr
                     RETURN_IF_EXCEPTION(scope, {});
                     auto value = item.toWTFString(globalObject);
                     RETURN_IF_EXCEPTION(scope, {});
-                    impl->set(name, value);
+                    if constexpr (append) {
+                        impl->append(name, value);
+                    } else {
+                        impl->set(name, value);
+                    }
                     RETURN_IF_EXCEPTION(scope, {});
                 }
                 for (unsigned i = 1; i < length; ++i) {
@@ -1414,7 +1419,11 @@ JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFr
 
             auto value = valueValue.toWTFString(globalObject);
             RETURN_IF_EXCEPTION(scope, {});
-            impl->set(name, value);
+            if constexpr (append) {
+                impl->append(name, value);
+            } else {
+                impl->set(name, value);
+            }
             RETURN_IF_EXCEPTION(scope, {});
             return JSValue::encode(jsUndefined());
         }
@@ -1423,6 +1432,16 @@ JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFr
     return JSValue::encode(jsUndefined());
 }
 
+JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFrame* callFrame))
+{
+    return jsHTTPSetOrAppendHeader<false>(globalObject, callFrame);
+}
+
+JSC_DEFINE_HOST_FUNCTION(jsHTTPAppendHeader, (JSGlobalObject * globalObject, CallFrame* callFrame))
+{
+    return jsHTTPSetOrAppendHeader<true>(globalObject, callFrame);
+}
+
 JSValue createNodeHTTPInternalBinding(Zig::GlobalObject* globalObject)
 {
     auto* obj = constructEmptyObject(globalObject);
@@ -1430,6 +1449,9 @@ JSValue createNodeHTTPInternalBinding(Zig::GlobalObject* globalObject)
     obj->putDirect(
         vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "setHeader"_s)),
         JSC::JSFunction::create(vm, globalObject, 3, "setHeader"_s, jsHTTPSetHeader, ImplementationVisibility::Public), 0);
+    obj->putDirect(
+        vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "appendHeader"_s)),
+        JSC::JSFunction::create(vm, globalObject, 3, "appendHeader"_s, jsHTTPAppendHeader, ImplementationVisibility::Public), 0);
     obj->putDirect(
         vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "getHeader"_s)),
         JSC::JSFunction::create(vm, globalObject, 2, "getHeader"_s, jsHTTPGetHeader, ImplementationVisibility::Public), 0);

src/js/internal/http.ts

@@ -1,6 +1,7 @@
 const {
   getHeader,
   setHeader,
+  appendHeader,
   Headers,
   assignHeaders: assignHeadersFast,
   setRequestTimeout,
@@ -12,7 +13,8 @@ const {
   setServerIdleTimeout,
 } = $cpp("NodeHTTP.cpp", "createNodeHTTPInternalBinding") as {
   getHeader: (headers: Headers, name: string) => string | undefined;
-  setHeader: (headers: Headers, name: string, value: string) => void;
+  setHeader: (headers: Headers, name: string, value: string | string[]) => void;
+  appendHeader: (headers: Headers, name: string, value: string | string[]) => void;
   Headers: (typeof globalThis)["Headers"];
   assignHeaders: (object: any, req: Request, headersTuple: any) => boolean;
   setRequestTimeout: (req: Request, timeout: number) => boolean;
@@ -366,6 +368,7 @@ export {
   METHODS,
   STATUS_CODES,
   abortedSymbol,
+  appendHeader,
   assignHeadersFast,
   bodyStreamSymbol,
   callCloseCallback,

src/js/node/_http_outgoing.ts

@@ -16,6 +16,7 @@ const {
   kHandle,
   getHeader,
   setHeader,
+  appendHeader,
   Headers,
   getRawKeys,
   kOutHeaders,
@@ -202,9 +203,14 @@ const OutgoingMessagePrototype = {
   _closed: false,
   _headerNames: undefined,
   appendHeader(name, value) {
+    if ((this._header !== undefined && this._header !== null) || this[headerStateSymbol] == NodeHTTPHeaderState.sent) {
+      throw $ERR_HTTP_HEADERS_SENT("set");
+    }
     validateString(name, "name");
+    validateHeaderValue(name, value);
     var headers = (this[headersSymbol] ??= new Headers());
-    headers.append(name, value);
+
+    appendHeader(headers, name, value);
     return this;
   },
 

test/js/node/http/fixtures/node-http-client-headers.mjs

@@ -0,0 +1,184 @@
+import http from "node:http";
+
+async function execute(test_name, options) {
+  console.log("%<test>" + test_name + "</test>");
+  const { promise, resolve, reject } = Promise.withResolvers();
+  http
+    .createServer(function (req, res) {
+      if (typeof Bun !== "undefined") {
+        // bun adds these headers by default
+        if (req.headers["user-agent"] === `Bun/${Bun.version}`) {
+          delete req.headers["user-agent"];
+        }
+        if (req.headers["accept"] === "*/*") {
+          delete req.headers["accept"];
+        }
+      }
+
+      this.close();
+
+      console.log(
+        JSON.stringify(
+          Object.fromEntries(Object.entries(req.headers).sort((a, b) => a[0].localeCompare(b[0]))),
+        ).replaceAll('"', "'"),
+      );
+
+      res.writeHead(200, { "Connection": "close" });
+      res.end();
+    })
+    .listen(0, function () {
+      options = Object.assign(options, {
+        port: this.address().port,
+        path: "/",
+      });
+      const req = http.request(options);
+      req.end();
+      req.on("response", rsp => {
+        console.log("-> " + rsp.statusCode);
+        resolve();
+      });
+    });
+  await promise;
+}
+
+await execute("headers array in object", {
+  headers: {
+    "a": "one",
+    "b": ["two", "three"],
+    "cookie": ["four", "five", "six"],
+    "Host": "example.com",
+  },
+});
+
+await execute("multiple of same header in array", {
+  headers: [
+    ["a", "one"],
+    ["b", "two"],
+    ["b", "three"],
+    ["cookie", "four"],
+    ["cookie", "five"],
+    ["cookie", "six"],
+    ["Host", "example.com"],
+  ],
+});
+
+await execute("multiple of same header in array 2", {
+  headers: [
+    ["a", "one"],
+    ["b", ["two", "three"]],
+    ["cookie", ["four", "five"]],
+    ["cookie", "six"],
+    ["Host", "example.com"],
+  ],
+});
+
+await execute("multiple of same header in array 3", {
+  headers: [
+    ["a", "one"],
+    ["b", "two"],
+    ["b", "three"],
+    ["cookie", ["four", "five", "six"]],
+    ["Host", "example.com"],
+  ],
+});
+
+await execute("multiple of same header in flat array", {
+  headers: [
+    "a",
+    "one",
+    "b",
+    "two",
+    "b",
+    "three",
+    "cookie",
+    "four",
+    "cookie",
+    "five",
+    "cookie",
+    "six",
+    "Host",
+    "example.com",
+  ],
+});
+
+await execute("arrays of headers in flat array", {
+  headers: ["a", "one", "b", ["two", "three"], "cookie", ["four", "five"], "cookie", "six", "Host", "example.com"],
+});
+
+await execute("set user agent and accept", {
+  headers: {
+    "abc": "def",
+    "user-agent": "my new user agent",
+    "accept": "text/html",
+    "host": "example.com",
+  },
+});
+
+await execute("set user agent and accept (array 1)", {
+  headers: [
+    ["user-agent", "my new user agent"],
+    ["accept", "text/html"],
+    ["host", "example.com"],
+  ],
+});
+
+await execute("set user agent and accept (flat array)", {
+  headers: ["user-agent", "my new user agent", "accept", "text/html", "host", "example.com"],
+});
+
+async function server() {
+  const { promise, resolve, reject } = Promise.withResolvers();
+
+  const server = http.createServer((req, res) => {
+    // Set response headers
+    res.setHeader("Content-Type", "text/plain");
+    res.setHeader("X-Powered-By", "Node.js");
+    res.setHeader("Cache-Control", ["no-cache", "yes-cache"]);
+    res.appendHeader("Cache-Control", "maybe-cache");
+    res.appendHeader("Cache-Control", ["please-cache", "please-dont-cache"]);
+    res.setHeader("Set-Cookie", ["a=b", "c=d"]);
+    res.appendHeader("Set-Cookie", "e=f");
+    res.appendHeader("Set-Cookie", ["g=h", "i=j"]);
+    res.setHeader("Abc", ["list-one", "list-two"]);
+    res.setHeader("Abc", ["list-three", "list-four"]);
+
+    // Write response
+    res.statusCode = 200;
+    res.end("Hello World\n");
+  });
+
+  const PORT = 0;
+  server.listen(PORT, async () => {
+    const port = server.address().port;
+    console.log(`Server running`);
+
+    // Test the server response headers using fetch
+    try {
+      const response = await fetch(`http://localhost:${port}/`);
+      console.log("Response status: " + response.status);
+
+      // Check headers
+      console.log("Headers test results:");
+      for (const [key, value] of [...response.headers.entries()].sort((a, b) => a[0].localeCompare(b[0]))) {
+        if (key === "date") continue;
+        if (key === "keep-alive") continue;
+        if (key === "connection") continue;
+        console.log(`${key}: ${value}`);
+      }
+
+      const body = await response.text();
+      console.log("Body:", body);
+      resolve();
+    } catch (error) {
+      console.error("Error testing server:", error);
+      reject(error);
+    } finally {
+      // Uncomment to close server after test
+      // server.close();
+    }
+  });
+  await promise;
+  server.close();
+}
+console.log("%<test>server</test>");
+await server();

test/js/node/http/node-http-client-headers.test.ts

@@ -0,0 +1,30 @@
+import { bunExe } from "harness";
+
+function toObject(str: string) {
+  const split = str.split("%<test>");
+  const result = {};
+  for (const line of split) {
+    if (!line.trim()) continue;
+    const [key, value] = line.split("</test>");
+    result[key] = value;
+  }
+  return result;
+}
+
+describe("node-http-client-headers", async () => {
+  const expected = Bun.spawnSync(["node", import.meta.dir + "/fixtures/node-http-client-headers.mjs"], {
+    stdio: ["ignore", "pipe", "pipe"],
+  });
+  const actual = Bun.spawnSync([bunExe(), import.meta.dir + "/fixtures/node-http-client-headers.mjs"], {
+    stdio: ["ignore", "pipe", "pipe"],
+  });
+  expect(actual.stderr.toString()).toEqual(expected.stderr.toString());
+  expect(actual.exitCode).toEqual(expected.exitCode);
+  const expected_obj = toObject(expected.stdout.toString());
+  const actual_obj = toObject(actual.stdout.toString());
+  for (const [key, value] of Object.entries(expected_obj)) {
+    test(key, () => {
+      expect(actual_obj[key]).toEqual(value);
+    });
+  }
+});

test/js/node/test/parallel/test-http-client-headers-array.js

@@ -0,0 +1,76 @@
+'use strict';
+
+const {mustCall} = require('../common');
+
+const assert = require('assert');
+const http = require('http');
+
+function execute(options) {
+  http.createServer(mustCall(function(req, res) {
+    const expectHeaders = {
+      'x-foo': 'boom',
+      'cookie': 'a=1; b=2; c=3',
+      'connection': 'keep-alive',
+      'host': 'example.com',
+    };
+
+    // no Host header when you set headers an array
+    if (!Array.isArray(options.headers)) {
+      expectHeaders.host = `localhost:${this.address().port}`;
+    }
+
+    // no Authorization header when you set headers an array
+    if (options.auth && !Array.isArray(options.headers)) {
+      expectHeaders.authorization =
+          `Basic ${Buffer.from(options.auth).toString('base64')}`;
+    }
+
+    if(typeof Bun !== 'undefined') {
+      // bun adds these headers by default
+      expectHeaders['user-agent'] ??= `Bun/${Bun.version}`;
+      expectHeaders['accept'] ??= '*/*';
+    }
+
+    this.close();
+
+    assert.deepStrictEqual(req.headers, expectHeaders);
+
+    res.writeHead(200, { 'Connection': 'close' });
+    res.end();
+  })).listen(0, mustCall(function() {
+    options = Object.assign(options, {
+      port: this.address().port,
+      path: '/'
+    });
+    const req = http.request(options);
+    req.end();
+  }));
+}
+
+// Should be the same except for implicit Host header on the first two
+execute({ headers: { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } });
+execute({ headers: { 'x-foo': 'boom', 'cookie': [ 'a=1', 'b=2', 'c=3' ] } });
+execute({ headers: [
+  [ 'x-foo', 'boom' ],
+  [ 'cookie', 'a=1; b=2; c=3' ],
+  [ 'Host', 'example.com' ],
+] });
+execute({ headers: [
+  [ 'x-foo', 'boom' ],
+  [ 'cookie', [ 'a=1', 'b=2', 'c=3' ]],
+  [ 'Host', 'example.com' ],
+] });
+execute({ headers: [
+  [ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ],
+  [ 'cookie', 'b=2' ], [ 'cookie', 'c=3' ],
+  [ 'Host', 'example.com'],
+] });
+
+// Authorization and Host header both missing from the second
+execute({ auth: 'foo:bar', headers:
+  { 'x-foo': 'boom', 'cookie': 'a=1; b=2; c=3' } });
+execute({ auth: 'foo:bar', headers: [
+  [ 'x-foo', 'boom' ], [ 'cookie', 'a=1' ],
+  [ 'cookie', 'b=2' ], [ 'cookie', 'c=3'],
+  [ 'Host', 'example.com'],
+] });