node:http: walk the headers of writeHead(status, headers) in native code and send them as given

RO
1mo ago
+3219-10

Problem

  • res.writeHead(200, { "Set-Cookie": "a=1", "set-cookie": "b=2" }) sends only set-cookie: b=2. Node.js sends both lines.
  • res.writeHead(200, ["Set-Cookie", KEPT, "Set-Cookie", sid]) pushes sid into KEPT. The next user receives it (1.4.0 to main).
  • _writeHead (src/js/node/_http_server.ts:2301) folds the argument into the header store. Node folds only when a header was set before (_http_server.js:433).

Fix

  • _writeHead has Node's gate. With no header set before, flattenRawHead (src/jsc/bindings/NodeHTTP.cpp) walks the argument in C++ with Node's checks. It returns the [name, value, ...] list that the handles take on main.
  • The response keeps that list until the first write. The handles do not change. JavaScript keeps the gate and the fields that Node's matchHeader sets.
  • Verified: test/js/node/http/node-http-writehead.test.ts (305 tests, 142 fail on main, 280 pass under Node v26.3.0), node-http.test.ts, 761 vendored test-http* files. Self-reviewed: 40 concerns raised, 35 addressed.

Background

  • The header store (kOutHeaders) has one slot per lowercased name. Bun writes the head at the first write() or end().
  • Considered a patch inside the fold: order, spelling and getters stay different from Node. Considered bytes in the native response: Notes has a question.

Downsides

  • After such a writeHead(), getHeader() reports nothing, as Node documents. Two spellings of one name send two lines, also for Content-Length. A bad entry throws where Bun skipped it.
  • Other responses: at most 57 more instructions per request (0.4%). The new route: 10% to 29% fewer.
  • Binary text: +33,550 bytes. node:http loads 47 more GC cells.
Notes

What is native now, and what is not

The first revision did the walk in JavaScript (walkRawHead() in node:_http_outgoing). The review asked for native code. In this revision:

  • C++ (jsHTTPFlattenRawHead in NodeHTTP.cpp, and three error helpers in ErrorCode.cpp): the three forms of the argument (object, flat list, list of pairs), validateHeaderName, validateHeaderValue with the strict rule and the rule of insecureHTTPParser, array values (one line for each element, Cookie and the names of the uniqueHeaders option joined by their join("; ")), and what Node's matchHeader reads from each line. node:_http_outgoing is equal to main again.
  • JavaScript (src/js +185/-10, the first revision had +195/-19): the gate, the fields that matchHeader stores on the response (_contentLength, _last, shouldKeepAlive, chunkedEncoding, _defaultKeepAlive, the _removed flags), the 204/304 rule and the Trailer check of _storeHeader, and the places that read the list.
  • Those fields are JavaScript properties of the response, and two of them are accessors. C++ would need a property name for each one at every start of a VM. So the function reports eight bits and the Content-Length number, and JavaScript stores them.
  • The handles (NodeHTTPServer__writeHead and http1_server_fallback.ts) are equal to main. They get the same kind of list as before.

Question for a maintainer: where does the head live?

Node stores the rendered head inside writeHead(). Bun writes the head at the first write() or end(), so something must hold the lines until then.

  • This revision holds the flat list on the ServerResponse (one slot, kRawHead). It is the list that both handles read on main. #45047 holds the same kind of list for every writeHead().
  • The other choice is the bytes of the head in the native response. NodeHTTPResponse can hold them for an accepted socket. A socket given to 'connection' has no native response, so its JavaScript handle needs the same field. Both handles then get a second way to take a head.
  • I built the bytes as one string first, with a second arm in both handles. At the first write it ran about 1,850 fewer instructions for 2 lines and 7,000 fewer for 8 than this revision, and the binary was about 8,000 bytes smaller. The reason is the flat arm of the native handle: it costs about 800 instructions for each line on main. I did not ship the string, because it is a second head format in two handles. The flat arm is the same for every response, so a faster arm is a change of its own.
  • If you want the bytes in NodeHTTPResponse, tell me. The gate, the walk and the test file stay. Only the hold changes.

