diff --git a/docs/runtime/html-rewriter.mdx b/docs/runtime/html-rewriter.mdx index ad70d1ca7f9e..044c0b0bc554 100644 --- a/docs/runtime/html-rewriter.mdx +++ b/docs/runtime/html-rewriter.mdx @@ -342,6 +342,7 @@ rewriter.onDocument({ When transforming a Response, HTMLRewriter: - Preserves the status code, headers, and other response properties +- Sets `Content-Type` from the input body when no header gives one: the `type` of a `Bun.file()` or `Blob` body, or `text/plain;charset=utf-8` for a string body - Transforms the body while maintaining streaming capabilities - Handles content-encoding (like gzip) automatically - Marks the original response body as used after transformation diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 59eb0e840910..c16382614269 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -22,6 +22,7 @@ use bun_sys::Error as SysError; use crate::api::native_promise_context; use crate::generated_classes::{js_HTMLRewriterTransform, js_Response}; use crate::webcore::blob::SizeType as BlobSizeType; +use crate::webcore::response::HeadersRef; use crate::webcore::sink::JSSink; use crate::webcore::streams::{ self, SourceHandle, Start, StartTag, StreamError, StreamResult, Writable, WritablePending, @@ -963,6 +964,19 @@ impl RewriterPipe { original: &Response, sync_only_noun: Option<&'static str>, ) -> JsResult { + // Taken before `wire_input` consumes the body its Content-Type may derive from (#3334). + let mut init = original.clone_init(global)?; + // A string body is `text/plain` by itself; the Response overload's output body is not. + if sync_only_noun.is_none() && original.get_body_value().was_string() { + init.headers + .get_or_insert_with(HeadersRef::create_empty) + .put_default( + jsc::HTTPHeaderName::ContentType, + &BunString::ascii(&bun_http_types::MimeType::TEXT.value), + global, + )?; + } + let pipe = bun_core::heap::alloc_nn(RewriterPipe { global: GlobalRef::from(global), cell: Cell::new(JSValue::ZERO), @@ -1023,10 +1037,7 @@ impl RewriterPipe { // the sink buffers into `output_buffer`, and `on_start_streaming` // hands that over as `DrainResult::Owned`. let result = bun_core::heap::alloc_nn(Response::init( - webcore::response::Init { - status_code: 200, - ..Default::default() - }, + init, webcore::Body::new({ let mut pv = webcore::body::PendingValue::new(global); pv.task = Some(pipe.cast::()); @@ -1044,15 +1055,6 @@ impl RewriterPipe { this.response .set(Some(unsafe { RefPtr::init_ref(result.as_ptr()) })); - result_ref.set_init( - original.get_method(), - original.get_init_status_code(), - original.get_init_status_text().clone(), - ); - - // https://github.com/oven-sh/bun/issues/3334 - result_ref.set_init_headers(original.clone_init_headers(global)?); - let response_js_value = result_ref.to_js(&this.global); // Hand ownership of `pipe` to its `JSHTMLRewriterTransform` wrapper cell. diff --git a/src/runtime/webcore/Request.rs b/src/runtime/webcore/Request.rs index 8ceaadd119dd..24f0599096f8 100644 --- a/src/runtime/webcore/Request.rs +++ b/src/runtime/webcore/Request.rs @@ -199,7 +199,7 @@ impl Request { /// Immutable view of the body value. #[inline] - fn body_value(&self) -> &BodyValue { + pub(crate) fn body_value(&self) -> &BodyValue { &self.body } @@ -1157,18 +1157,16 @@ impl Request { } if !fields.contains(Fields::Headers) { - if let Some(headers) = response.get_init_headers_mut() { - // The flag is set unconditionally once `getInitHeaders()` yielded a - // value, even if `cloneThis` returns null — so a later arg can't - // repopulate headers from a different source. - match headers.clone_this(global_this) { - Ok(h) => { - // SAFETY: clone_this returns a +1 ref FetchHeaders. - req.headers.set(h.map(|p| unsafe { HeadersRef::adopt(p) })); + // Flagged whenever the Response had headers (even if the copy came back null) so a later arg can't repopulate them. + let had_headers = response.get_init_headers().is_some(); + match response.clone_headers(global_this) { + Ok(headers) => { + if had_headers || headers.is_some() { + req.headers.set(headers); fields.insert(Fields::Headers); } - Err(e) => bail!(Err(e)), } + Err(e) => bail!(Err(e)), } } diff --git a/src/runtime/webcore/Response.rs b/src/runtime/webcore/Response.rs index 79b0121bddab..5576ece31678 100644 --- a/src/runtime/webcore/Response.rs +++ b/src/runtime/webcore/Response.rs @@ -84,6 +84,24 @@ impl HeadersRef { .clone_this(global)? .map(|p| unsafe { Self::adopt(p) })) } + + /// What a lazy `.headers` getter creates for `body`: its Blob's `Content-Type` (`Bun.file()` mime, `Blob.type`), or `None`. + pub(crate) fn for_body(body: &BodyValue, global: &JSGlobalObject) -> JsResult> { + let BodyValue::Blob(blob) = body else { + return Ok(None); + }; + let content_type = blob.content_type_slice(); + if content_type.is_empty() { + return Ok(None); + } + let mut headers = Self::create_empty(); + headers.put( + HTTPHeaderName::ContentType, + &BunString::ascii(content_type), + global, + )?; + Ok(Some(headers)) + } } impl core::ops::Deref for HeadersRef { @@ -324,31 +342,6 @@ impl Response { } } - #[inline] - pub(crate) fn set_init(&self, method: Method, status_code: u16, status_text: BunString) { - self.init.with_mut(|init| { - init.method = method; - init.status_code = status_code; - init.status_text = status_text; - }); - } - - #[inline] - pub(crate) fn set_init_headers(&self, headers: Option) { - // old headers dropped (HeadersRef::Drop derefs the C++ handle) - self.init.with_mut(|init| init.headers = headers); - } - - #[inline] - pub(crate) fn get_init_status_code(&self) -> u16 { - self.init.get().status_code - } - - #[inline] - pub(crate) fn get_init_status_text(&self) -> &BunString { - &self.init.get().status_text - } - #[inline] pub(crate) fn set_url(&self, url: BunString) { self.url.set(url); @@ -389,20 +382,25 @@ impl Response { self.init_mut().headers.as_deref_mut() } - /// Deep-copy this response's init headers (if any) into a fresh - /// `HeadersRef`. Centralises the `FetchHeaders::clone_this` + - /// `HeadersRef::adopt` pair so callers stay `unsafe`-free. - #[inline] - pub(crate) fn clone_init_headers( - &self, - global: &JSGlobalObject, - ) -> JsResult> { - match self.init_mut().headers.as_ref() { + /// Deep copy of what the `.headers` getter would report, without materializing it on `self`. + pub(crate) fn clone_headers(&self, global: &JSGlobalObject) -> JsResult> { + match self.init.get().headers.as_ref() { Some(headers) => headers.clone_this(global), - None => Ok(None), + None => HeadersRef::for_body(self.body.get().value.get(), global), } } + /// Status and what `.headers` would report, for a Response that takes them over with a new body (`new Response(body, response)`, `HTMLRewriter`). + pub(crate) fn clone_init(&self, global: &JSGlobalObject) -> JsResult { + let init = self.init.get(); + Ok(Init { + headers: self.clone_headers(global)?, + status_code: init.status_code, + status_text: init.status_text.clone(), + method: init.method, + }) + } + #[inline] pub(crate) fn swap_init_headers(&self) -> Option { self.init.with_mut(|init| init.headers.take()) @@ -589,26 +587,16 @@ impl Response { &self, global_this: &JSGlobalObject, ) -> JsResult<&mut HeadersRef> { + if self.init.get().headers.is_none() { + let headers = HeadersRef::for_body(self.body.get().value.get(), global_this)? + .unwrap_or_else(HeadersRef::create_empty); + self.init.with_mut(|init| init.headers = Some(headers)); + } + // R-2 escape hatch via `init_mut()` — the returned `&mut HeadersRef` // borrows `self.init`; callers (`get_headers`, `construct_*`) do not // hold the borrow across calls that re-enter Response host-fns. - let init = self.init_mut(); - if init.headers.is_none() { - init.headers = Some(HeadersRef::create_empty()); - - if let BodyValue::Blob(blob) = self.body.get().value.get() { - let content_type = blob.content_type_slice(); - if !content_type.is_empty() { - init.headers.as_mut().unwrap().put( - HTTPHeaderName::ContentType, - &BunString::ascii(content_type), - global_this, - )?; - } - } - } - - Ok(init.headers.as_mut().unwrap()) + Ok(self.init_mut().headers.as_mut().unwrap()) } pub(crate) fn get_headers(this: &Self, global_this: &JSGlobalObject) -> JsResult { @@ -1248,16 +1236,20 @@ impl Init { if js_type == JSType::DOMWrapper { // fast path: it's a Request object or a Response object - // we can skip calling JS getters + // we can skip calling JS getters, but must report what they would if let Some(req) = response_init.as_direct::() { // SAFETY: `as_direct` returned a live `*mut Request` owned by the // JS wrapper cell; the wrapper is rooted by `response_init` for // the duration of this call, so no GC can finalize it here. // Everything touched is `&self`. let req = unsafe { &*req }; - if let Some(headers) = req.get_fetch_headers_unless_empty() { - result.headers = headers.clone_this(global_this)?; - } + result.headers = match req.get_fetch_headers_unless_empty() { + Some(headers) => headers.clone_this(global_this)?, + None if !req.has_fetch_headers() => { + HeadersRef::for_body(req.body_value(), global_this)? + } + None => None, + }; result.method = req.method; return Ok(Some(result)); @@ -1267,7 +1259,7 @@ impl Init { // SAFETY: `as_direct` returned a live `*mut Response` owned by the // JS wrapper cell; rooted by `response_init` for this call. let resp = unsafe { &*resp }; - return Ok(Some(resp.init.get().clone(global_this)?)); + return Ok(Some(resp.clone_init(global_this)?)); } } diff --git a/test/js/web/fetch/response.test.ts b/test/js/web/fetch/response.test.ts index a4e286172358..ba490a94a6cb 100644 --- a/test/js/web/fetch/response.test.ts +++ b/test/js/web/fetch/response.test.ts @@ -44,12 +44,42 @@ describe("2-arg form", () => { expect(response.status).toBe(200); expect(response.statusText).toBe(""); }); + + // A Response or Request used as the init carries the headers its `.headers` + // getter reports, including a Content-Type that only its body Blob implies + // and that the getter would have added on first access. + test("a Response or Request init carries its body Blob's Content-Type", () => { + const typed = () => new Blob(["

hi

"], { type: "text/html" }); + const contentType = (init: Response | Request) => new Response("replaced", init).headers.get("content-type"); + + expect(contentType(new Response(typed(), { status: 201 }))).toBe("text/html;charset=utf-8"); + expect(new Response("replaced", new Response(typed(), { status: 201 })).status).toBe(201); + expect(contentType(new Request("http://example.com/", { method: "POST", body: typed() }))).toBe( + "text/html;charset=utf-8", + ); + // An explicit header wins, a deleted one stays deleted, an untyped body adds none. + expect(contentType(new Response(typed(), { headers: { "content-type": "text/x-custom" } }))).toBe("text/x-custom"); + const deleted = new Response(typed()); + deleted.headers.delete("content-type"); + expect(contentType(deleted)).toBe(null); + expect(contentType(new Response(new Blob(["

hi

"])))).toBe(null); + expect(contentType(new Response("a string"))).toBe(null); + // `new Request(input, init)` with a Response init takes its headers the same way. + expect(new Request("http://example.com/", new Response(typed())).headers.get("content-type")).toBe( + "text/html;charset=utf-8", + ); + expect( + new Request(new Request("http://example.com/", { method: "POST", body: "a" }), new Response(typed())).headers.get( + "content-type", + ), + ).toBe("text/html;charset=utf-8"); + }); }); test("print size", () => { expect(normalizeBunSnapshot(Bun.inspect(new Response(Bun.file(import.meta.filename)))), import.meta.dir) .toMatchInlineSnapshot(` - "Response (8.0 KB) { + "Response (9.75 KB) { ok: true, url: "", status: 200, diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index 38af7579ccb5..0a860fc40fe8 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -16,6 +16,7 @@ import { import { createServer as createTcpServer } from "net"; import path, { join } from "path"; import { setImmediate as setImmediatePromise } from "timers/promises"; +import { pathToFileURL } from "url"; var setTimeoutAsync = (fn, delay) => { return new Promise((resolve, reject) => { setTimeout(() => { @@ -1788,6 +1789,144 @@ it("#3334 regression", async () => { Bun.gc(true); }); +// A Response built without a `headers` init only reports its body's +// Content-Type once `.headers` is read, and a string body's `text/plain` only +// when Bun.serve sends it. The transformed Response's body is a stream, so +// transform() has to carry that header over itself. +describe("transform() carries the input Response's Content-Type", () => { + const rewrite = input => + new HTMLRewriter() + .on("p", { + element(element) { + element.setInnerContent("rewritten"); + }, + }) + .transform(input); + + const contentTypeAndBody = async response => ({ + contentType: response.headers.get("content-type"), + body: await response.text(), + }); + + it("Bun.file() body", async () => { + using dir = tempDir("html-rewriter-content-type", { "index.html": "

original

" }); + const response = rewrite(new Response(Bun.file(join(String(dir), "index.html")))); + expect(await contentTypeAndBody(response)).toEqual({ + contentType: "text/html;charset=utf-8", + body: "

rewritten

", + }); + }); + + it("typed Blob body", async () => { + const response = rewrite(new Response(new Blob(["

original

"], { type: "text/html" }))); + expect(await contentTypeAndBody(response)).toEqual({ + contentType: "text/html;charset=utf-8", + body: "

rewritten

", + }); + }); + + it("Response built by fetch() for a data: or file: URL", async () => { + using dir = tempDir("html-rewriter-content-type-fetch", { "index.html": "

original

" }); + for (const url of ["data:text/html,

original

", pathToFileURL(join(String(dir), "index.html"))]) { + const response = rewrite(await fetch(url)); + expect(await contentTypeAndBody(response)).toEqual({ + contentType: "text/html;charset=utf-8", + body: "

rewritten

", + }); + } + }); + + it("FormData body keeps the boundary of the encoded body", async () => { + const form = new FormData(); + form.append("field", "

original

"); + const { contentType, body } = await contentTypeAndBody(rewrite(new Response(form))); + expect(contentType).toStartWith("multipart/form-data; boundary="); + const boundary = contentType.slice("multipart/form-data; boundary=".length); + expect(body).toStartWith(`--${boundary}\r\n`); + expect(body).toContain("

rewritten

"); + }); + + it("headers init and the body's Content-Type are both carried", async () => { + const response = rewrite( + new Response(new Blob(["

original

"], { type: "text/html" }), { headers: { "x-custom": "1" } }), + ); + expect([...response.headers]).toEqual([ + ["content-type", "text/html;charset=utf-8"], + ["x-custom", "1"], + ]); + }); + + it("a Content-Type deleted from the input's headers stays deleted", async () => { + const input = new Response(new Blob(["

original

"], { type: "text/html" })); + input.headers.delete("content-type"); + expect(await contentTypeAndBody(rewrite(input))).toEqual({ contentType: null, body: "

rewritten

" }); + }); + + it("an untyped Blob body gets no Content-Type", async () => { + const response = rewrite(new Response(new Blob(["

original

"]))); + expect(await contentTypeAndBody(response)).toEqual({ contentType: null, body: "

rewritten

" }); + }); + + it("a string body is text/plain, as Bun.serve sends it", async () => { + expect(await contentTypeAndBody(rewrite(new Response("

original

")))).toEqual({ + contentType: "text/plain;charset=utf-8", + body: "

rewritten

", + }); + // Non-ASCII takes the UTF-8 re-encode path for the input. + expect(await contentTypeAndBody(rewrite(new Response("

original ☃

")))).toEqual({ + contentType: "text/plain;charset=utf-8", + body: "

rewritten

", + }); + }); + + it("a string body keeps the Content-Type of its headers init", async () => { + const response = rewrite(new Response("

original

", { headers: { "Content-Type": "text/html" } })); + expect([...response.headers]).toEqual([["content-type", "text/html"]]); + }); + + it("the string and ArrayBuffer overloads still return the body alone", () => { + expect(rewrite("

original

")).toBe("

rewritten

"); + const output = rewrite(new TextEncoder().encode("

original

").buffer); + expect(output).toBeInstanceOf(ArrayBuffer); + expect(new TextDecoder().decode(output)).toBe("

rewritten

"); + }); + + it("Bun.serve sends the carried Content-Type", async () => { + // `big.html` takes more than one file read, so its output is still + // streaming (chunked) when the server writes the headers. + const filler = Buffer.alloc(512 * 1024, "x").toString(); + const big = `

original

${filler}

original

`; + const bigRewritten = `

rewritten

${filler}

rewritten

`; + using dir = tempDir("html-rewriter-served-content-type", { "index.html": "

original

", "big.html": big }); + const inputs = { + "/file": () => new Response(Bun.file(join(String(dir), "index.html"))), + "/big-file": () => new Response(Bun.file(join(String(dir), "big.html"))), + "/blob": () => new Response(new Blob(["

original

"], { type: "text/html" })), + "/string": () => new Response("

original

"), + "/header": () => new Response("

original

", { headers: { "content-type": "text/html" } }), + }; + await using server = Bun.serve({ + port: 0, + fetch: req => rewrite(inputs[new URL(req.url).pathname]()), + }); + const served = {}; + for (const path of Object.keys(inputs)) { + const response = await fetch(new URL(path, server.url)); + let body = await response.text(); + if (path === "/big-file") + body = body === bigRewritten ? "(big.html rewritten)" : `unexpected ${body.length} bytes`; + served[path] = { contentType: response.headers.get("content-type"), body }; + } + expect(served).toEqual({ + "/file": { contentType: "text/html;charset=utf-8", body: "

rewritten

" }, + "/big-file": { contentType: "text/html;charset=utf-8", body: "(big.html rewritten)" }, + "/blob": { contentType: "text/html;charset=utf-8", body: "

rewritten

" }, + "/string": { contentType: "text/plain;charset=utf-8", body: "

rewritten

" }, + "/header": { contentType: "text/html", body: "

rewritten

" }, + }); + }); +}); + it("#3489", async () => { var el; await new HTMLRewriter()