webcore: fix Content-Type lost when Request/Response body is read before headers

Closed
RO
2mo ago
+289-19

Problem

The Content-Type header that Bun derives from a FormData, URLSearchParams, or typed Blob body is only copied into the Request/Response headers lazily, on the first access of .headers. Consuming the body first replaces the internal Blob with Value::Used, after which the lazy path finds no Blob and synthesizes empty headers.

For a multipart body the boundary lives only in that header, so it became unrecoverable:

const fd = new FormData();
fd.append("n", "V");
const req = new Request("http://x/", { method: "POST", body: fd });
const body = await req.arrayBuffer(); // read the body first, as a proxy/middleware does

req.headers.get("content-type");
// bun:  null  (the boundary is now unknowable)
// node: "multipart/form-data; boundary=----formdata-undici-0416..."
// ...while `body` already starts with "------WebKitFormBoundaryc00e33..."

Reading .headers before the body returned the correct value, so the result depended on access order. This breaks the standard proxy/signing/middleware shape (consume or hash the body, then forward the headers plus bytes): the forwarded multipart body has no boundary the upstream can parse. Per Fetch, the header list is fixed at construction, so it must not depend on which of the two is read first.

URLSearchParams and Blob bodies with a type lost their Content-Type the same way, Response had the identical bug, and Response.body lost it too. fetch(request) was also affected: it extracts the body Blob directly, so afterwards the user-held Request's headers.get("content-type") was null even though the correct boundary went out on the wire.

Cause

Request::construct_into only copied the Blob's content type into the headers when a headers init was supplied; otherwise it deferred to ensure_fetch_headers, which reads the content type off BodyValue::Blob(blob). Once the body is consumed that match arm no longer hits and nothing is written. Response::constructor / get_or_create_headers had the same guard.

Fix

The two classes need different treatment.

Request: write the Content-Type into the headers at construction (construct_into). When the extracted body is a Blob with a non-empty content type, the headers object is created (if it does not already exist) and the Content-Type written, unless the headers already have one. Nothing depends on a user-constructed Request's headers being unmaterialized, and fixing it at construction closes every path that takes the body Blob afterwards at once: the BodyMixin consumers, fetch(request) (fetch.rs:1249), Bun.write, the shell, new Blob([req]), and anything added later. fetch() already skips the body's content type when the headers contain one (from_fetch_headers), so nothing is sent twice.

Response: preserve the Content-Type at consumption time. Construction-time materialization regressed 25 Content-Range tests in serve.test.ts plus 1 in bun-serve-file.test.ts in an earlier revision of this PR, because Bun.serve read Response.init.headers being unset as "the user passed no headers init" when deciding whether a sliced Bun.file() response gets an automatic 206 (RequestContext::render_metadata). The last commit replaces that overloaded signal (see the second fix below), but preservation at consumption time is still the right shape for Response: it is the only approach that also covers the Responses fetch() builds internally for data:, file:, and blob: URLs (which never pass through the JS constructor), and it keeps new Response(body) on the Bun.serve path from allocating a FetchHeaders nothing reads. So BodyMixin gains preserve_body_content_type, a gated default that materializes the lazy headers only when they do not exist yet and the body is a Value::Blob with a non-empty content type; every body-consuming method (get_text, get_json, get_array_buffer, get_bytes, get_form_data, get_blob_with_this_value, and get_text_stream, which main added while this PR was open) and the .body getter call it right before the Blob is taken. Request and Response each supply a one-line materialize_headers that routes to their existing lazy materializer. For a Request this hook is now a no-op (the headers are already materialized), which its doc comment states.

Response external consumers that bypass BodyMixin (Bun.write(dest, response), HTMLRewriter.transform, the shell, WebAssembly.compileStreaming, new Blob([response]), and the Bun.serve send path) are intentionally not hooked: they consume the Response as an input rather than as the headers-then-body proxy shape this fixes. The internal jsFunctionGetCompleteRequestOrResponseBodyValueAsArrayBuffer is also not hooked; it has no JS call site.

Verification

test/js/web/fetch/body.test.ts, under "content-type survives reading the body before the headers", covers Request and Response x {FormData, URLSearchParams, typed Blob} x {arrayBuffer, bytes, text, blob, json, formData, draining .body, draining .textStream()}, asserts the header matches the headers-first order, asserts the multipart boundary in the header is the one actually used in the body bytes, and asserts an explicit content-type from the init is not overridden. A separate test, "Request content-type survives fetch(request) and matches the wire", fetches the Request to a local Bun.serve and asserts the user-held Request's boundary is non-null and identical to the Content-Type the server received and to the boundary in the body bytes.