An earlier comment of mine on this pull request said that a response that waits behind another one has no native response. That was wrong. Each request gets its NodeHTTPResponse when it is dispatched.

Repro

import http from "node:http";
import net from "node:net";
const cases = {
  "object, two spellings of one name": res => res.writeHead(200, { "Set-Cookie": "a=1", "set-cookie": "b=2" }),
  "flat list, a name repeated around another": res => res.writeHead(200, ["X-D", "1", "X-A", "q", "X-D", "2"]),
  "flat list with Date": res => res.writeHead(200, ["Date", "Mon, 01 Jan 2024 00:00:00 GMT", "X-A", "1"]),
};
for (const [name, fn] of Object.entries(cases)) {
  let after;
  const server = http.createServer((req, res) => {
    fn(res);
    after = { sendDate: res.sendDate, names: res.getHeaderNames() };
    res.end("ok");
  });
  await new Promise(r => server.listen(0, "127.0.0.1", r));
  const head = await new Promise(done => {
    const c = net.connect(server.address().port, "127.0.0.1", () =>
      c.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"),
    );
    let b = "";
    c.on("data", d => (b += d.toString("latin1")));
    c.on("close", () => done(b.split("\r\n\r\n")[0]));
  });
  const lines = head
    .split("\r\n")
    .slice(1)
    .filter(l => !/^(connection|content-length|transfer-encoding|keep-alive):/i.test(l))
    .filter(l => name.includes("Date") || !/^date:/i.test(l));
  console.log(name, "=>", lines.join(" | "), "sendDate=" + after.sendDate, "getHeaderNames=" + JSON.stringify(after.names));
  server.close();
}

Node v26.3.0 and this branch:

object, two spellings of one name => Set-Cookie: a=1 | set-cookie: b=2 sendDate=true getHeaderNames=[]
flat list, a name repeated around another => X-D: 1 | X-A: q | X-D: 2 sendDate=true getHeaderNames=[]
flat list with Date => Date: Mon, 01 Jan 2024 00:00:00 GMT | X-A: 1 sendDate=true getHeaderNames=[]

main:

object, two spellings of one name => set-cookie: b=2 sendDate=true getHeaderNames=["set-cookie"]
flat list, a name repeated around another => X-D: 1 | X-D: 2 | X-A: q sendDate=true getHeaderNames=["x-d","x-a"]
flat list with Date => Date: Mon, 01 Jan 2024 00:00:00 GMT | X-A: 1 sendDate=false getHeaderNames=["date","x-a"]

Where it shows up

No issue reports this. A comparison with Node v26.3.0 found it. The fold is in node:http since #572 (2022). #33870 had the same gate for the list forms. A cleanup of stale pull requests closed it before a maintainer reviewed it.

The bug needs a response with no header set before, and one of these in the argument: two spellings of one name, a name repeated in a list, an entry that makes writeHead() throw, or an array that the caller keeps. Each row was run on Node v26.3.0, on a release build of main and on a release build of this branch. Node and this branch agree in every row.

Casemain
proxy@2.2.0 relays an upstream reply with Set-Cookie: a=1, set-cookie: b=2, X-D: 1, X-A: q, x-d: 2sends set-cookie: b=2, x-d: 2, X-A: q
socket.io@4.7.1 with its cookie option and an initial_headers listener that sets headers["set-cookie"]does not send the io session cookie
fastify@5.2.2, a reply with set-cookie and then a header value that is not validthe 500 carries set-cookie: session=abc and x-ok: 1 of the rejected reply
nock 14.0.17, tests/got/test_reply_headers.js (a ServerResponse with no socket)18 of 20 tests pass. Node and this branch: 20
a handler catches the throw of writeHead(200, { "Content-Length": 100, "X-User": bad }) and calls res.end("bad header")Content-Length: 100 with a 10-byte body
writeHead(200, ["Set-Cookie", KEPT, "Set-Cookie", sid]) with KEPT kept across requestspushes each sid into KEPT, so the next user gets it. Bun 1.3.13 does not

