node:http: walk the headers of writeHead(status, headers) in native code and send them as given
Problem
res.writeHead(200, { "Set-Cookie": "a=1", "set-cookie": "b=2" })sends onlyset-cookie: b=2. Node.js sends both lines.res.writeHead(200, ["Set-Cookie", KEPT, "Set-Cookie", sid])pushessidintoKEPT. 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
_writeHeadhas 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
matchHeadersets. - 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 vendoredtest-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 firstwrite()orend(). - 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 forContent-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:httploads 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++ (
jsHTTPFlattenRawHeadinNodeHTTP.cpp, and three error helpers inErrorCode.cpp): the three forms of the argument (object, flat list, list of pairs),validateHeaderName,validateHeaderValuewith the strict rule and the rule ofinsecureHTTPParser, array values (one line for each element,Cookieand the names of theuniqueHeadersoption joined by theirjoin("; ")), and what Node'smatchHeaderreads from each line.node:_http_outgoingis equal to main again. - JavaScript (
src/js+185/-10, the first revision had +195/-19): the gate, the fields thatmatchHeaderstores on the response (_contentLength,_last,shouldKeepAlive,chunkedEncoding,_defaultKeepAlive, the_removedflags), the 204/304 rule and theTrailercheck 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-Lengthnumber, and JavaScript stores them. - The handles (
NodeHTTPServer__writeHeadandhttp1_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 everywriteHead(). - The other choice is the bytes of the head in the native response.
NodeHTTPResponsecan 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.
| Case | main |
|---|---|
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: 2 | sends 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 valid | the 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 requests | pushes 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-onlyres.headersreport nothing afterwards. A logger that readsres.getHeader("content-length")at'finish'getsundefined. fastify 5 passes its headers towriteHead()in this way, soreply.raw.getHeaderNames()is[]in anonResponsehook.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 twoContent-Lengthlines. - 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 isfalse(ERR_INVALID_HTTP_TOKEN) ornullorundefined(aTypeError). A name in a flat list that is not a string throwsERR_INVALID_HTTP_TOKEN, wasERR_INVALID_ARG_TYPE. - A value goes out as
name + ": " + valuegives it:valueOf()beforetoString(). - A
writeHead()that throws keeps none of its header lines. Afterclosein aConnectionline 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-EncodingorConnectionno longer clearsres.sendDate, and no longer sets_removedContLen,_removedTEor_removedConnection. A header that the argument names clears its_removedflag. res.setHeader,res.appendHeaderandres.removeHeaderare not called for the entries.- A
Trailerline in the argument is checked with Node's rule. A 204 or 304 with a chunkedTransfer-Encodingsetsres.shouldKeepAlivetofalse. strictContentLengthchecks a body whoseTransfer-Encodingis not chunked. With aContent-Lengththat is not a number,write()does not fail andend()fails.- A
ServerResponsewith no socket (nock, msw) throwsERR_INVALID_CHARfor aContent-Dispositionthat is not ASCII after aContent-Length. writeHead()after the headers were sent saysCannot write headers after they are sent to the client. main saidCannot 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()) makeswriteHead()throwERR_HTTP_HEADERS_SENT. Node sends a second head inside the body. - Node checks
String(value)and sends whatvalueOf()gives. When the two texts differ, this branch checks the sent text too. AvalueOf()that gives"a\r\nX-Injected: 1"throwsERR_INVALID_CHAR. OutgoingMessage.prototype.setHeader.call(res, ...)after such awriteHead()sets a header that is not sent. main sent it. Node throws.- A
writeHead()that throws does not leave theContent-Lengthof its lines inres._contentLength. Node leaves that number there, and its next head can sayContent-Length: 100for a body of 2 bytes. - What the lines say about the response (
res._last,res.chunkedEncodingand the others) is set when the walk ends. Node sets it line by line. Only atoString()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 aTransfer-Encodingthat is not chunked or aContent-Lengththat is not a number. Bun writes it when it gets the connection, where a throw has no caller. res.headersSent = falseis a Bun-only setter. It moves the lines to the header store, sosetHeader()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 asString(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 4todotests that pass under Node. - Values that the store keeps are read when the head is written (#45047, #44778).
res._headerstaysnullafterwriteHead()on a response with a handle.- With a native handle, a
Content-Dispositionthat is not ASCII is sent after aContent-Length, one byte for each character. Node throws there. - On the store route,
addTrailers()beforewriteHead()still throws for a response that is not chunked (#33871 owns that check). - The message for a name that is a
Bufferor aStringobject shows how Bun inspects it. Node shows its text. Http2ServerResponse,writeInformation(),writeEarlyHints()andClientRequestare 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 offlattenRawHeadgoes into it insidewriteHead(). 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 forres.headers =. Land it first or second. - #33871 (frames the body by the value of
Transfer-Encoding): 4 regions in_http_server.tsand 1 ininternal/http.ts. ItsdecideFraming()readsContent-Length,Transfer-EncodingandTrailerfrom the store. For a raw head it must read the bits. The one that lands second makes that change, andrawHeadIsChunked()goes away. - #44321: no conflict. This revision adds no auto-header bit.
- #44338 (trailers travel with
end()): 1 region, the trailers gate ofend(). The one that lands second keeps #44338's block and the_contentLengthtest 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):
| Handler | main | this branch |
|---|---|---|
res.end("hi") | 10,097 | 10,124 |
writeHead(200), end | 11,135 | 11,172 |
setHeader twice, end | 14,545 | 14,600 |
setHeader four times, end | 18,953 | 19,010 |
setHeader, writeHead(200, object of 2), end | 20,083 | 20,140 |
writeHead(200, object of 2), end | 16,247 | 14,639 |
writeHead(200, object of 4), end | 22,100 | 19,499 |
writeHead(200, object of 8), end | 33,187 | 26,870 |
writeHead(200, flat list of 4), end | 27,963 | 19,740 |
writeHead(200, 4 pairs), end | 27,334 | 19,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):
end6,288 to 6,301,setHeadertwice 6,692 to 6,705,setHeaderandwriteHead(200, object)7,131 to 7,148, threewrite()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 inrenderNativeHeaders, 4 at the gate, 2 in eachwrite(). - 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
sendtofor 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:testandnode:assert, asnode-http-req-complete.test.tsdoes. 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
todoin 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.Float64Arraybefore it loadsnode:http. node-http.test.ts: 283 pass on both builds. The 761 vendoredtest-http*files exit 0 on both. 60 other test files usenode:httporwriteHead. 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_SENTis 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.Float64ArraymadewriteHead()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-Dispositionthat is not ASCII before aContent-Lengththrew. Node's_storeHeaderreads the lines in their order again. - A
writeHead()inside thetoString()of a value: the outer call took the status of the inner one, and an outer call that threw removed theContent-Lengthof the inner one. Connection: closebefore a value whosetoString()ends the response did not close the connection.- A
Cookiearray is joined by its ownjoin(), an object forConnectionorTransfer-Encodingis 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.headersSentsetter. 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.