Response.redirect(""): store an empty Location value instead of a null one

[robobun]

Problem

• Response.redirect("") (or Response.redirect()) gives headers where has("location") is true but get("location") is null and iteration, forEach and new Headers(h) skip it. new Headers({ location: "" }) reads back as "" everywhere.
• Cause: the native header put path stores an empty value as WTF's null string, and the header map reads a null value as "absent" in get() and iteration but not in has().
• Only put is affected; the JS set / append / constructor paths go through WebIDL and never produce a null string. Predates Response.redirect: parse and serialize the url into the Location header #33126.

Fix

• put swaps a null conversion result for the empty string before storing. Other put callers skip empty values or never pass one, so only Response.redirect and the dev server's Response.render("") change.
• Check: Response.redirect("").headers now equals new Headers({ location: "" }). The wire already sent an empty Location:, so only the in-memory object changes.
• Storing "" rather than throwing follows Bun's rule of keeping non-absolute redirect targets verbatim; Node throws here, but also throws for Response.redirect("/login"), which Bun accepts.
• Verification: 7 new test cases fail on the released binary and pass with the change; existing headers and dev server tests still pass on a debug build.

Background

• Bun's Headers is WebKit's FetchHeaders over an HTTPHeaderMap, which treats a null value as "absent" in get() and its iterator while contains() (behind has()) still finds the entry.
• WTF::String has a null string distinct from the empty string; both have length 0, only isNull() differs.
• BunString is the string type crossing the Zig/C++ boundary. Its one Empty tag converts to the null WTF::String.
• WebCore__FetchHeaders__put is the binding Zig uses to set a header directly, bypassing the WebIDL conversion that JS headers.set() gets.

Original description

Reproduction

const h = Response.redirect("").headers;   // same with Response.redirect()
h.has("location");   // true
h.get("location");   // null      (expected "")
[...h];              // []        (expected [["location", ""]])

size() and has() say the header exists, but get() returns null and entries() / forEach / new Headers(h) never yield it. headers.set("location", ""), new Headers({ location: "" }) and Response.redirect(" ") (trimmed to empty) all produce a Headers object that reads back as "" everywhere.

Cause

WebCore__FetchHeaders__put (bindings.cpp) converts the value with BunString::toWTFString(). A BunString has exactly one representation of the empty string, the Empty tag, and toWTFString() maps it to the null WTF::String. FetchHeaders::set stores it as-is, and HTTPHeaderMap uses a null value to mean "no such header" in get() and in FetchHeaders::Iterator::next(), while contains() still finds the entry. put is the only way a BunString reaches the header map; the JS-facing set / append / constructor paths go through WebIDL conversion and always get a non-null string, which is why headers.set("location", "") works.

This predates #33126: the previous Zig::toStringCopy on a zero-length ZigString returned the null String as well.

Fix

The binding turns a null conversion result into WTF::emptyString() before calling FetchHeaders::set. That covers every put caller with an empty value (Response.redirect(""), Response.redirect() and Response.render("") in the dev server); the content-type callers already skip empty values and the S3 Location is never empty, so nothing else changes.

Storing "" rather than throwing is the consistent choice for Bun: non-absolute redirect targets are documented to be stored verbatim, and "" is just the degenerate one. Node throws a TypeError here, but it throws for Response.redirect("/login") for the same reason, which Bun deliberately accepts, so rejecting only "" would be an arbitrary carve-out. The response on the wire already sent Location: with an empty value before this change (toUWSResponse writes the entry), so this only makes the in-memory Headers object agree with what is sent and with every other way of producing the same header.

Tests

test/js/web/fetch/response.test.ts: each Response.redirect arity with an empty url (plus no argument, and clone()) is checked through has(), get(), iteration, forEach and copying into a new Headers, and compared against new Headers({ location: "" }). All 7 new cases fail on the released binary and pass with this change. The self-referential print size snapshot in the same file is bumped for the added bytes, as in earlier changes to this file.

Also ran headers.test.ts, fetch_headers.test.js, headers-case.test.ts, headers.undici.test.ts and test/bake/dev/react-response.test.ts (covers Response.render / Response.redirect in the dev server) against the debug build with no failures.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5d732d1d-ecd6-4c0e-bb78-bd46fb7d7006

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and fd69355.