Not affected, because Node folds there too: a header set before writeHead() (Express, the on-headers middleware, socket.io with its cors option). The array row is a regression of 1.4.0. #44778 owns it for those routes. This pull request closes it only when no header was set before.

A headers argument with one spelling for each name, and strings or numbers as values, sends the same lines as before.

What changes for code that runs on Bun today

What Node v26.3.0 does, and Bun now does too. It applies only to writeHead(status, headers) on a response with no header set before.

  • getHeader(), hasHeader(), getHeaders(), getHeaderNames(), getRawHeaderNames() and the Bun-only res.headers report nothing afterwards. A logger that reads res.getHeader("content-length") at 'finish' gets undefined. fastify 5 passes its headers to writeHead() in this way, so reply.raw.getHeaderNames() is [] in an onResponse hook. reply.getHeaders() is not changed.
  • Two spellings of one name are two lines. On main the second replaced the first. writeHead(200, { ...upstream.headers, "Content-Length": n }) can now send two Content-Length lines.
  • A name repeated in a list keeps its position.
  • A bad entry throws where main skipped it: an empty name (ERR_INVALID_HTTP_TOKEN), an entry of a list of pairs that is false (ERR_INVALID_HTTP_TOKEN) or null or undefined (a TypeError). A name in a flat list that is not a string throws ERR_INVALID_HTTP_TOKEN, was ERR_INVALID_ARG_TYPE.
  • A value goes out as name + ": " + value gives it: valueOf() before toString().
  • A writeHead() that throws keeps none of its header lines. After close in a Connection line the connection still closes, as in Node and on main.
  • The argument and its array values are read once, inside writeHead(). A later change to them is not sent. An array of the caller is not written to.
  • A list that names Date, Content-Length, Transfer-Encoding or Connection no longer clears res.sendDate, and no longer sets _removedContLen, _removedTE or _removedConnection. A header that the argument names clears its _removed flag.
  • res.setHeader, res.appendHeader and res.removeHeader are not called for the entries.
  • A Trailer line in the argument is checked with Node's rule. A 204 or 304 with a chunked Transfer-Encoding sets res.shouldKeepAlive to false.
  • strictContentLength checks a body whose Transfer-Encoding is not chunked. With a Content-Length that is not a number, write() does not fail and end() fails.
  • A ServerResponse with no socket (nock, msw) throws ERR_INVALID_CHAR for a Content-Disposition that is not ASCII after a Content-Length.
  • writeHead() after the headers were sent says Cannot write headers after they are sent to the client. main said Cannot writeHead headers.

What neither Node nor main does. Each one is a choice of this pull request:

  • A value whose toString() sends the head (flushHeaders(), write()) makes writeHead() throw ERR_HTTP_HEADERS_SENT. Node sends a second head inside the body.
  • Node checks String(value) and sends what valueOf() gives. When the two texts differ, this branch checks the sent text too. A valueOf() that gives "a\r\nX-Injected: 1" throws ERR_INVALID_CHAR.
  • OutgoingMessage.prototype.setHeader.call(res, ...) after such a writeHead() sets a header that is not sent. main sent it. Node throws.
  • A writeHead() that throws does not leave the Content-Length of its lines in res._contentLength. Node leaves that number there, and its next head can say Content-Length: 100 for a body of 2 bytes.
  • What the lines say about the response (res._last, res.chunkedEncoding and the others) is set when the walk ends. Node sets it line by line. Only a toString() that reads those fields in the middle of the call can see the difference.
  • With strictContentLength, a response that waits behind another one is not checked for a Transfer-Encoding that is not chunked or a Content-Length that is not a number. Bun writes it when it gets the connection, where a throw has no caller.
  • res.headersSent = false is a Bun-only setter. It moves the lines to the header store, so setHeader() works on them as on main.

