Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/jsc/FetchHeaders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use core::ptr::NonNull;

use crate::virtual_machine::VirtualMachine;
use crate::{JSGlobalObject, JSValue, JsResult, VM, host_fn};
use bun_core::{StringPointer, ZigString};
use bun_core::{String as BunString, StringPointer, ZigString};
use bun_uws::ResponseKind;

bun_opaque::opaque_ffi! {
Expand All @@ -14,8 +14,9 @@ bun_opaque::opaque_ffi! {
// `FetchHeaders`/`JSGlobalObject`/`VM` are opaque `UnsafeCell`-backed ZST
// handles, so `&T` is ABI-identical to a non-null `*const T` and C++ mutating
// header storage / VM state through them is interior mutation invisible to
// Rust. `ZigString` is a plain `#[repr(C)]` POD; `&ZigString`/`&mut ZigString`
// at the FFI boundary are sound (C++ reads/writes only the named struct).
// Rust. `ZigString` and `String` (`BunString`) are plain `#[repr(C)]` PODs;
// `&`/`&mut` refs to them at the FFI boundary are sound (C++ reads/writes
// only the named struct).
// Shims that traffic only in such refs + scalars are declared `safe fn`; those
// that take raw `*mut c_void` / unsized `*mut StringPointer` arrays / `deref`
// (which may free) keep their `unsafe fn` body.
Expand Down Expand Up @@ -102,7 +103,7 @@ unsafe extern "C" {
safe fn WebCore__FetchHeaders__put(
this: &FetchHeaders,
name_: HTTPHeaderName,
value: &ZigString,
value: &BunString,
global: &JSGlobalObject,
);
}
Expand Down Expand Up @@ -151,7 +152,7 @@ impl FetchHeaders {
pub fn put_default(
&mut self,
name_: HTTPHeaderName,
value: &[u8],
value: &BunString,
global: &JSGlobalObject,
) -> JsResult<()> {
if self.fast_has(name_) {
Expand Down Expand Up @@ -231,15 +232,16 @@ impl FetchHeaders {
WebCore__FetchHeaders__append(self, name_, value, global)
}

/// `value`'s tag carries its encoding, and a `WTFStringImpl`-tagged value
/// is ref'd by the C++ side instead of copied character-by-character.
pub fn put(
&mut self,
name_: HTTPHeaderName,
value: &[u8],
value: &BunString,
global: &JSGlobalObject,
) -> JsResult<()> {
host_fn::from_js_host_call_generic(global, || {
let zs = ZigString::init(value);
WebCore__FetchHeaders__put(self, name_, &zs, global)
WebCore__FetchHeaders__put(self, name_, value, global)
})
}

Expand Down
5 changes: 3 additions & 2 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2103,11 +2103,12 @@ bool WebCore__FetchHeaders__has(WebCore::FetchHeaders* headers, const ZigString*
} else
return result.releaseReturnValue();
}
extern "C" void WebCore__FetchHeaders__put(WebCore::FetchHeaders* headers, HTTPHeaderName name, const ZigString* arg2, JSC::JSGlobalObject* global)
extern "C" void WebCore__FetchHeaders__put(WebCore::FetchHeaders* headers, HTTPHeaderName name, const BunString* arg2, JSC::JSGlobalObject* global)
{
auto throwScope = DECLARE_THROW_SCOPE(global->vm());
throwScope.assertNoException(); // can't throw an exception when there's already one.
WebCore::propagateException(*global, throwScope, headers->set(name, Zig::toStringCopy(*arg2)));
// `toWTFString()` refs a `WTFStringImpl`-tagged value instead of copying it.
WebCore::propagateException(*global, throwScope, headers->set(name, arg2->toWTFString()));
}
void WebCore__FetchHeaders__remove(WebCore::FetchHeaders* headers, const ZigString* arg1, JSC::JSGlobalObject* global)
{
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/webcore/FetchHeaders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ static ExceptionOr<bool> canWriteHeader(const HTTPHeaderName name, const String&
{
ASSERT(value.isEmpty() || (!isHTTPSpace(value[0]) && !isHTTPSpace(value[value.length() - 1])));
if (!isValidHTTPHeaderValue((value)))
return Exception { TypeError, makeString("Header '"_s, name, "' has invalid value: '"_s, value, "'"_s) };
return Exception { TypeError, makeString("Header '"_s, httpHeaderNameString(name), "' has invalid value: '"_s, value, "'"_s) };
if (guard == FetchHeaders::Guard::Immutable)
return Exception { TypeError, "Headers object's guard is 'immutable'"_s };
return true;
Expand Down
5 changes: 1 addition & 4 deletions src/runtime/webcore/BakeResponse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,13 @@ pub(crate) fn construct_render(
// Get the path string
let path_str = bun_core::OwnedString::new(path_arg.to_bun_string(global_this)?);

let path_utf8 = path_str.to_utf8();
// `defer path_utf8.deinit()` → handled by Drop on the UTF-8 slice guard

// Create a Response with Render body
let response = Box::new(Response::init(
Init {
status_code: 200,
headers: {
let mut headers = HeadersRef::create_empty();
headers.put(HTTPHeaderName::Location, path_utf8.slice(), global_this)?;
headers.put(HTTPHeaderName::Location, &path_str, global_this)?;
Some(headers)
},
..Default::default()
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/webcore/Request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ impl Request {
if !content_type_.is_empty() {
self.headers_mut().as_mut().unwrap().put(
HTTPHeaderName::ContentType,
content_type_,
&BunString::ascii(content_type_),
global_this,
)?;
}
Expand Down Expand Up @@ -1481,7 +1481,7 @@ impl Request {
match req.headers_mut().as_mut().unwrap().put(
HTTPHeaderName::ContentType,
// SAFETY: ct_ptr borrows req.body which is not mutated here.
unsafe { &*ct_ptr },
&BunString::ascii(unsafe { &*ct_ptr }),
global_this,
) {
Ok(()) => {}
Expand Down
45 changes: 27 additions & 18 deletions src/runtime/webcore/Response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ impl Response {
if !content_type.is_empty() {
init.headers.as_mut().unwrap().put(
HTTPHeaderName::ContentType,
content_type,
&BunString::ascii(content_type),
global_this,
)?;
}
Expand Down Expand Up @@ -1027,7 +1027,7 @@ impl Response {
let json_mime = bun_http_types::MimeType::JSON;
headers_ref.put_default(
HTTPHeaderName::ContentType,
json_mime.value.as_ref(),
&BunString::ascii(json_mime.value.as_ref()),
global_this,
)?;
// Disarm the body-reset guard: all fallible ops have succeeded.
Expand Down Expand Up @@ -1074,8 +1074,8 @@ impl Response {
let mut args =
bun_jsc::ArgumentsSlice::init(global_this.bun_vm(), &args_list.ptr[0..args_list.len]);

let url_string_slice;
// url_string_slice drops at scope exit
// url_string drops (derefs the WTF string) at scope exit
let url_string: OwnedString;
let response: Response = 'brk: {
let response = Response {
init: JsCell::new(Init {
Expand All @@ -1087,12 +1087,11 @@ impl Response {
};

let url_string_value = args.next_eat().unwrap_or(JSValue::ZERO);
let mut url_string = ZigString::init(b"");

if !url_string_value.is_empty() {
url_string = url_string_value.get_zig_string(global_this)?;
}
url_string_slice = url_string.to_slice();
url_string = OwnedString::new(if url_string_value.is_empty() {
BunString::empty()
} else {
url_string_value.to_bun_string(global_this)?
});
// `Init`'s drop glue (HeadersRef + OwnedString)
// handles cleanup on `?`.

Expand Down Expand Up @@ -1122,13 +1121,15 @@ impl Response {
break 'brk response;
};

let headers = response.get_or_create_headers(global_this)?;
// `get_or_create_headers` already populated init.headers.
headers.put(
HTTPHeaderName::Location,
url_string_slice.slice(),
global_this,
)?;
let headers = response.get_or_create_headers(global_this)?;
// https://fetch.spec.whatwg.org/#dom-response-redirect steps 1 & 6: `Location`
// gets the serialization of the parsed url, not the raw input. Non-absolute
// input keeps the raw string: relative redirects are documented Bun behavior.
let href = OwnedString::new(bun_url::href_from_string(&url_string));
// The JS string's own WTF string (no re-encode), same as `Headers.prototype.set`.
let location = if href.is_empty() { &url_string } else { &href };
headers.put(HTTPHeaderName::Location, location, global_this)?;
Ok(response)
}

Expand Down Expand Up @@ -1213,7 +1214,11 @@ impl Response {
// `defer result.deinit()` — SignResult: Drop frees owned buffers at scope exit.
response.redirected.set(true);
let headers = response.get_or_create_headers(global_this)?;
headers.put(HTTPHeaderName::Location, &result.url, global_this)?;
headers.put(
HTTPHeaderName::Location,
&BunString::ascii(&result.url),
global_this,
)?;
return Ok(bun_core::heap::into_raw(Box::new(response)));
}
}
Expand Down Expand Up @@ -1265,7 +1270,11 @@ impl Response {
if let Some(headers) = init.headers.as_deref_mut() {
let content_type = blob.content_type_slice();
if !content_type.is_empty() && !headers.fast_has(HTTPHeaderName::ContentType) {
headers.put(HTTPHeaderName::ContentType, content_type, global_this)?;
headers.put(
HTTPHeaderName::ContentType,
&BunString::ascii(content_type),
global_this,
)?;
}
}
}
Expand Down
20 changes: 11 additions & 9 deletions test/js/web/fetch/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1216,16 +1216,18 @@ describe("Response", () => {
});
describe("Response.redirect", () => {
it("works", () => {
// Location is the serialization of the parsed url, so an empty path
// gains a trailing "/". https://fetch.spec.whatwg.org/#dom-response-redirect
const inputs = [
"http://example.com",
"http://example.com/",
"http://example.com/hello",
"http://example.com/hello/",
"http://example.com/hello/world",
"http://example.com/hello/world/",
["http://example.com", "http://example.com/"],
["http://example.com/", "http://example.com/"],
["http://example.com/hello", "http://example.com/hello"],
["http://example.com/hello/", "http://example.com/hello/"],
["http://example.com/hello/world", "http://example.com/hello/world"],
["http://example.com/hello/world/", "http://example.com/hello/world/"],
];
for (let input of inputs) {
expect(Response.redirect(input).headers.get("Location")).toBe(input);
for (const [input, expected] of inputs) {
expect(Response.redirect(input).headers.get("Location")).toBe(expected);
}
});

Expand All @@ -1239,7 +1241,7 @@ describe("Response", () => {
status: 307,
});
expect(response.headers.get("x-hello")).toBe("world");
expect(response.headers.get("Location")).toBe("https://example.com");
expect(response.headers.get("Location")).toBe("https://example.com/");
expect(response.status).toBe(307);
expect(response.type).toBe("default");
expect(response.ok).toBe(false);
Expand Down
7 changes: 7 additions & 0 deletions test/js/web/fetch/fetch_headers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ describe("Headers", async () => {
expect(() => fetch(url, { headers: { "x-test": "❤️" } })).toThrow("Header 'x-test' has invalid value: '❤️'");
});

it("Invalid values for well-known headers name the header, not its index", () => {
// The HTTPHeaderName fast path must report the header's name (e.g. 'Location'),
// not its numeric enum value (e.g. '51').
expect(() => new Headers({ location: "a\nb" })).toThrow("Header 'Location' has invalid value: 'a\nb'");
expect(() => new Headers({ "content-type": "\0" })).toThrow("Header 'Content-Type' has invalid value: '\0'");
});

it("repro 1602", async () => {
const origString = "😂1234".slice(3);

Expand Down
41 changes: 40 additions & 1 deletion test/js/web/fetch/response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("2-arg form", () => {
test("print size", () => {
expect(normalizeBunSnapshot(Bun.inspect(new Response(Bun.file(import.meta.filename)))), import.meta.dir)
.toMatchInlineSnapshot(`
"Response (5.83 KB) {
"Response (8.0 KB) {
ok: true,
url: "",
status: 200,
Expand Down Expand Up @@ -100,6 +100,45 @@ test("Response.redirect status code validation", () => {
expect(Response.redirect("url", { status: 308 }).status).toBe(308);
});

// https://fetch.spec.whatwg.org/#dom-response-redirect
// `Location` gets the serialization of the parsed url, not the raw input string.
test.each([
// percent-encoding
["http://example.com/a b", "http://example.com/a%20b"],
["http://x/é", "http://x/%C3%A9"],
// ASCII tab and newline are stripped by the URL parser instead of
// surfacing as a header-validation TypeError
["http://x/a\nb", "http://x/ab"],
["http://x/a\tb", "http://x/ab"],
// scheme/host lowercased, default port removed, dot-segments resolved
["HTTP://U:P@EX.COM:80/p/../q", "http://U:P@ex.com/q"],
// empty path serializes as "/"
["http://example.com", "http://example.com/"],
// IDN host is punycode-encoded
["http://bücher.example/", "http://xn--bcher-kva.example/"],
])("Response.redirect(%j) serializes the url into Location", (input, expected) => {
expect(Response.redirect(input).headers.get("location")).toBe(expected);
// every arity takes the same path into the Location header
expect(Response.redirect(input, 307).headers.get("location")).toBe(expected);
expect(Response.redirect(input, { status: 308 }).headers.get("location")).toBe(expected);
});

test("Response.redirect keeps a non-absolute url as-is in Location", () => {
// Relative redirect targets are documented Bun behavior (see docs/runtime/http).
expect(Response.redirect("/login").headers.get("location")).toBe("/login");
expect(Response.redirect("/login?next=1#a").headers.get("location")).toBe("/login?next=1#a");
// non-ASCII must round-trip, not come back as a latin-1 view of the UTF-8 bytes
expect(Response.redirect("/café").headers.get("location")).toBe("/café");
});

test("Response.redirect rejects a non-absolute url that is not a valid header value", () => {
// A code point above U+00FF cannot be a header value, so this throws the same
// TypeError that `new Headers({ location: "/€" })` does, instead of silently
// writing a latin-1-corrupted Location ("/â¬").
expect(() => Response.redirect("/€")).toThrow("Header 'Location' has invalid value: '/€'");
expect(() => Response.redirect("/搜索")).toThrow("Header 'Location' has invalid value: '/搜索'");
});

test("new Response(123, { statusText: 123 }) does not throw", () => {
// @ts-expect-error
expect(new Response("123", { statusText: 123 }).statusText).toBe("123");
Expand Down
4 changes: 3 additions & 1 deletion test/vendor.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
"repository": "https://github.com/elysiajs/elysia",
"tag": "1.4.28",
"skipTests": {
"ws*connection.test.ts": "TEMPORARY: elysia 1.4.28 asserts wasClean=false for a server-initiated ws.close(), but that's a clean Close handshake — Bun now reports wasClean=true to match Node/WHATWG (oven-sh/bun#31518). Fixed upstream in elysiajs/elysia#1908; remove this skip on the next elysia bump. (glob * = path separator, so it also matches Windows backslash paths)"
"ws*connection.test.ts": "TEMPORARY: elysia 1.4.28 asserts wasClean=false for a server-initiated ws.close(), but that's a clean Close handshake — Bun now reports wasClean=true to match Node/WHATWG (oven-sh/bun#31518). Fixed upstream in elysiajs/elysia#1908; remove this skip on the next elysia bump. (glob * = path separator, so it also matches Windows backslash paths)",
"adapter*web-standard*map-response.test.ts": "TEMPORARY: Bun's Response.redirect now puts the WHATWG serialization of the url into Location (oven-sh/bun#33126), matching Node and https://fetch.spec.whatwg.org/#dom-response-redirect, so Response.redirect('https://cunny.school') yields 'https://cunny.school/'. elysia 1.4.28's 'map redirect' test asserts the unserialized string; remove this skip once the upstream assertion expects the trailing slash.",
"adapter*web-standard*map-early-response.test.ts": "TEMPORARY: same as adapter*web-standard*map-response.test.ts; its 'map redirect' test asserts the unserialized Response.redirect Location value."
}
}
]
Loading