📒 Files selected for processing (2)

• src/jsc/bindings/bindings.cpp
• test/js/web/fetch/response.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 3:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit fd69355 has 3 failures in Build #93407 (All Failures):

• test/js/third_party/grpc-js/test-tonic.test.ts - code 1 on 🐧 13 x64-asan
• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 13 x64-asan
• scripts/build/ci.ts - build failed on 🍎 x64 - build-bun
• scripts/build/ci.ts - build failed on :freebsd: aarch64 - build-bun
• scripts/build/ci.ts - build failed on :freebsd: x64 - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 37893

That installs a local version of the PR into your bun-37893 executable, so you can run:

bun-37893 --bun

[robobun]

Status: fix pushed, waiting on CI. Automated review raised no concerns; a second review pass is in progress and anything it turns up will be addressed here.

Reproduced on bun 1.4.0 with Response.redirect("").headers: has("location") is true, get("location") is null, [...headers] is empty. Cause and fix are in the PR description; the 7 new cases in test/js/web/fetch/response.test.ts fail on the released binary and pass with this branch.

[claude]

LGTM — a targeted 4-line normalization at the FFI boundary, following the same isNull() → emptyString() pattern already used elsewhere in the bindings.

What was reviewed:

• Confirmed BunString::toWTFString() maps the Empty tag to a null WTF::String, and that the fix mirrors the existing tag == Empty ? emptyString() : toWTFString() idiom at bindings.cpp:4210, bindings.cpp:4601, BunString.cpp:909/944.
• Audited all FetchHeaders::put / put_default callers on the Rust side: the ContentType sites already guard on !is_empty(), so only the two Location writers (Response.redirect and BakeResponse render/redirect) reach this path with an empty value — matches the PR description.
• Tests cover every Response.redirect arity plus clone(), and exercise has/get/iteration/forEach/copy against the new Headers({ location: "" }) reference; snapshot bump is the expected self-referential file-size change.

Extended reasoning...

Overview

The PR fixes an inconsistency where Response.redirect("") produces a Headers object whose Location entry is visible to has() but returns null from get() and is skipped by iteration. The root cause is that WebCore__FetchHeaders__put converts the incoming BunString with toWTFString(), which maps the Empty tag to a null WTF::String; HTTPHeaderMap then stores a null value that contains() treats as present but get() and the iterator treat as absent. The fix inserts a single null-to-empty normalization before FetchHeaders::set, and adds a test.each covering every Response.redirect arity (plus clone()) that reaches this path.

Security risks

None. This is a header-value normalization inside an internal C++/Rust binding that is only reached by Bun's own Response.redirect / Response.render / content-type paths, not by user JS (the JS-facing Headers.set goes through WebIDL and already produces non-null strings). The wire behavior (toUWSResponse) already emitted Location: for this case, so no new response content is exposed — only the in-memory Headers object is brought into agreement.

Level of scrutiny

Low-to-medium. The C++ change is 4 lines and is a direct instance of an idiom already used at four other sites in the same bindings layer (key->tag == BunStringTag::Empty ? WTF::emptyString() : key->toWTFString()). Using value.isNull() after conversion is equivalent and slightly more robust (also covers a Dead tag). No allocation, ownership, or exception-scope changes; the existing throwScope / propagateException structure is preserved.

Other factors

I audited every HTTPHeaderName::* put/put_default caller in src/runtime/webcore/ and src/runtime/server/: all ContentType writers gate on !content_type.is_empty(), and the S3-response Location writer uses a non-empty resolved URL, so the only behavioral change lands on Response.redirect("")/Response.redirect() and the dev-server Response.render("") — exactly as the PR description claims. The design decision to store "" rather than throw is consistent with Bun's documented acceptance of non-absolute redirect targets (Response.redirect("/login")), so no new API surface is introduced. Test coverage is strong: 7 parametrized cases asserting through five independent read paths, plus an equality check against new Headers({ location: "" }), all stated to fail on the released binary. The print size snapshot bump is the standard consequence of this file's self-referential size test.