What stays different from Node.js

  • A header set before writeHead(status, headers): the argument goes through the store, as in Node. A list of pairs is still accepted there, and values go out as String(value) gives them.
  • The handle frames the body by the presence of Transfer-Encoding, not by its value, and not for HTTP/1.0 (#33871). An HTTP/1.0 keep-alive request on an accepted socket is closed after the response (#44321). The test file has these as 4 todo tests that pass under Node.
  • Values that the store keeps are read when the head is written (#45047, #44778).
  • res._header stays null after writeHead() on a response with a handle.
  • With a native handle, a Content-Disposition that is not ASCII is sent after a Content-Length, one byte for each character. Node throws there.
  • On the store route, addTrailers() before writeHead() still throws for a response that is not chunked (#33871 owns that check).
  • The message for a name that is a Buffer or a String object shows how Bun inspects it. Node shows its text.
  • Http2ServerResponse, writeInformation(), writeEarlyHints() and ClientRequest are not touched.

Order with the other open pull requests in these files

Trial merges of this branch with each head (git merge-tree):

  • #45047 (renders the head inside writeHead()): 1 region, the count of bytes of a queued response. Take #45047's count. It holds the same kind of list (kHeldHead), so the list of flattenRawHead goes into it inside writeHead(). Either order works.
  • #44778 (copies the array values of a header list): no conflict. It owns the array leak for a header set before writeHead() and for res.headers =. Land it first or second.
  • #33871 (frames the body by the value of Transfer-Encoding): 4 regions in _http_server.ts and 1 in internal/http.ts. Its decideFraming() reads Content-Length, Transfer-Encoding and Trailer from the store. For a raw head it must read the bits. The one that lands second makes that change, and rawHeadIsChunked() goes away.
  • #44321: no conflict. This revision adds no auto-header bit.
  • #44338 (trailers travel with end()): 1 region, the trailers gate of end(). The one that lands second keeps #44338's block and the _contentLength test of this branch.
  • #44871: 1 region, one more name in an import list. Keep both.

Measurements

Release builds of the merge base (6c1eb067e3) and of this branch. perf, valgrind, strace and bloaty are not installed on my machine, so the instruction counts come from a ptrace single-step counter.

Instructions of one request, from the entry of the native dispatch to its return (concurrent JIT off, GC timer off, the least of 12 requests after 20,000):

Handlermainthis branch
res.end("hi")10,09710,124
writeHead(200), end11,13511,172
setHeader twice, end14,54514,600
setHeader four times, end18,95319,010
setHeader, writeHead(200, object of 2), end20,08320,140
writeHead(200, object of 2), end16,24714,639
writeHead(200, object of 4), end22,10019,499
writeHead(200, object of 8), end33,18726,870
writeHead(200, flat list of 4), end27,96319,740
writeHead(200, 4 pairs), end27,33419,918
  • The same counts with only the Baseline JIT: 86 to 177 more for the rows that do not take the new route (at most 0.35%).
  • The walk alone: 2,041 instructions for 2 lines, 3,343 for 4, 5,834 for 8. The native head writer runs the same count on both builds (3,839 for 4 lines).
  • Executed bytecodes for one request cycle (debug build, Baseline trace): end 6,288 to 6,301, setHeader twice 6,692 to 6,705, setHeader and writeHead(200, object) 7,131 to 7,148, three write() calls 7,631 to 7,650, object of 2 6,958 to 6,666, object of 4 7,318 to 6,688, flat list of 3 7,707 to 6,672. The added ones: 4 for the slot in the constructor, 9 in renderNativeHeaders, 4 at the gate, 2 in each write().
  • GC cells for one response (Array / Object / string): equal for the first five rows. Object of 4: 36.3 / 35.5 / 3 to 28.3 / 34.5 / 5. Flat list of 3: 36.3 / 34.5 / 9 to 29.3 / 33.5 / 1.
  • size: text 89,151,860 to 89,185,410. Data and bss are equal. About 5,700 of those bytes are the text of the two builtin modules.
  • require("node:http"): 8,692 to 8,739 GC cells. Own properties of a response: 49 to 50. The hidden classes of responses after three mixes of handlers: 2, 2 and 3 on both.
  • Send calls: 2,000 sendto for 1,000 request cycles on both.
  • bench/snippets/http-writehead.node.mjs, user CPU seconds of the server for 200,000 requests, 6 pairs run in turn, on an earlier build of this revision: main 6.09 to 7.30 (median 7.06), this branch 5.58 to 6.54 (median 6.26). The ranges overlap.

Tests

  • The test file uses node:test and node:assert, as node-http-req-complete.test.ts does. Its last test runs the same file under Node.js. 280 of the 305 tests run there and pass on v26.3.0. The others are for Bun only.
  • Release builds: the merge base 159 pass and 142 fail, this branch 301 pass. Bun 1.4.2: 151 fail. 4 tests are todo in Bun and pass under Node.
  • Three tests compute what they expect from Node's own expressions (tokenRegExp, the strict and the lenient rule for a value) for every byte and for some 16-bit characters.
  • One test starts a process that replaces globalThis.Float64Array before it loads node:http.
  • node-http.test.ts: 283 pass on both builds. The 761 vendored test-http* files exit 0 on both. 60 other test files use node:http or writeHead. 56 pass on both builds. 4 fail on both, with counts that change from run to run (serve.test.ts, node-tls-server.test.ts, fetch-backpressure.test.ts, fetch.stream.test.ts).
  • Debug build with ASAN: 301 pass, and no report with BUN_JSC_validateExceptionChecks=1.
  • Each clause of the change was removed or changed in turn: 78 variants of the JavaScript and 57 of the C++. Each variant fails at least one test.

Self-review

A review of the first native build (the head as one string) raised 28 concerns that held. It said to keep the gate, the walk and the tests, and to use the list that the handles already take. This revision is that rework. 24 of the 28 changed the code, the tests or this text. Among them:

  • One head format. The string needed a second arm in both handles and used an auto-header bit that #44321 also uses.
  • A subclass that calls writeHead() in its constructor lost its headers on a socket given to 'connection'. The list is now kept also when the response has no handle yet. Four tests cover it.
  • The text of ERR_HTTP_HEADERS_SENT is Node's at all three places.
  • Two changes that were not part of this fix are out: the addTrailers() rule and two arms of the native handle that no caller uses.
  • The checks of names and values have table tests.
  • The costs for other responses are measured and are in this text.

A second check read this revision. It raised 12 more concerns, and 11 changed the code or the tests:

  • A program that replaces globalThis.Float64Array made writeHead() write to memory that it did not own. The function gets its two numbers back through an array of the engine now, and checks that array.
  • On a response with no socket, a Content-Disposition that is not ASCII before a Content-Length threw. Node's _storeHeader reads the lines in their order again.
  • A writeHead() inside the toString() of a value: the outer call took the status of the inner one, and an outer call that threw removed the Content-Length of the inner one.
  • Connection: close before a value whose toString() ends the response did not close the connection.
  • A Cookie array is joined by its own join(), an object for Connection or Transfer-Encoding is asked for its string as often as Node asks, and the message for a name that is not a string follows Node's %s.

5 were not taken:

  • Remove the Bun-only res.headersSent setter. That is a change of its own.
  • First land a patch inside the fold (about 16 lines) as a pull request of its own. It leaves the order of a list, the second spelling in a getter and the getters different from Node, and the review asked for native code.
  • One pull request for all open changes to the response head. The order above says how they fit. A maintainer can ask for the other way.
  • Keep the getters answering after such a writeHead(). Node documents that they do not.
  • A shorter run of the file under Node.js in its last test. It takes about 2 s, as long as the file has this many tests.