A third block, "fetch() non-remote response content-type survives body consumption", covers the three fetch() URL schemes that build a Response whose Content-Type lives only on the body Blob and so had the identical bug: data:, file:, and blob: URLs each had headers.get("content-type") return null if the body was read first. The BodyMixin hook fixes them; these tests pin that down.

Against unmodified main plus only the tests:

  • bun bd test test/js/web/fetch/body.test.ts -t "content-type survives": 52 fail, 8 pass (the 8 are the explicit-header cases and Request's .body path, which already worked)
  • bun bd test test/js/web/fetch/body.test.ts -t "via draining .textStream": 6 fail, 0 pass
  • bun bd test test/js/web/fetch/body.test.ts -t "survives fetch": 1 fail

With the fix (numbers as of the rebase onto current main):

  • bun bd test test/js/web/fetch/body.test.ts: 512 pass, 0 fail
  • bun bd test test/js/bun/http/serve.test.ts -t "Content-Range": 42 pass, 0 fail
  • bun bd test test/js/bun/http/bun-serve-file.test.ts: 105 pass, 0 fail
  • bun bd test test/js/bun/http/fetch-file-upload.test.ts: 11 pass, 0 fail

Rebase notes

Rebased onto current main as a single commit. Three source conflicts, all resolved in favor of keeping both sides:

  • RequestContext.rs: main moved sendfile into a Cell<SendfileContext> read once into a local, so the 206 condition now uses that local instead of self.sendfile.total.
  • Body.rs: main added stream-ownership bookkeeping (check_body_stream_ref) to the .body getter; the preserve_body_content_type call sits before it. Main also added a new consumer, get_text_stream (.textStream()), which takes the Blob the same way, so it gets the same call and a row in the test matrix (fails 6/6 without the fix).
  • Response.rs: main narrowed the Init fields to pub(crate); the new headers_from_init field follows suit.

Also verified that fetch() with a FormData body (both the options-object and the fetch(new Request(...)) shapes) still sends a single multipart/form-data; boundary=... header whose boundary matches the body bytes, that request.clone() and the original keep the same boundary, and that typeless bodies (plain string, Blob without a type) still correctly get no Content-Type.

Second fix: Bun.serve auto-206 for a sliced blob keyed on the wrong signal

Found while landing the above. Bun.serve decides whether a .slice()-driven Bun.file() response should auto-promote to 206 with a Content-Range by checking whether Response.init.headers exists. That signal is overloaded: init.headers is also created by the lazy headers getter, and (with this PR) by the Content-Type preservation above, neither of which means the caller supplied a headers init.

The lazy-getter case is a pre-existing bug, reproducible on unmodified bun:

Bun.serve({
  fetch() {
    const response = new Response(Bun.file(path).slice(0, 10));
    response.headers.get("x-not-set"); // any read of .headers
    return response;                   // 200, should be 206
  },
});

Response::Init gains a headers_from_init flag, set inside Init::init (the one place a user ResponseInit is parsed, so new Response(body, init), Response.json(data, init), and Response.redirect(url, init) are all covered, not just the constructor). Bun.serve keys the auto-206 decision on it instead of on init.headers being present.

Init::init has three paths (a Request as the init, a Response as the init, and a plain object). Each computes headers_from_init as result.headers.is_some() for itself. The Response-as-init path clones the donor's Init for the other fields but must not inherit the donor's headers_from_init: the donor may have materialized its own headers lazily, but new Response(slice, donor) is still a headers-carrying init from this caller's point of view. An earlier revision did inherit it via Init::clone, which would have regressed that shape from 200 to 206; Init::clone is otherwise unchanged since its other caller is response.clone(), where the verbatim copy is correct.

This is also what makes it safe for anything other than a user init to create init.headers at all, and it removes the one consumer that depended on a Response's headers staying unmaterialized.

New tests in test/js/bun/http/serve.test.ts under "should support Content-Range with Bun.file()":

  • "206 for a slice is not defeated by reading response.headers": fails on unmodified bun (200), passes with the fix.
  • "slice with a headers init and no Content-Range stays 200": guards the preserved contract, that a caller who supplies a headers init without a Content-Range is managing the response themselves. Passes before and after.
  • "slice with a Response as the headers-carrying init stays 200": the Response-as-init shape above. Passes on unmodified bun and with the fix, and would have failed on the revision that inherited headers_from_init from the donor.

With the fix, bun bd test test/js/bun/http/serve.test.ts -t "Content-Range": 42 pass, 0 fail.

no test proof · iteration 10 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts test/js/web/fetch/body.test.ts