From bd7f9835bb735d35e4128646882c500b88a1b21d Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 23 Jul 2026 23:12:04 +0000 Subject: [PATCH 01/13] HTMLRewriter: read the input body through ResumableSink instead of ValueBufferer HTMLRewriter.transform() rejected any Response whose body was a JS-created ReadableStream (start/enqueue source, TransformStream readable, async generator, type: 'direct') with ERR_STREAM_CANNOT_PIPE. The ValueBufferer::buffer_locked_body_value match on the stream source kind had: Source::JavaScript | Source::Direct => { // this is broken right now // return self.create_js_sink(stream); return Err(crate::Error::UnsupportedStreamType); } ValueBufferer was a bespoke 'read a whole body into one slice' helper whose only caller was HTMLRewriter, and it never finished implementing two of the five stream source kinds. The runtime already has the primitive it wanted to be: ResumableSink (src/runtime/webcore/ResumableSink.rs), a two-method write_request_data / write_end_request trait that handles every source kind (Source::Bytes via native Pipe, JavaScript/Direct/Blob/File via the JS pump Bun__assignStreamIntoResumableSink). fetch() request-body uploads and S3 multipart uploads are built on it. This swaps BufferOutputSink from ValueBufferer to ResumableSink: - A third codegen'd sink variant ResumableHTMLRewriterSink sits alongside ResumableFetchSink / ResumableS3UploadSink. - BufferOutputSink::start_reading_input does the Value dispatch ValueBufferer used to: materialised bodies (string/Blob/buffer/empty) run the rewrite immediately, a file-backed Blob schedules an async read, and any body carrying a ReadableStream goes to ResumableSink::init. - impl ResumableSinkContext for BufferOutputSink buffers each chunk and runs the rewrite once in write_end_request. This keeps the single write()+end() shape the nested-event-loop async-handler path still relies on; moving the write into write_request_data is the one-line follow-up for true streaming once that constraint is lifted. Deleted, all dead once ValueBufferer is gone: the ValueBufferer struct/impls (~430 lines of Body.rs), Bun__BodyValueBufferer__onResolveStream/onRejectStream and their PromiseFunctions enum entries (ZigGlobalObject.{h,cpp}, headers.h, headers-cpp.h), the NativePromiseContext BodyValueBufferer tag (Rust and C++), JSSink::detach_self, the ArrayBufferJSSink alias, and the crate::Error variants UnsupportedStreamType/StreamAlreadyUsed/InvalidStream. Tests land in test/js/workerd/html-rewriter.test.js: 18 new cases covering single/multi/mixed-chunk JS streams, direct streams, streams that produce after transform() returns, every promise-returning reader, handlers observing the document, upstream errors before/after transform() returns, bad chunk types, source reuse, handler-mutated source buffers, GC while the source is in flight, and the Bun.serve shape from #11758. The two pre-existing it.todo('works with payload of type direct'/'default') cases in that file were todo for exactly this reason and now pass. All 20 fail on main with ERR_STREAM_CANNOT_PIPE. Fixes #14216 Fixes #11758 --- src/jsc/bindings/NativePromiseContext.h | 1 - src/jsc/bindings/ZigGlobalObject.cpp | 4 - src/jsc/bindings/ZigGlobalObject.h | 4 +- src/jsc/bindings/headers-cpp.h | 3 - src/jsc/bindings/headers.h | 10 - src/jsc/generated.rs | 5 +- src/jsc/generated_classes_list.rs | 1 + src/runtime/api/NativePromiseContext.rs | 25 +- src/runtime/api/ResumableSink.classes.ts | 6 +- src/runtime/api/html_rewriter.rs | 276 +++++++++++---- src/runtime/error.rs | 9 - src/runtime/webcore.rs | 4 +- src/runtime/webcore/Body.rs | 431 +---------------------- src/runtime/webcore/ResumableSink.rs | 8 +- src/runtime/webcore/Sink.rs | 13 - test/js/workerd/html-rewriter.test.js | 296 +++++++++++++++- 16 files changed, 530 insertions(+), 566 deletions(-) diff --git a/src/jsc/bindings/NativePromiseContext.h b/src/jsc/bindings/NativePromiseContext.h index 42d64c73402..b33d38971ba 100644 --- a/src/jsc/bindings/NativePromiseContext.h +++ b/src/jsc/bindings/NativePromiseContext.h @@ -42,7 +42,6 @@ class NativePromiseContext final : public JSC::JSCell { HTTPSServerRequestContext, DebugHTTPServerRequestContext, DebugHTTPSServerRequestContext, - BodyValueBufferer, HTTPSServerH3RequestContext, DebugHTTPSServerH3RequestContext, }; diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a45cf0016df..1e1fe0a1ca1 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4040,10 +4040,6 @@ GlobalObject::PromiseFunctions GlobalObject::promiseHandlerID(Zig::FFIFunction h return GlobalObject::PromiseFunctions::Bun__TestScope__Describe2__bunTestThen; } else if (handler == Bun__TestScope__Describe2__bunTestCatch) { return GlobalObject::PromiseFunctions::Bun__TestScope__Describe2__bunTestCatch; - } else if (handler == Bun__BodyValueBufferer__onResolveStream) { - return GlobalObject::PromiseFunctions::Bun__BodyValueBufferer__onResolveStream; - } else if (handler == Bun__BodyValueBufferer__onRejectStream) { - return GlobalObject::PromiseFunctions::Bun__BodyValueBufferer__onRejectStream; } else if (handler == Bun__onResolveEntryPointResult) { return GlobalObject::PromiseFunctions::Bun__onResolveEntryPointResult; } else if (handler == Bun__onRejectEntryPointResult) { diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index ca759a74da9..ab79b3a1780 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -392,8 +392,6 @@ class GlobalObject : public Bun::GlobalScope { jsFunctionOnLoadObjectResultReject, Bun__TestScope__Describe2__bunTestThen, Bun__TestScope__Describe2__bunTestCatch, - Bun__BodyValueBufferer__onRejectStream, - Bun__BodyValueBufferer__onResolveStream, Bun__onResolveEntryPointResult, Bun__onRejectEntryPointResult, Bun__NodeHTTPRequest__onResolve, @@ -413,7 +411,7 @@ class GlobalObject : public Bun::GlobalScope { Bun__HTTPRequestContextDebugH3__onResolve, Bun__HTTPRequestContextDebugH3__onResolveStream, }; - static constexpr size_t promiseFunctionsSize = 42; + static constexpr size_t promiseFunctionsSize = 40; static PromiseFunctions promiseHandlerID(SYSV_ABI EncodedJSValue (*handler)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1)); diff --git a/src/jsc/bindings/headers-cpp.h b/src/jsc/bindings/headers-cpp.h index 1453b0a0fb1..82ae825c7f6 100644 --- a/src/jsc/bindings/headers-cpp.h +++ b/src/jsc/bindings/headers-cpp.h @@ -182,9 +182,6 @@ extern "C" const size_t Bun__Timer_object_align_ = alignof(Bun__Timer); #include "" #endif -extern "C" const size_t Bun__BodyValueBufferer_object_size_ = sizeof(Bun__BodyValueBufferer); -extern "C" const size_t Bun__BodyValueBufferer_object_align_ = alignof(Bun__BodyValueBufferer); - const size_t sizes[39] = {sizeof(JSC::JSObject), sizeof(WebCore::DOMURL), sizeof(WebCore::DOMFormData), sizeof(WebCore::FetchHeaders), sizeof(SystemError), sizeof(JSC::JSCell), sizeof(JSC::JSString), sizeof(JSC::JSModuleLoader), sizeof(WebCore::AbortSignal), sizeof(JSC::JSPromise), sizeof(JSC::JSPromise), sizeof(JSC::JSFunction), sizeof(JSC::JSGlobalObject), sizeof(JSC::JSMap), sizeof(JSC::JSValue), sizeof(JSC::Exception), sizeof(JSC::VM), sizeof(JSC::ThrowScope), sizeof(JSC::TopExceptionScope), sizeof(FFI__ptr), sizeof(Reader__u8), sizeof(Reader__u16), sizeof(Reader__u32), sizeof(Reader__ptr), sizeof(Reader__i8), sizeof(Reader__i16), sizeof(Reader__i32), sizeof(Reader__f32), sizeof(Reader__f64), sizeof(Reader__i64), sizeof(Reader__u64), sizeof(Reader__intptr), sizeof(Zig::GlobalObject), sizeof(Bun__Path), sizeof(ArrayBufferSink), sizeof(HTTPSResponseSink), sizeof(HTTPResponseSink), sizeof(FileSink), sizeof(FileSink)}; const size_t aligns[39] = {alignof(JSC::JSObject), alignof(WebCore::DOMURL), alignof(WebCore::DOMFormData), alignof(WebCore::FetchHeaders), alignof(SystemError), alignof(JSC::JSCell), alignof(JSC::JSString), alignof(JSC::JSModuleLoader), alignof(WebCore::AbortSignal), alignof(JSC::JSPromise), alignof(JSC::JSPromise), alignof(JSC::JSFunction), alignof(JSC::JSGlobalObject), alignof(JSC::JSMap), alignof(JSC::JSValue), alignof(JSC::Exception), alignof(JSC::VM), alignof(JSC::ThrowScope), alignof(JSC::TopExceptionScope), alignof(FFI__ptr), alignof(Reader__u8), alignof(Reader__u16), alignof(Reader__u32), alignof(Reader__ptr), alignof(Reader__i8), alignof(Reader__i16), alignof(Reader__i32), alignof(Reader__f32), alignof(Reader__f64), alignof(Reader__i64), alignof(Reader__u64), alignof(Reader__intptr), alignof(Zig::GlobalObject), alignof(Bun__Path), alignof(ArrayBufferSink), alignof(HTTPSResponseSink), alignof(HTTPResponseSink), alignof(FileSink), alignof(FileSink)}; diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index ab056ebc60b..d0a11b5ebfe 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -772,16 +772,6 @@ BUN_DECLARE_HOST_FUNCTION(Bun__HTTPRequestContextDebugTLS__onResolveStream); #endif -#pragma mark - Bun__BodyValueBufferer - - -#ifdef __cplusplus - -BUN_DECLARE_HOST_FUNCTION(Bun__BodyValueBufferer__onRejectStream); -BUN_DECLARE_HOST_FUNCTION(Bun__BodyValueBufferer__onResolveStream); - -#endif - #ifdef __cplusplus BUN_DECLARE_HOST_FUNCTION(Bun__TestScope__Describe2__bunTestThen); diff --git a/src/jsc/generated.rs b/src/jsc/generated.rs index 5849fe99546..f648c5e0022 100644 --- a/src/jsc/generated.rs +++ b/src/jsc/generated.rs @@ -1134,8 +1134,9 @@ js_class_module!(JSBlob = "Blob" as crate::webcore_types::Blob { name, js_class_module!(JSResponse = "Response" { body, headers, url, statusText, stream }); js_class_module!(JSRequest = "Request" { body, headers, url, signal, stream }); // `values: ["ondrain", "oncancel", "stream"]` in src/runtime/api/ResumableSink.classes.ts. -js_class_module!(JSResumableFetchSink = "ResumableFetchSink" { ondrain, oncancel, stream }); -js_class_module!(JSResumableS3UploadSink = "ResumableS3UploadSink" { ondrain, oncancel, stream }); +js_class_module!(JSResumableFetchSink = "ResumableFetchSink" { ondrain, oncancel, stream }); +js_class_module!(JSResumableS3UploadSink = "ResumableS3UploadSink" { ondrain, oncancel, stream }); +js_class_module!(JSResumableHTMLRewriterSink = "ResumableHTMLRewriterSink" { ondrain, oncancel, stream }); // `values: ["resolve", "reject"]` in src/runtime/api/Shell.classes.ts. js_class_module!(JSShellInterpreter = "ShellInterpreter" { resolve, reject }); // `src/runtime/crypto/crypto.classes.ts` — one entry per `StaticCryptoHasher` diff --git a/src/jsc/generated_classes_list.rs b/src/jsc/generated_classes_list.rs index 53eccf23d62..ca36dfac014 100644 --- a/src/jsc/generated_classes_list.rs +++ b/src/jsc/generated_classes_list.rs @@ -98,6 +98,7 @@ pub mod Classes { pub use crate::webcore::Request; pub use crate::webcore::Response; pub use crate::webcore::ResumableFetchSink; + pub use crate::webcore::ResumableHTMLRewriterSink; pub use crate::webcore::ResumableS3UploadSink; pub use crate::webcore::S3Client; pub use crate::webcore::S3Stat; diff --git a/src/runtime/api/NativePromiseContext.rs b/src/runtime/api/NativePromiseContext.rs index 86fa7fe727c..2b0dfa36ec2 100644 --- a/src/runtime/api/NativePromiseContext.rs +++ b/src/runtime/api/NativePromiseContext.rs @@ -24,9 +24,7 @@ use bun_event_loop::{Task, TaskTag, Taskable, task_tag}; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{JSGlobalObject, JSValue}; -use crate::api::html_rewriter; use crate::api::server; -use crate::webcore::body; // Request contexts are a single generic // `NewRequestContext`; alias the six @@ -53,13 +51,12 @@ pub enum Tag { HTTPSServerRequestContext, DebugHTTPServerRequestContext, DebugHTTPSServerRequestContext, - BodyValueBufferer, HTTPSServerH3RequestContext, DebugHTTPSServerH3RequestContext, } impl Tag { - pub const COUNT: usize = 7; + pub const COUNT: usize = 6; #[inline] const fn from_raw(n: u8) -> Tag { @@ -68,9 +65,8 @@ impl Tag { 1 => Tag::HTTPSServerRequestContext, 2 => Tag::DebugHTTPServerRequestContext, 3 => Tag::DebugHTTPSServerRequestContext, - 4 => Tag::BodyValueBufferer, - 5 => Tag::HTTPSServerH3RequestContext, - 6 => Tag::DebugHTTPSServerH3RequestContext, + 4 => Tag::HTTPSServerH3RequestContext, + 5 => Tag::DebugHTTPSServerH3RequestContext, _ => unreachable!(), } } @@ -103,9 +99,6 @@ impl NativePromise { const TAG: Tag = npc_tag_for(SSL, DBG, H3); } -impl NativePromiseContextType for body::ValueBufferer<'_> { - const TAG: Tag = Tag::BodyValueBufferer; -} // `&JSGlobalObject` is ABI-identical to a non-null pointer. `ctx` is stored // opaquely (never dereferenced by the C++ side), so the FFI itself has no @@ -214,16 +207,6 @@ impl DeferredDerefTask { Tag::DebugHTTPSServerRequestContext => { (*ctx.cast::()).deref() } - Tag::BodyValueBufferer => { - // ValueBufferer is embedded by value inside HTMLRewriter's - // BufferOutputSink, with the owner pointer stored in .ctx. - // The pending-promise ref was taken on the owner, so we - // release it there. - let bufferer = &*ctx.cast::>(); - html_rewriter::BufferOutputSink::deref( - bufferer.ctx.cast::(), - ); - } Tag::HTTPSServerH3RequestContext => { (*ctx.cast::()).deref() } @@ -246,5 +229,3 @@ const _: () = assert!(core::mem::align_of::() > DeferredDerefTask::TAG_MASK); const _: () = assert!(core::mem::align_of::() > DeferredDerefTask::TAG_MASK); -const _: () = - assert!(core::mem::align_of::>() > DeferredDerefTask::TAG_MASK); diff --git a/src/runtime/api/ResumableSink.classes.ts b/src/runtime/api/ResumableSink.classes.ts index 388343aa4d1..26c1fbe8304 100644 --- a/src/runtime/api/ResumableSink.classes.ts +++ b/src/runtime/api/ResumableSink.classes.ts @@ -32,4 +32,8 @@ function generate(name) { values: ["ondrain", "oncancel", "stream"], }); } -export default [generate("ResumableFetchSink"), generate("ResumableS3UploadSink")]; +export default [ + generate("ResumableFetchSink"), + generate("ResumableS3UploadSink"), + generate("ResumableHTMLRewriterSink"), +]; diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 2efac29cb9a..d9a3258a45e 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -16,8 +16,12 @@ use bun_jsc::{ // owner of the `on_quiet_unhandled_rejection_handler_capture_value` assoc fn. use bun_jsc::virtual_machine::VirtualMachine; +use crate::webcore::resumable_sink::{ + ResumableHTMLRewriterSink, ResumableSink, ResumableSinkBackpressure, ResumableSinkContext, +}; +use crate::webcore::blob::BlobExt as _; use crate::webcore::response::HeadersRef; -use crate::webcore::{self, Response}; +use crate::webcore::{self, ReadableStream, Response}; use bun_core::String as BunString; // `ZigString` re-exports `bun_core::ZigString`; JSC-side methods // (`to_js`, `with_encoding`, …) come from the `ZigStringJsc` extension trait. @@ -551,13 +555,36 @@ pub struct BufferOutputSink { pub context: Rc>, pub response: *mut Response, // BORROW_FIELD: kept alive by response_value Strong pub response_value: StrongOptional, - pub body_value_bufferer: Option>, + pub input_buffer: MutableString, + pub input_sink: Option>, + // The `heap::into_raw` root pointer (set in `init()` once the heap address + // is known). `ResumableSinkContext` enters via `&mut self`, but + // `run_output_sink` re-enters `SinkRef::handle_chunk` via the root pointer; + // under Stacked Borrows that pops the `&mut self` tag, so the trait impls + // recover root provenance from here before driving the rewriter. + this: *mut BufferOutputSink, // Points at the `sink_error` stack local in `init()`; // only written while `init()` is on the stack. // See `write_tmp_sync_error` for the full liveness/provenance argument. pub tmp_sync_error: Option>, } +impl ResumableSinkContext for BufferOutputSink { + fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { + let _ = self.input_buffer.append(bytes); + ResumableSinkBackpressure::WantMore + } + + fn write_end_request(&mut self, err: Option) { + // Recover root provenance (see the `this` field doc) and release the + // `&mut self` borrow before re-entering the rewriter. + let sink = self.this; + // SAFETY: `sink` is the live `heap::into_raw` allocation; the +1 taken + // for the in-flight reader in `init()` is consumed by `on_input_end`. + unsafe { Self::on_input_end(sink, err) }; + } +} + impl BufferOutputSink { // `ref_()`/`deref()` provided by `#[derive(CellRefCounted)]`. @@ -592,9 +619,13 @@ impl BufferOutputSink { context, response: core::ptr::null_mut(), response_value: StrongOptional::empty(), - body_value_bufferer: None, + input_buffer: MutableString::init_empty(), + input_sink: None, + this: core::ptr::null_mut(), tmp_sync_error: None, })); + // SAFETY: `sink` is the `heap::into_raw` allocation above. + unsafe { (*sink).this = sink }; // SAFETY: `sink` is the `heap::into_raw` allocation above; refcount >= 1. let _sink_guard = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; // Note: do not hold a long-lived `&mut *sink` here — the same @@ -721,50 +752,27 @@ impl BufferOutputSink { // SAFETY: original is a live *Response kept alive by caller; sink live. let owned_readable_stream = unsafe { (*original).get_body_readable_stream(&(*sink).global) }; - // SAFETY: sink is a live heap allocation (refcount >= 1). - unsafe { - (*sink).ref_(); - (*sink).body_value_bufferer = Some(webcore::body::ValueBufferer::init( - sink.cast::(), - // Note: `ValueBuffererCallback` takes `*mut c_void` for ctx; - // `on_finished_buffering` takes `*mut BufferOutputSink`. The - // wrapper trampoline restores the concrete type. - Self::on_finished_buffering_trampoline, - &(*sink).global, - )); - } response_js_value.ensure_still_alive(); - // SAFETY: sink is a live heap allocation; body_value_bufferer was just - // set to Some above. `run()` may synchronously invoke - // `on_finished_buffering`, which (via the rewriter's output sink) - // re-enters `SinkRef::handle_chunk` and forms a fresh - // `&mut *sink`. Hoist the bufferer through a raw pointer so no `&mut` - // derived from `*sink` is live across that callback. - let buffering_result: crate::Result<()> = unsafe { - let bufferer: *mut webcore::body::ValueBufferer = - (*sink).body_value_bufferer.as_mut().unwrap(); - (*bufferer).run(value, owned_readable_stream) - }; - if let Err(buffering_error) = buffering_result { + // SAFETY: sink is a live heap allocation (refcount >= 1). + unsafe { (*sink).ref_() }; + // SAFETY: sink is a live heap allocation; `start_reading_input` may + // synchronously invoke `on_input_end`, which (via the rewriter's output + // sink) re-enters `SinkRef::handle_chunk` through the same root `sink` + // pointer. No `&mut *sink` is formed here. + if let Err(err) = unsafe { Self::start_reading_input(sink, value, owned_readable_stream) } { // SAFETY: `sink` is a live `heap::into_raw` allocation; release the - // ref taken for the in-flight bufferer. + // ref taken for the in-flight reader. unsafe { BufferOutputSink::deref(sink) }; - return Ok(match buffering_error { - crate::Error::StreamAlreadyUsed => { - let err = system_error( - "ERR_STREAM_ALREADY_FINISHED", - "Stream already used, please create a new one", - ); - err.to_error_instance(global) - } - _ => { - let err = system_error("ERR_STREAM_CANNOT_PIPE", "Failed to pipe stream"); - err.to_error_instance(global) - } - }); + return Ok((*err).to_error_instance(global)); } + // SAFETY: sink is a live heap allocation (refcount >= 1). Nulling + // `tmp_sync_error` both invalidates the stack-local pointer before it + // goes out of scope and signals to `on_input_end` that any later call + // arrived from the event loop. + unsafe { (*sink).tmp_sync_error = None }; + // sync error occurs — read via the Cell (shares SharedReadWrite // provenance with the raw-pointer writers; see Note above). let captured = sink_error.get(); @@ -781,28 +789,146 @@ impl BufferOutputSink { Ok(response_js_value) } - fn on_finished_buffering_trampoline( - ctx: *mut core::ffi::c_void, - bytes: &[u8], - js_err: Option, - is_async: bool, - ) { - // SAFETY: `ctx` is the `sink` heap allocation registered with the - // bufferer in `init()`; it was `ref_()`'d there so refcount > 0. - unsafe { - Self::on_finished_buffering(ctx.cast::(), bytes, js_err, is_async) + /// Dispatch the input `Response` body into the rewriter. + /// + /// Materialised bodies (string / blob / buffer / empty) run the rewrite + /// immediately; a file-backed blob schedules an async read; anything + /// carrying a `ReadableStream` hands it to `ResumableSink`, which drives + /// every stream source kind (`Bytes` via native pipe, `JavaScript` / + /// `Direct` / `Blob` / `File` via the JS pump) into + /// `write_request_data` / `write_end_request`. + /// + /// # Safety + /// `sink` must be a live `BufferOutputSink` heap allocation with + /// refcount > 0; `(*sink).rewriter` and `(*sink).response` must be set. + /// On `Ok`, the +1 taken for the in-flight reader in `init()` is (or will + /// be) consumed by `on_input_end`; on `Err` the caller releases it. + unsafe fn start_reading_input( + sink: *mut Self, + value: &mut webcore::body::Value, + owned_readable_stream: Option, + ) -> Result<(), Box> { + // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). + let global = unsafe { (*sink).global }; + + value.to_blob_if_possible(); + + let readable_stream: ReadableStream = match value { + webcore::body::Value::Used => { + return Err(Box::new(system_error( + "ERR_STREAM_ALREADY_FINISHED", + "Stream already used, please create a new one", + ))); + } + webcore::body::Value::Empty | webcore::body::Value::Null => { + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, None) }; + return Ok(()); + } + webcore::body::Value::Error(err) => { + let js_err = err.to_js(&global); + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, Some(js_err)) }; + return Ok(()); + } + webcore::body::Value::WTFStringImpl(_) + | webcore::body::Value::InternalBlob(_) + | webcore::body::Value::Blob(_) => { + let mut input = value.use_as_any_blob_allow_non_utf8_string(); + if input.needs_to_read_file() { + if let webcore::AnyBlob::Blob(blob) = &mut input { + struct LoadFileAdapter; + impl webcore::blob::InternalReadFileFn for LoadFileAdapter { + fn call( + sink: *mut BufferOutputSink, + bytes: webcore::blob::read_file::ReadFileResultType, + ) { + // SAFETY: `sink` was set from the live heap allocation + // below and outlives the read (refcount > 0). + unsafe { BufferOutputSink::on_file_read(sink, bytes) }; + } + } + blob.do_read_file_internal::(sink, &global); + return Ok(()); + } + } + // SAFETY: see fn safety contract. + let _ = unsafe { (*sink).input_buffer.append(input.slice()) }; + input.detach(); + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, None) }; + return Ok(()); + } + webcore::body::Value::Locked(locked) => 'brk: { + if let Some(stream) = owned_readable_stream { + break 'brk stream; + } + if let Some(stream) = locked.readable.get(&global) { + break 'brk stream; + } + let js_stream = match value.to_readable_stream(&global) { + Ok(v) => v, + Err(err) => { + let js_err = global.take_exception(err); + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, Some(js_err)) }; + return Ok(()); + } + }; + match ReadableStream::from_js(js_stream, &global) { + Ok(Some(stream)) => break 'brk stream, + _ => { + return Err(Box::new(system_error( + "ERR_STREAM_CANNOT_PIPE", + "Failed to pipe stream", + ))); + } + } + } + }; + + *value = webcore::body::Value::Used; + // Caller already took +1 for the in-flight reader; `init_exact_refs(2)` + // adds the sink's own +1 on top, released by `clear_input_sink`. + let input_sink = ResumableSink::init_exact_refs(&global, readable_stream, sink, 2); + // SAFETY: sink is a live heap allocation (refcount > 0). + unsafe { (*sink).input_sink = NonNull::new(input_sink) }; + Ok(()) + } + + /// # Safety + /// `sink` must be a live `BufferOutputSink` heap allocation with + /// refcount > 0 (the +1 taken in `init()` is consumed by the forwarded + /// `on_input_end`). + unsafe fn on_file_read(sink: *mut Self, bytes: webcore::blob::read_file::ReadFileResultType) { + // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). + let global = unsafe { (*sink).global }; + match bytes { + webcore::blob::read_file::ReadFileResultType::Err(err) => { + let js_err = err.to_error_instance(&global); + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, Some(js_err)) }; + } + webcore::blob::read_file::ReadFileResultType::Result(data) => { + // SAFETY: every producer sets `buf = heap::alloc(v.into_boxed_slice())` + // (read_file.rs); reclaim ownership here. Dropped at end of scope. + let buf = unsafe { Box::<[u8]>::from_raw(data.buf) }; + // SAFETY: sink is a live heap allocation (refcount > 0). + let _ = unsafe { (*sink).input_buffer.append(&buf) }; + // SAFETY: see fn safety contract. + unsafe { Self::on_input_end(sink, None) }; + } } } + /// Deliver the buffered input (or an upstream error) to the rewriter. + /// Called at most once per transform; consumes the +1 taken in `init()` + /// for the in-flight reader. + /// /// # Safety /// `sink` must be a live `BufferOutputSink` heap allocation with /// refcount > 0 (the +1 taken in `init()` is consumed here). - unsafe fn on_finished_buffering( - sink: *mut BufferOutputSink, - bytes: &[u8], - js_err: Option, - is_async: bool, - ) { + unsafe fn on_input_end(sink: *mut BufferOutputSink, js_err: Option) { // SAFETY: `sink` was ref'd in `init()` before scheduling this callback; // refcount > 0 so the allocation is live. `adopt` consumes that +1 on Drop. let _g = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; @@ -816,8 +942,11 @@ impl BufferOutputSink { // SAFETY: sink was ref'd in init() before scheduling this callback; // refcount > 0 so the allocation is live. let global = unsafe { (*sink).global }; + // SAFETY: sink is a live heap allocation (refcount > 0). + let is_async = unsafe { (*sink).tmp_sync_error.is_none() }; - if let Some(mut err) = js_err { + if let Some(err) = js_err { + err.ensure_still_alive(); // SAFETY: (*sink).response is the heap Response allocated in init() // and kept alive by (*sink).response_value (Strong root). let sink_body_value = unsafe { (*(*sink).response).get_body_value() }; @@ -845,13 +974,13 @@ impl BufferOutputSink { } } if is_async { - let _ = sink_body_value.to_error_instance(err.dupe(&global), &global); + let ref_ = jsc::strong::Optional::create(err, &global); + let _ = sink_body_value + .to_error_instance(webcore::body::ValueError::JSValue(ref_), &global); // TODO: properly propagate exception upwards } else { - let ret_err = err.to_js(&global); - ret_err.ensure_still_alive(); - ret_err.protect(); - Self::write_tmp_sync_error(sink, ret_err); + err.protect(); + Self::write_tmp_sync_error(sink, err); } // Do not `end()` the rewriter: that would run `done()`, replacing // the error just stored on the body with the truncated output. @@ -859,14 +988,30 @@ impl BufferOutputSink { return; } + // SAFETY: sink is a live heap allocation (refcount > 0). + let bytes = + core::mem::replace(unsafe { &mut (*sink).input_buffer }, MutableString::init_empty()); // SAFETY: `sink` is live (refcount > 0, see fn safety contract). - if let Some(ret_err) = unsafe { Self::run_output_sink(sink, bytes, is_async) } { + if let Some(ret_err) = unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) } + { ret_err.ensure_still_alive(); ret_err.protect(); Self::write_tmp_sync_error(sink, ret_err); } } + fn clear_input_sink(&mut self) { + if let Some(input_sink) = self.input_sink.take() { + // SAFETY: `input_sink` came from `ResumableSink::init_exact_refs` + // with refcount 2; this releases our +1. `detach_js` first so a + // still-rooted JS wrapper becomes collectible (runs no JS). + unsafe { + (*input_sink.as_ptr()).detach_js(); + ResumableHTMLRewriterSink::deref_(input_sink.as_ptr()); + } + } + } + /// Note: takes `*mut Self` (not `&mut self`) because /// `HtmlRewriter::write/end` re-enter /// `SinkRef::handle_chunk(&mut self)` through the @@ -967,7 +1112,8 @@ impl lol_html::OutputSink for SinkRef { impl Drop for BufferOutputSink { fn drop(&mut self) { - // bytes, body_value_bufferer, context (Rc), response_value (Strong) drop automatically. + // bytes, input_buffer, context (Rc), response_value (Strong) drop automatically. + self.clear_input_sink(); if !self.rewriter.is_null() { // SAFETY: rewriter heap-allocated by init() and not yet freed // (`run_output_sink` nulls the field before consuming it in `end`). diff --git a/src/runtime/error.rs b/src/runtime/error.rs index 19466ef76a5..32b4afc5b32 100644 --- a/src/runtime/error.rs +++ b/src/runtime/error.rs @@ -26,12 +26,6 @@ pub enum Error { SyntaxError, #[error("FmtError")] FmtError, - #[error("StreamAlreadyUsed")] - StreamAlreadyUsed, - #[error("InvalidStream")] - InvalidStream, - #[error("UnsupportedStreamType")] - UnsupportedStreamType, #[error("JSError")] JSError, #[error("ERR_TLS_CERT_ALTNAME_INVALID")] @@ -600,9 +594,6 @@ impl Error { Self::SnapshotInConcurrentGroup => "SnapshotInConcurrentGroup", Self::SyntaxError => "SyntaxError", Self::FmtError => "FmtError", - Self::StreamAlreadyUsed => "StreamAlreadyUsed", - Self::InvalidStream => "InvalidStream", - Self::UnsupportedStreamType => "UnsupportedStreamType", Self::JSError => "JSError", Self::ERR_TLS_CERT_ALTNAME_INVALID => "ERR_TLS_CERT_ALTNAME_INVALID", Self::RequestBodyNotReusable => "RequestBodyNotReusable", diff --git a/src/runtime/webcore.rs b/src/runtime/webcore.rs index 63a73e34a2c..208f6b6050c 100644 --- a/src/runtime/webcore.rs +++ b/src/runtime/webcore.rs @@ -38,7 +38,9 @@ pub use s3_stat::S3Stat; // `JSGlobalObject` as a raw pointer (the FFI boundary cannot carry a Rust // lifetime), so the type aliases are lifetime-free and re-exported directly. pub use cookie_map::{CookieMap, CookieMapRef}; -pub use resumable_sink::{ResumableFetchSink, ResumableS3UploadSink, ResumableSinkBackpressure}; +pub use resumable_sink::{ + ResumableFetchSink, ResumableHTMLRewriterSink, ResumableS3UploadSink, ResumableSinkBackpressure, +}; pub use s3_client::S3Client; pub use streams::{ H3ResponseSink, HTTPResponseSink, HTTPSResponseSink, HTTPServerWritable, NetworkSink, diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index d2402bc94dd..fb5957f1916 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -1,6 +1,5 @@ //! https://developer.mozilla.org/en-US/docs/Web/API/Body -use bun_collections::VecExt; use core::ffi::c_void; use core::ptr::NonNull; @@ -18,8 +17,7 @@ use bun_http_types::MimeType::MimeType; use crate::jsc::HTTPHeaderName; pub use crate::webcore::InternalBlob; use crate::webcore::form_data::AsyncFormDataExt as _; -use crate::webcore::sink::{self, ArrayBufferSink}; -use bun_core::{MutableString, String as BunString, ZigString}; +use bun_core::{String as BunString, ZigString}; use bun_core::{WTFStringImpl, WTFStringImplExt as _, WTFStringImplStruct}; use bun_jsc::ZigStringJsc as _; use bun_jsc::{JsCell, StringJsc as _}; @@ -88,7 +86,6 @@ fn as_url_search_params(value: JSValue) -> Option<*mut URLSearchParams> { bun_core::declare_scope!(BodyValue, visible); bun_core::declare_scope!(BodyMixin, visible); -bun_core::declare_scope!(BodyValueBufferer, visible); type JsTerminated = jsc::JsResult; @@ -1560,12 +1557,9 @@ impl Value { } // ──────────────────────────────────────────────────────────────────────────── -// JSC-integration: extract / BodyMixin (host-fn methods) / ValueBufferer. +// JSC-integration: extract / BodyMixin (host-fn methods). // ──────────────────────────────────────────────────────────────────────────── -// `sink::JSSink` is a free generic (inherent associated types are unstable). -type ArrayBufferJSSink = sink::JSSink; - // https://github.com/WebKit/webkit/blob/main/Source/WebCore/Modules/fetch/FetchBody.cpp#L45 pub(crate) fn extract(global_this: &JSGlobalObject, value: JSValue) -> JsResult { let body_value = Value::from_js(global_this, value)?; @@ -2152,424 +2146,3 @@ fn handle_body_error(value: &mut Value, global_object: &JSGlobalObject) -> Optio Some(JSPromise::rejected_promise(global_object, err.to_js(global_object)).to_js()) } -// ──────────────────────────────────────────────────────────────────────────── -// ValueBufferer -// ──────────────────────────────────────────────────────────────────────────── - -pub(crate) type ValueBuffererCallback = - fn(ctx: *mut c_void, bytes: &[u8], err: Option, is_async: bool); - -pub struct ValueBufferer<'a> { - pub ctx: *mut c_void, - pub on_finished_buffering: ValueBuffererCallback, - - pub js_sink: Option>, - pub byte_stream: Option>, - // readable stream strong ref to keep byte stream alive - pub readable_stream_ref: webcore::readable_stream::Strong, - pub stream_buffer: MutableString, - // allocator dropped — global mimalloc - pub global: &'a JSGlobalObject, -} - -impl<'a> Drop for ValueBufferer<'a> { - fn drop(&mut self) { - // stream_buffer dropped automatically - if let Some(byte_stream) = self.byte_stream { - // Kept alive by `readable_stream_ref` while set — satisfies the - // `BackRef` outlives-holder invariant. R-2: `unpipe_without_deref` - // takes `&self` (interior-mutable). - bun_ptr::BackRef::from(byte_stream).unpipe_without_deref(); - } - self.readable_stream_ref.deinit(); - - if let Some(mut buffer_stream) = self.js_sink.take() { - buffer_stream.detach_self(self.global); - // The wrapper is a `Box>`; dropping it - // frees the box and runs `Vec`'s Drop. - drop(buffer_stream); - } - } -} - -impl<'a> ValueBufferer<'a> { - pub(crate) fn init( - ctx: *mut c_void, - on_finish: ValueBuffererCallback, - global: &'a JSGlobalObject, - ) -> Self { - Self { - ctx, - on_finished_buffering: on_finish, - js_sink: None, - byte_stream: None, - readable_stream_ref: Default::default(), - global, - stream_buffer: MutableString::default(), - } - } - - pub(crate) fn run( - &mut self, - value: &mut Value, - owned_readable_stream: Option, - ) -> crate::Result<()> { - value.to_blob_if_possible(); - - match value { - Value::Used => { - bun_core::scoped_log!(BodyValueBufferer, "Used"); - return Err(crate::Error::StreamAlreadyUsed); - } - Value::Empty | Value::Null => { - bun_core::scoped_log!(BodyValueBufferer, "Empty"); - (self.on_finished_buffering)(self.ctx, b"", None, false); - return Ok(()); - } - Value::Error(err) => { - bun_core::scoped_log!(BodyValueBufferer, "Error"); - // The payload (BunString / Strong) owns refs and has Drop, so a `ptr::read` - // bitwise copy would manufacture a second owner → double-deref when both - // sides drop. Produce a properly ref-bumped duplicate instead. - let err_copy = err.dupe(self.global); - (self.on_finished_buffering)(self.ctx, b"", Some(err_copy), false); - return Ok(()); - } - // Value::InlineBlob(_) | - Value::WTFStringImpl(_) | Value::InternalBlob(_) | Value::Blob(_) => { - // toBlobIfPossible checks for WTFString needing a conversion. - let mut input = value.use_as_any_blob_allow_non_utf8_string(); - let is_pending = input.needs_to_read_file(); - - if is_pending { - if let AnyBlob::Blob(blob) = &mut input { - // The ZST `InternalReadFileFn` impl lets `do_read_file_internal` - // monomorphize a `fn(*mut c_void, ReadFileResultType)` thunk. - struct LoadFileAdapter; - impl<'b> blob::InternalReadFileFn> for LoadFileAdapter { - fn call( - sink: *mut ValueBufferer<'b>, - bytes: blob::read_file::ReadFileResultType, - ) { - // SAFETY: `sink` was set from `self as *mut Self` below and - // outlives the read (ValueBufferer is heap-pinned by caller). - unsafe { &mut *sink }.on_finished_loading_file(bytes); - } - } - let global = self.global; - blob.do_read_file_internal::( - std::ptr::from_mut::(self), - global, - ); - } - } else { - let bytes = input.slice(); - bun_core::scoped_log!(BodyValueBufferer, "Blob {}", bytes.len()); - (self.on_finished_buffering)(self.ctx, bytes, None, false); - input.detach(); - } - return Ok(()); - } - Value::Locked(_) => { - self.buffer_locked_body_value(value, owned_readable_stream)?; - } - } - Ok(()) - } - - fn on_finished_loading_file(&mut self, bytes: blob::read_file::ReadFileResultType) { - match bytes { - blob::read_file::ReadFileResultType::Err(err) => { - bun_core::scoped_log!(BodyValueBufferer, "onFinishedLoadingFile Error"); - (self.on_finished_buffering)( - self.ctx, - b"", - Some(ValueError::SystemError(err)), - true, - ); - } - blob::read_file::ReadFileResultType::Result(data) => { - // SAFETY: every producer sets `buf = heap::alloc(v.into_boxed_slice())` - // (read_file.rs); reclaim ownership here. Dropped at end of scope. - let buf = unsafe { Box::<[u8]>::from_raw(data.buf) }; - bun_core::scoped_log!( - BodyValueBufferer, - "onFinishedLoadingFile Data {}", - buf.len() - ); - (self.on_finished_buffering)(self.ctx, &buf, None, true); - } - } - } - - fn on_stream_pipe(&mut self, stream: &streams::Result) { - if let streams::Result::Err(err) = stream { - bun_core::scoped_log!(BodyValueBufferer, "onStreamPipe error"); - let js_err = err.to_js(self.global); - let ref_ = jsc::strong::Optional::create(js_err, self.global); - (self.on_finished_buffering)(self.ctx, b"", Some(ValueError::JSValue(ref_)), true); - return; - } - let chunk = stream.slice(); - bun_core::scoped_log!(BodyValueBufferer, "onStreamPipe chunk {}", chunk.len()); - let _ = self.stream_buffer.write(chunk); - if stream.is_done() { - let bytes = self.stream_buffer.list.as_slice(); - bun_core::scoped_log!(BodyValueBufferer, "onStreamPipe done {}", bytes.len()); - (self.on_finished_buffering)(self.ctx, bytes, None, true); - } - } - - /// Reclaim the `*mut Self` smuggled through a `NativePromiseContext` cell - /// as an exclusive borrow. Centralises the `Option>` deref - /// for the two host-fn entry points below (one accessor, N safe callers). - /// - /// # Safety (encapsulated) - /// `NativePromiseContext::take` returns the live ctx pointer set in - /// `create()` (caller stashed `&mut Self` and held a +1 ref); the cell is - /// nulled on take so this is the sole owner. `ValueBufferer` is heap- - /// pinned by its caller for the stream's duration. - #[inline] - fn take_ctx<'r>(cell: JSValue) -> Option<&'r mut Self> { - // SAFETY: see fn doc — +1 ref transferred back; sole live `&mut`. - crate::api::NativePromiseContext::take::(cell).map(|mut p| unsafe { p.as_mut() }) - } - - pub(crate) fn on_resolve_stream( - _global: &JSGlobalObject, - callframe: &CallFrame, - ) -> JsResult { - let args = callframe.arguments(); - let Some(sink) = Self::take_ctx(args[args.len() - 1]) else { - return Ok(JSValue::UNDEFINED); - }; - sink.handle_resolve_stream(true); - Ok(JSValue::UNDEFINED) - } - - pub(crate) fn on_reject_stream( - _global: &JSGlobalObject, - callframe: &CallFrame, - ) -> JsResult { - let args = callframe.arguments(); - let Some(sink) = Self::take_ctx(args[args.len() - 1]) else { - return Ok(JSValue::UNDEFINED); - }; - let err = args[0]; - sink.handle_reject_stream(err, true); - Ok(JSValue::UNDEFINED) - } - - fn handle_reject_stream(&mut self, err: JSValue, is_async: bool) { - if let Some(mut wrapper) = self.js_sink.take() { - wrapper.detach_self(self.global); - // see `Drop` impl — dropping the Box frees the wrapper - // and runs `Vec`'s Drop. - drop(wrapper); - } - // `jsc::strong::Optional` owns a GC root; `ptr::read`-duplicating it would - // double-deinit. Transfer the single owner directly to the callback; the callback - // (or its returned `ValueError`'s Drop) is responsible for releasing it. - let ref_ = jsc::strong::Optional::create(err, self.global); - (self.on_finished_buffering)(self.ctx, b"", Some(ValueError::JSValue(ref_)), is_async); - } - - fn handle_resolve_stream(&mut self, is_async: bool) { - if let Some(wrapper) = &self.js_sink { - let bytes = wrapper.sink.bytes.slice(); - bun_core::scoped_log!(BodyValueBufferer, "handleResolveStream {}", bytes.len()); - (self.on_finished_buffering)(self.ctx, bytes, None, is_async); - } else { - bun_core::scoped_log!(BodyValueBufferer, "handleResolveStream no sink"); - (self.on_finished_buffering)(self.ctx, b"", None, is_async); - } - } - - fn buffer_locked_body_value( - &mut self, - value: &mut Value, - owned_readable_stream: Option, - ) -> crate::Result<()> { - debug_assert!(matches!(value, Value::Locked(_))); - let Value::Locked(locked) = value else { - unreachable!() - }; - let readable_stream = 'brk: { - if let Some(stream) = locked.readable.get(self.global) { - // keep the stream alive until we're done with it. - // Transfer ownership: `*value = .Used` below would otherwise - // drop `locked.readable` anyway, so moving the existing GC - // root preserves the refcount balance. - self.readable_stream_ref = core::mem::take(&mut locked.readable); - break 'brk Some(stream); - } - if let Some(stream) = owned_readable_stream { - // response owns the stream, so we hold a strong reference to it - self.readable_stream_ref = - webcore::readable_stream::Strong::init(stream, self.global); - break 'brk Some(stream); - } - None - }; - if let Some(stream) = readable_stream { - *value = Value::Used; - - if stream.is_locked(self.global) { - return Err(crate::Error::StreamAlreadyUsed); - } - - match stream.ptr { - webcore::readable_stream::Source::Invalid => { - return Err(crate::Error::InvalidStream); - } - // toBlobIfPossible should've caught this - webcore::readable_stream::Source::Blob(_) - | webcore::readable_stream::Source::File(_) => unreachable!(), - webcore::readable_stream::Source::JavaScript - | webcore::readable_stream::Source::Direct => { - // this is broken right now - // return self.create_js_sink(stream); - return Err(crate::Error::UnsupportedStreamType); - } - webcore::readable_stream::Source::Bytes(byte_stream_ptr) => { - // BACKREF: see `Source::bytes()` — payload owned by the - // readable stream, kept alive via `self.readable_stream_ref` - // above. R-2: all touched fields are interior-mutable. - let byte_stream = stream.ptr.bytes().expect("matched Bytes"); - debug_assert!(byte_stream.pipe.get().ctx.is_none()); - debug_assert!(self.byte_stream.is_none()); - - let bytes = byte_stream.buffer.get().as_slice(); - // If we've received the complete body by the time this function is called - // we can avoid streaming it and just send it all at once. - if byte_stream.has_received_last_chunk.get() { - if let streams::Result::Err(err) = &byte_stream.pending.get().result { - bun_core::scoped_log!( - BodyValueBufferer, - "byte stream has_received_last_chunk error" - ); - let js_err = err.to_js(self.global); - let ref_ = jsc::strong::Optional::create(js_err, self.global); - (self.on_finished_buffering)( - self.ctx, - b"", - Some(ValueError::JSValue(ref_)), - false, - ); - stream.done(self.global); - return Ok(()); - } - bun_core::scoped_log!( - BodyValueBufferer, - "byte stream has_received_last_chunk {}", - bytes.len() - ); - (self.on_finished_buffering)(self.ctx, bytes, None, false); - // is safe to detach here because we're not going to receive any more data - stream.done(self.global); - return Ok(()); - } - - byte_stream - .pipe - .set(crate::webcore::Wrap::::init(self)); - self.byte_stream = NonNull::new(byte_stream_ptr); - bun_core::scoped_log!( - BodyValueBufferer, - "byte stream pre-buffered {}", - bytes.len() - ); - - let _ = self.stream_buffer.write(bytes); - return Ok(()); - } - } - } - - // reshaped for borrowck — re-borrow locked after possible *value = Used above. - let Value::Locked(locked) = value else { - unreachable!() - }; - - if locked.on_receive_value.is_some() || locked.task.is_some() { - // ValueBufferer wants the whole body; tell the producer to never - // pause for JS backpressure before the stream is materialised. - if let (Some(on_start_buffering), Some(task)) = - (locked.on_start_buffering.take(), locked.task) - { - on_start_buffering(task); - } - // someone else is waiting for the stream or waiting for `onStartStreaming` - let readable = value - .to_readable_stream(self.global) - .map_err(|_| crate::Error::JSError)?; - // The JS exception value is - // flattened to a string-coded error because `run`'s callers consume - // `crate::Error` (the exception itself stays pending on the VM). - readable.ensure_still_alive(); - readable.protect(); - return self.buffer_locked_body_value(value, None); - } - // is safe to wait it buffer - locked.task = Some(std::ptr::from_mut::(self).cast::()); - locked.on_receive_value = Some(Self::on_receive_value); - Ok(()) - } - - fn on_receive_value(ctx: *mut c_void, value: &mut Value) { - // SAFETY: ctx was set from `self as *mut Self` in buffer_locked_body_value. - let sink = unsafe { bun_ptr::callback_ctx::(ctx) }; - match value { - Value::Error(err) => { - bun_core::scoped_log!(BodyValueBufferer, "onReceiveValue Error"); - // See run(): produce a ref-bumped duplicate instead of `ptr::read`ing a - // non-Copy owned value (would double-deref on drop). - let err_copy = err.dupe(sink.global); - (sink.on_finished_buffering)(sink.ctx, b"", Some(err_copy), true); - } - _ => { - value.to_blob_if_possible(); - let input = value.use_as_any_blob_allow_non_utf8_string(); - let bytes = input.slice(); - bun_core::scoped_log!(BodyValueBufferer, "onReceiveValue {}", bytes.len()); - (sink.on_finished_buffering)(sink.ctx, bytes, None, true); - } - } - } -} - -// `webcore::Wrap` requires `T: PipeHandler`. -impl<'a> crate::webcore::PipeHandler for ValueBufferer<'a> { - fn on_pipe(&mut self, stream: streams::Result) { - self.on_stream_pipe(&stream) - } -} - -// `#[bun_jsc::host_fn]` on on_resolve_stream/on_reject_stream emits the JSC ABI shim; -// these no_mangle re-exports point at those shims under the C names the C++ side expects. -bun_jsc::jsc_host_abi! { - #[unsafe(no_mangle)] - pub(crate) unsafe fn Bun__BodyValueBufferer__onResolveStream( - global: *mut JSGlobalObject, - callframe: *mut CallFrame, - ) -> JSValue { - // S008: `JSGlobalObject`/`CallFrame` are `opaque_ffi!` ZST handles — - // safe `*mut → &` via `opaque_deref` (JSC guarantees non-null/live). - let (global, callframe) = - (bun_opaque::opaque_deref(global), bun_opaque::opaque_deref(callframe)); - jsc::to_js_host_fn_result(global, ValueBufferer::on_resolve_stream(global, callframe)) - } -} -bun_jsc::jsc_host_abi! { - #[unsafe(no_mangle)] - pub(crate) unsafe fn Bun__BodyValueBufferer__onRejectStream( - global: *mut JSGlobalObject, - callframe: *mut CallFrame, - ) -> JSValue { - // S008: `JSGlobalObject`/`CallFrame` are `opaque_ffi!` ZST handles — - // safe `*mut → &` via `opaque_deref` (JSC guarantees non-null/live). - let (global, callframe) = - (bun_opaque::opaque_deref(global), bun_opaque::opaque_deref(callframe)); - jsc::to_js_host_fn_result(global, ValueBufferer::on_reject_stream(global, callframe)) - } -} diff --git a/src/runtime/webcore/ResumableSink.rs b/src/runtime/webcore/ResumableSink.rs index 7235f7607d2..23c00430644 100644 --- a/src/runtime/webcore/ResumableSink.rs +++ b/src/runtime/webcore/ResumableSink.rs @@ -11,6 +11,7 @@ use bun_core::String as BunString; use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsRef, JsResult, SystemError}; use bun_output::{declare_scope, scoped_log}; +use crate::api::html_rewriter::BufferOutputSink; use crate::node::{ErrorCode, StringOrBuffer}; use crate::webcore::fetch::fetch_tasklet::FetchTasklet; use crate::webcore::s3::client::S3UploadStreamWrapper; @@ -620,7 +621,11 @@ macro_rules! impl_resumable_sink_js { } )*}; } -impl_resumable_sink_js!(JSResumableFetchSink, JSResumableS3UploadSink); +impl_resumable_sink_js!( + JSResumableFetchSink, + JSResumableS3UploadSink, + JSResumableHTMLRewriterSink, +); // Forward to the inherent methods on each Context type; the trait bound is // satisfied by delegating to those inherent impls. @@ -638,6 +643,7 @@ impl ResumableSinkContext for FetchTasklet { pub type ResumableFetchSink = ResumableSink; pub type ResumableS3UploadSink = ResumableSink; +pub type ResumableHTMLRewriterSink = ResumableSink; unsafe extern "C" { safe fn Bun__assignStreamIntoResumableSink( diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index 2d33b1e0b7f..9e9ce866293 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -12,19 +12,6 @@ pub use crate::webcore::array_buffer_sink::ArrayBufferSink; crate::impl_js_sink_abi!(ArrayBufferSink, "ArrayBufferSink"); -impl JSSink { - /// Unprotects the controller cell stashed in `signal.ptr` - /// and tells C++ to drop its back-pointer. Called from - /// `Body::ValueBufferer` Drop / reject paths. - // Renamed from `detach` to avoid colliding with the generic - // `JSSink::detach(signal, global)` associated fn — Rust - // forbids same-name items across impl blocks for the same type even with - // different signatures (E0592). - pub fn detach_self(&mut self, global: &JSGlobalObject) { - JSSink::::detach(&mut self.sink.signal, global); - } -} - // ────────────────────────────────────────────────────────────────────────── // JSSink // diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index 6ffc837062e..b6a2392c738 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -303,6 +303,298 @@ describe("HTMLRewriter", () => { }); }); + describe("transform() accepts a JavaScript-backed ReadableStream body", () => { + // https://github.com/oven-sh/bun/issues/14216 + // https://github.com/oven-sh/bun/issues/11758 + const encode = s => new TextEncoder().encode(s); + + function rewriter() { + return new HTMLRewriter().on("p", { + element(element) { + element.setInnerContent("bye"); + }, + }); + } + + function streamOf(...chunks) { + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(chunk); + controller.close(); + }, + }); + } + + it("single Uint8Array chunk", async () => { + const transformed = rewriter().transform(new Response(streamOf(encode("

hi

")))); + expect(await transformed.text()).toBe("

bye

"); + }); + + it("single string chunk", async () => { + const transformed = rewriter().transform(new Response(streamOf("

hi

"))); + expect(await transformed.text()).toBe("

bye

"); + }); + + it("an element split across chunk boundaries", async () => { + const transformed = rewriter().transform( + new Response(streamOf(encode("

h"), encode("i

two

"))), + ); + expect(await transformed.text()).toBe("

bye

bye

"); + }); + + it("mixed string and binary chunks", async () => { + const transformed = rewriter().transform(new Response(streamOf("

a

", encode("

b

")))); + expect(await transformed.text()).toBe("

bye

bye

"); + }); + + it("empty stream", async () => { + let endCalls = 0; + const transformed = new HTMLRewriter() + .onDocument({ + end() { + endCalls++; + }, + }) + .transform(new Response(streamOf())); + expect(await transformed.text()).toBe(""); + expect(endCalls).toBe(1); + }); + + it("a direct stream", async () => { + const body = new ReadableStream({ + type: "direct", + pull(controller) { + controller.write("

hi

"); + controller.close(); + }, + }); + expect(await rewriter().transform(new Response(body)).text()).toBe("

bye

"); + }); + + it("a stream that only produces chunks after transform() returns", async () => { + // start() stays pending across transform(), so the rewriter has to take + // the asynchronous path instead of buffering everything up front. + const { promise: gate, resolve: openGate } = Promise.withResolvers(); + const body = new ReadableStream({ + async start(controller) { + await gate; + controller.enqueue(encode("

hi

")); + controller.close(); + }, + }); + const text = rewriter().transform(new Response(body)).text(); + openGate(); + expect(await text).toBe("

bye

"); + }); + + it("every promise-returning reader on the transformed response", async () => { + // `.body.getReader()` is covered by the `.todo` below: the ResumableSink + // pump delivers chunks from a microtask, so at the instant `transform()` + // returns the body is still `Locked`, which is the pre-existing #19305 + // output-side bug. Readers that return a promise await a turn first and + // are unaffected. + const read = { + text: response => response.text(), + arrayBuffer: async response => new TextDecoder().decode(await response.arrayBuffer()), + bytes: async response => new TextDecoder().decode(await response.bytes()), + blob: response => response.blob().then(blob => blob.text()), + json: response => response.json().then(value => JSON.stringify(value)), + }; + + const html = '

hi

there

'; + const expected = '

bye

bye

'; + for (const [name, consume] of Object.entries(read)) { + const transformed = rewriter().transform(new Response(streamOf(encode(html)))); + if (name === "json") { + // Not valid JSON, but it must fail as a JSON parse error, which + // still proves the transformed bytes reached the parser. + await expect(consume(transformed)).rejects.toThrow(/JSON/i); + continue; + } + expect({ [name]: await consume(transformed) }).toEqual({ [name]: expected }); + } + }); + + it("element handlers observe the streamed document", async () => { + const tags = []; + const transformed = new HTMLRewriter() + .on("*", { + element(element) { + tags.push(element.tagName); + }, + }) + .transform(new Response(streamOf(encode("

hi

")))); + expect(await transformed.text()).toBe("

hi

"); + expect(tags).toEqual(["div", "p"]); + }); + + it("a stream that errors rejects the transformed body", async () => { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(encode("

hi

")); + controller.error(new Error("upstream boom")); + }, + }); + const transformed = rewriter().transform(new Response(body)); + // Must reject rather than resolve with the truncated document. + await expect(transformed.text()).rejects.toThrow("upstream boom"); + }); + + it("a stream that errors after transform() returns rejects the transformed body", async () => { + const { promise: gate, resolve: openGate } = Promise.withResolvers(); + const body = new ReadableStream({ + async start(controller) { + await gate; + controller.error(new Error("late boom")); + }, + }); + const text = rewriter().transform(new Response(body)).text(); + openGate(); + await expect(text).rejects.toThrow("late boom"); + }); + + it("a chunk that is neither a string nor a view rejects the transformed body", async () => { + const transformed = rewriter().transform(new Response(streamOf(42))); + // The underlying TypeError must surface, not the opaque + // "Failed to pipe stream" that transform() used to throw. + await expect(transformed.text()).rejects.toThrow(TypeError); + }); + + it("reusing the transformed response's source stream throws", async () => { + const response = new Response(streamOf(encode("

hi

"))); + expect(await rewriter().transform(response).text()).toBe("

bye

"); + expect(() => rewriter().transform(response)).toThrow("Response body already used"); + }); + + it("does not rewrite out of the source buffer a handler can detach", async () => { + // ResumableSink copies each chunk into the sink's own input buffer before + // returning to the pump, so a handler that mutates (or transfers, then + // frees) the user's buffer mid-scan must not corrupt bytes lol-html has + // yet to tokenize. + let chunk; + const body = new ReadableStream({ + start(controller) { + chunk = encode("xy"); + controller.enqueue(chunk); + controller.close(); + }, + }); + const transformed = new HTMLRewriter() + .on("a", { + element() { + // overwrite "" (not yet tokenized) with "" + chunk.set(encode("qqq"), 9); + // and drop the backing store the rewriter would be reading + chunk.buffer.transfer(); + Bun.gc(true); + }, + }) + .transform(new Response(body)); + expect(await transformed.text()).toBe("xy"); + }); + + // A live transform is kept alive only by whatever can still settle the + // stream. That holds because settling needs the controller, and the + // controller holds the stream. Each case hides the stream from userland and + // collects hard before letting it finish. + describe("a source the bufferer no longer roots still completes", () => { + const cases = { + "controller held only by a timer": () => + new ReadableStream({ + start(controller) { + setTimeout(() => { + controller.enqueue(encode("

hi

")); + controller.close(); + }, 1); + }, + }), + "controller escaping to an outer scope": () => { + let escaped; + const stream = new ReadableStream({ + start(controller) { + escaped = controller; + }, + }); + queueMicrotask(() => { + escaped.enqueue(encode("

hi

")); + escaped.close(); + }); + return stream; + }, + "controller reachable only from a pending pull": () => + new ReadableStream({ + type: "direct", + async pull(controller) { + await Bun.sleep(1); + controller.write("

hi

"); + controller.close(); + }, + }), + }; + + for (const [name, makeStream] of Object.entries(cases)) { + it(name, async () => { + const transformed = rewriter().transform(new Response(makeStream())); + // Collect aggressively while the source is still in flight. + for (let i = 0; i < 3; i++) { + Bun.gc(true); + await Bun.sleep(1); + } + expect(await transformed.text()).toBe("

bye

"); + }); + } + }); + + // Resolves with "" instead: the `.body` getter builds a ByteStream the + // producer is never told about, so done() closes it empty. Pre-existing and + // not specific to JS sources — a fetch body that is still mid-stream when + // transform() returns does the same thing on main (#19305, and #6068 for + // the Bun.serve shape, which hangs). Un-skip once the output side is fixed. + it.todo(".body of a transform whose source is still pending", async () => { + const { promise: gate, resolve: openGate } = Promise.withResolvers(); + const body = new ReadableStream({ + async start(controller) { + await gate; + controller.enqueue(encode("

hi

")); + controller.close(); + }, + }); + const transformed = rewriter().transform(new Response(body)); + const reader = transformed.body.getReader(); + openGate(); + const parts = []; + for (let chunk = await reader.read(); !chunk.done; chunk = await reader.read()) { + parts.push(new TextDecoder().decode(chunk.value)); + } + expect(parts.join("")).toBe("

bye

"); + }); + + it("served over Bun.serve", async () => { + using server = Bun.serve({ + port: 0, + fetch() { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(encode("hello world")); + controller.close(); + }, + }); + return new HTMLRewriter() + .on("b", { + element(element) { + element.before("

", { html: true }); + element.after("

", { html: true }); + element.removeAndKeepContent(); + }, + }) + .transform(new Response(body, { headers: { "content-type": "text/html" } })); + }, + }); + const response = await fetch(server.url); + expect(await response.text()).toBe("

hello world

"); + }); + }); + it("HTMLRewriter: async replacement using fetch + Bun.serve", async () => { await gcTick(); let content; @@ -946,12 +1238,12 @@ const payloads = [ { name: "direct", data: getStream("direct", "none"), - test: it.todo, + test: it, }, { name: "default", data: getStream("default", "none"), - test: it.todo, + test: it, }, { name: "file", From d1694521b1dfbf737dbdd1214b5b7c2cee7f9b9d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:14:46 +0000 Subject: [PATCH 02/13] [autofix.ci] apply automated fixes --- src/runtime/api/html_rewriter.rs | 13 ++++++++----- src/runtime/webcore/Body.rs | 1 - 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index d9a3258a45e..2a68072e047 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -16,11 +16,11 @@ use bun_jsc::{ // owner of the `on_quiet_unhandled_rejection_handler_capture_value` assoc fn. use bun_jsc::virtual_machine::VirtualMachine; +use crate::webcore::blob::BlobExt as _; +use crate::webcore::response::HeadersRef; use crate::webcore::resumable_sink::{ ResumableHTMLRewriterSink, ResumableSink, ResumableSinkBackpressure, ResumableSinkContext, }; -use crate::webcore::blob::BlobExt as _; -use crate::webcore::response::HeadersRef; use crate::webcore::{self, ReadableStream, Response}; use bun_core::String as BunString; // `ZigString` re-exports `bun_core::ZigString`; JSC-side methods @@ -989,10 +989,13 @@ impl BufferOutputSink { } // SAFETY: sink is a live heap allocation (refcount > 0). - let bytes = - core::mem::replace(unsafe { &mut (*sink).input_buffer }, MutableString::init_empty()); + let bytes = core::mem::replace( + unsafe { &mut (*sink).input_buffer }, + MutableString::init_empty(), + ); // SAFETY: `sink` is live (refcount > 0, see fn safety contract). - if let Some(ret_err) = unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) } + if let Some(ret_err) = + unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) } { ret_err.ensure_still_alive(); ret_err.protect(); diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index fb5957f1916..40c73ae296d 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -2145,4 +2145,3 @@ fn handle_body_error(value: &mut Value, global_object: &JSGlobalObject) -> Optio }; Some(JSPromise::rejected_promise(global_object, err.to_js(global_object)).to_js()) } - From 2d9016f1a1d13a62ea68ba0ca9f19af94a7f71fe Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 23 Jul 2026 23:21:02 +0000 Subject: [PATCH 03/13] clippy: keep SAFETY comment adjacent to unsafe after rustfmt wrap --- src/runtime/api/html_rewriter.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 2a68072e047..1830862c8db 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -994,9 +994,8 @@ impl BufferOutputSink { MutableString::init_empty(), ); // SAFETY: `sink` is live (refcount > 0, see fn safety contract). - if let Some(ret_err) = - unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) } - { + let ret_err = unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) }; + if let Some(ret_err) = ret_err { ret_err.ensure_still_alive(); ret_err.protect(); Self::write_tmp_sync_error(sink, ret_err); From acd4cb9e402a305ccfd848da20c56075c1467f6d Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 24 Jul 2026 00:01:44 +0000 Subject: [PATCH 04/13] html_rewriter: drop the ResumableSink back-pointer BufferOutputSink::Drop ran inside ResumableSink::end_pipe's &mut self on the Source::Bytes path (end_pipe -> on_end -> write_end_request -> on_input_end -> ScopedRef drop -> BufferOutputSink::Drop -> clear_input_sink), and clear_input_sink re-derived a fresh &mut ResumableSink from the stored root pointer, popping end_pipe's Unique tag before end_pipe read self.js_this.is_strong(). No runtime consequence (the allocation was still live and detach_js was a no-op), but it broke the Stacked Borrows discipline the rest of this change is careful about. BufferOutputSink never needed to own a ref on the ResumableSink in the first place: the in-flight +1 on BufferOutputSink keeps the context pointer valid until write_end_request fires, and the ResumableSink's own lifecycle (pipe ref or JS wrapper) governs its allocation. Dropping to init() (ref_count 1) and not storing the pointer removes the reverse edge entirely. --- src/runtime/api/html_rewriter.rs | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 1830862c8db..0b913ad44ee 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -19,7 +19,7 @@ use bun_jsc::virtual_machine::VirtualMachine; use crate::webcore::blob::BlobExt as _; use crate::webcore::response::HeadersRef; use crate::webcore::resumable_sink::{ - ResumableHTMLRewriterSink, ResumableSink, ResumableSinkBackpressure, ResumableSinkContext, + ResumableHTMLRewriterSink, ResumableSinkBackpressure, ResumableSinkContext, }; use crate::webcore::{self, ReadableStream, Response}; use bun_core::String as BunString; @@ -556,7 +556,6 @@ pub struct BufferOutputSink { pub response: *mut Response, // BORROW_FIELD: kept alive by response_value Strong pub response_value: StrongOptional, pub input_buffer: MutableString, - pub input_sink: Option>, // The `heap::into_raw` root pointer (set in `init()` once the heap address // is known). `ResumableSinkContext` enters via `&mut self`, but // `run_output_sink` re-enters `SinkRef::handle_chunk` via the root pointer; @@ -620,7 +619,6 @@ impl BufferOutputSink { response: core::ptr::null_mut(), response_value: StrongOptional::empty(), input_buffer: MutableString::init_empty(), - input_sink: None, this: core::ptr::null_mut(), tmp_sync_error: None, })); @@ -888,11 +886,13 @@ impl BufferOutputSink { }; *value = webcore::body::Value::Used; - // Caller already took +1 for the in-flight reader; `init_exact_refs(2)` - // adds the sink's own +1 on top, released by `clear_input_sink`. - let input_sink = ResumableSink::init_exact_refs(&global, readable_stream, sink, 2); - // SAFETY: sink is a live heap allocation (refcount > 0). - unsafe { (*sink).input_sink = NonNull::new(input_sink) }; + // The caller's in-flight +1 on `BufferOutputSink` keeps `context` valid + // until `write_end_request` fires; the `ResumableSink` allocation + // itself is owned solely by its own lifecycle (pipe ref / JS wrapper). + // Not stored: `on_input_end` runs inside `ResumableSink::end_pipe`'s + // `&mut self`, so a back-pointer this struct could deref from `Drop` + // would retag the sink's root pointer and pop `end_pipe`'s borrow. + let _ = ResumableHTMLRewriterSink::init(&global, readable_stream, sink); Ok(()) } @@ -1002,18 +1002,6 @@ impl BufferOutputSink { } } - fn clear_input_sink(&mut self) { - if let Some(input_sink) = self.input_sink.take() { - // SAFETY: `input_sink` came from `ResumableSink::init_exact_refs` - // with refcount 2; this releases our +1. `detach_js` first so a - // still-rooted JS wrapper becomes collectible (runs no JS). - unsafe { - (*input_sink.as_ptr()).detach_js(); - ResumableHTMLRewriterSink::deref_(input_sink.as_ptr()); - } - } - } - /// Note: takes `*mut Self` (not `&mut self`) because /// `HtmlRewriter::write/end` re-enter /// `SinkRef::handle_chunk(&mut self)` through the @@ -1115,7 +1103,6 @@ impl lol_html::OutputSink for SinkRef { impl Drop for BufferOutputSink { fn drop(&mut self) { // bytes, input_buffer, context (Rc), response_value (Strong) drop automatically. - self.clear_input_sink(); if !self.rewriter.is_null() { // SAFETY: rewriter heap-allocated by init() and not yet freed // (`run_output_sink` nulls the field before consuming it in `end`). From 7f89795eef697f6d7c0e09d414b7ebf56d7b2edf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 08:06:40 +0000 Subject: [PATCH 05/13] HTMLRewriter: write output to a ByteStream and feed the rewriter per chunk Addresses review on #35324: - write_request_data now drives lol_html::HtmlRewriter::write() per chunk instead of buffering the whole body first. A spill buffer covers the one re-entrant path (an async handler's wait_for_promise draining the next Source::Bytes pipe chunk while write() is on the stack); the JS pump cannot re-enter (JSResumableSinkPumpOperation::m_reading guards the drain loop and the next read is not issued until sink.write() returns). - The output Response body is a ByteStream from the start and SinkRef writes to it, not back into BufferOutputSink. That removes the self-reference (the reason run_output_sink took *mut Self and the this: *mut Self field existed) and makes .body.getReader() / Bun.serve work regardless of whether the input has settled (#19305). - ResumableSinkContext now takes *mut Self (borrow = ptr per src/CLAUDE.md), since HTMLRewriter's write_request_data can re-enter the sink. FetchTasklet and S3UploadStreamWrapper impls dereference once. - The in-flight ref on BufferOutputSink is RAII (ScopedRef::new / forget / adopt) instead of manual ref_() / deref(). - start_reading_input collapses to the materialised-body fast path (so transform(String | ArrayBuffer) keeps its synchronous contract) plus to_readable_stream() + ResumableSink::init for everything else; the per-variant file-read adapter is gone. Dropped fields: bytes, response, response_value, input_buffer, this, tmp_sync_error. Dropped methods: write_tmp_sync_error, on_file_read, on_input_end, run_output_sink, done, write. Handler errors are captured per write() (Self::rewrite) so the original JS error surfaces on async chunks too, not just inside init(). --- src/runtime/api/html_rewriter.rs | 689 ++++++++++----------------- src/runtime/webcore/ResumableSink.rs | 40 +- src/runtime/webcore/s3/client.rs | 11 +- 3 files changed, 295 insertions(+), 445 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 0b913ad44ee..63188581243 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -4,10 +4,9 @@ use core::cell::{Cell, RefCell}; use core::ptr::NonNull; use std::rc::Rc; -use bun_core::MutableString; use bun_jsc::{ self as jsc, CallFrame, GlobalRef, JSGlobalObject, JSValue, JsCell, JsResult, ProtectedJSValue, - StrongOptional, SystemError, bun_string_jsc, + bun_string_jsc, }; // Note: `bun_jsc::VirtualMachine` is a *module* re-export // (`pub use self::virtual_machine as VirtualMachine;`). The struct lives at @@ -16,12 +15,12 @@ use bun_jsc::{ // owner of the `on_quiet_unhandled_rejection_handler_capture_value` assoc fn. use bun_jsc::virtual_machine::VirtualMachine; -use crate::webcore::blob::BlobExt as _; +use crate::webcore::ByteStream; use crate::webcore::response::HeadersRef; use crate::webcore::resumable_sink::{ ResumableHTMLRewriterSink, ResumableSinkBackpressure, ResumableSinkContext, }; -use crate::webcore::{self, ReadableStream, Response}; +use crate::webcore::{self, ReadableStream, Response, streams}; use bun_core::String as BunString; // `ZigString` re-exports `bun_core::ZigString`; JSC-side methods // (`to_js`, `with_encoding`, …) come from the `ZigStringJsc` extension trait. @@ -61,15 +60,6 @@ fn cell_get<'a, T>(cell: &Cell<*mut T>) -> Option<&'a mut T> { unsafe { cell.get().as_mut() } } -/// Construct a `SystemError` with code+message and remaining fields defaulted. -fn system_error(code: &'static str, message: &'static str) -> SystemError { - SystemError { - code: BunString::static_(code).into(), - message: BunString::static_(message).into(), - ..Default::default() - } -} - // ─────────────────── instance-method arg-decode helpers ────────────────── // // Note: a `#[bun_jsc::host_fn(method)]` proc-macro form of typed argument @@ -542,66 +532,54 @@ impl HTMLRewriter { // ───────────────────────── BufferOutputSink ────────────────────────────── +/// Drives one `HTMLRewriter.transform()` call: pulls input chunks from the +/// source body via `ResumableSink`, feeds them to lol-html, and delivers the +/// rewritten output to a `ByteStream` that backs the returned `Response` body. +/// +/// The rewriter's `OutputSink` writes to that `ByteStream` (a separate +/// allocation), not back into this struct, so driving the rewriter never +/// re-enters its owner. lol-html itself is still borrowed exclusively during +/// `write()`/`end()`; `writing` + `pending_*` guard the one path +/// (`wait_for_promise` inside a handler on the `Source::Bytes` native pipe) +/// that can deliver the next input chunk while a `write()` is on the stack. #[derive(bun_ptr::CellRefCounted)] pub struct BufferOutputSink { - // Intrusive RefCount; *Self is the `SinkRef` carried inside `rewriter`. ref_count: Cell, - pub global: GlobalRef, // JSC_BORROW - pub bytes: MutableString, - // Heap-allocated (never held by value): `run_output_sink` must reach the - // rewriter through a raw pointer, never a `&mut` of `*sink`, because the - // output sink re-enters `&mut *sink` while the rewriter runs. - pub rewriter: *mut lol_html::HtmlRewriter<'static, SinkRef>, // null when unset + pub global: GlobalRef, + rewriter: Cell<*mut lol_html::HtmlRewriter<'static, SinkRef>>, pub context: Rc>, - pub response: *mut Response, // BORROW_FIELD: kept alive by response_value Strong - pub response_value: StrongOptional, - pub input_buffer: MutableString, - // The `heap::into_raw` root pointer (set in `init()` once the heap address - // is known). `ResumableSinkContext` enters via `&mut self`, but - // `run_output_sink` re-enters `SinkRef::handle_chunk` via the root pointer; - // under Stacked Borrows that pops the `&mut self` tag, so the trait impls - // recover root provenance from here before driving the rewriter. - this: *mut BufferOutputSink, - // Points at the `sink_error` stack local in `init()`; - // only written while `init()` is on the stack. - // See `write_tmp_sync_error` for the full liveness/provenance argument. - pub tmp_sync_error: Option>, + /// GC root for the output `ByteStream`'s JS wrapper; `SinkRef` writes to + /// its `context` payload via [`output_bytes`](Self::output_bytes). + output: webcore::readable_stream::Strong, + writing: Cell, + pending_input: Cell>, + pending_end: Cell>>, } impl ResumableSinkContext for BufferOutputSink { - fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { - let _ = self.input_buffer.append(bytes); + fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure { + // SAFETY: `this` is the live context registered in `ResumableSink::init`. + unsafe { Self::feed(this, bytes) }; ResumableSinkBackpressure::WantMore } - fn write_end_request(&mut self, err: Option) { - // Recover root provenance (see the `this` field doc) and release the - // `&mut self` borrow before re-entering the rewriter. - let sink = self.this; - // SAFETY: `sink` is the live `heap::into_raw` allocation; the +1 taken - // for the in-flight reader in `init()` is consumed by `on_input_end`. - unsafe { Self::on_input_end(sink, err) }; + fn write_end_request(this: *mut Self, err: Option) { + // SAFETY: `this` is the live context registered in `ResumableSink::init`. + if unsafe { (*this).writing.get() } { + if let Some(err) = err { + err.ensure_still_alive(); + } + // SAFETY: see above. + unsafe { (*this).pending_end.set(Some(err)) }; + return; + } + // SAFETY: `this` is live; the +1 taken for the in-flight reader in + // `init()` is consumed here. + unsafe { Self::finish(this, err) }; } } impl BufferOutputSink { - // `ref_()`/`deref()` provided by `#[derive(CellRefCounted)]`. - - /// Single unsafe deref site for the set-once - /// `tmp_sync_error: Option>` field, so the two callers in - /// `on_finished_buffering` stay safe. `tmp_sync_error` points at the - /// `sink_error: Cell` stack local in [`init`]; it is only written - /// through on the synchronous (`is_async == false`) path while `init` is - /// still on the stack, so the pointee is live and the `Cell`-derived - /// pointer carries `SharedReadWrite` provenance. - #[inline] - fn write_tmp_sync_error(sink: *mut Self, err: JSValue) { - // SAFETY: `sink` is a live heap allocation (refcount > 0, caller - // invariant); `tmp_sync_error` was set in `init()` and the synchronous - // caller is reached only while `init()` is still on the stack. - unsafe { *(*sink).tmp_sync_error.unwrap().as_ptr() = err }; - } - /// # Safety /// `original` must point to a live `Response` whose JS wrapper is kept /// alive for the duration of this call. @@ -610,78 +588,40 @@ impl BufferOutputSink { global: &JSGlobalObject, original: *mut Response, ) -> JsResult { - let sink = bun_core::heap::into_raw(Box::new(BufferOutputSink { - ref_count: Cell::new(1), - global: GlobalRef::from(global), - bytes: MutableString::init_empty(), - rewriter: core::ptr::null_mut(), - context, - response: core::ptr::null_mut(), - response_value: StrongOptional::empty(), - input_buffer: MutableString::init_empty(), - this: core::ptr::null_mut(), - tmp_sync_error: None, - })); - // SAFETY: `sink` is the `heap::into_raw` allocation above. - unsafe { (*sink).this = sink }; - // SAFETY: `sink` is the `heap::into_raw` allocation above; refcount >= 1. - let _sink_guard = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; - // Note: do not hold a long-lived `&mut *sink` here — the same - // allocation is also written through the raw pointer by the lol-html - // output-sink callback during `bufferer.run()` and by `deref(sink)` - // below. Access fields via raw-pointer place expressions instead. - - let result = bun_core::heap::into_raw(Box::new(Response::init( - webcore::response::Init { - status_code: 200, + // The output Response body is a ByteStream from the start so `SinkRef` + // never reaches back into this struct and every consumer path + // (`.text()`, `.body.getReader()`, `Bun.serve`) reads the same stream. + let source = webcore::readable_stream::NewSource::::new_mut( + webcore::readable_stream::NewSource { + context: ByteStream::default(), + global_this: Some(bun_ptr::BackRef::new(global)), ..Default::default() }, - webcore::Body::new({ - let mut pv = webcore::body::PendingValue::new(global); - pv.task = Some(sink.cast::()); - webcore::body::Value::Locked(pv) - }), - BunString::empty(), - false, - ))); + ); + source.context.setup(); + let out_bytes: *mut ByteStream = &raw mut source.context; + let out_stream_js = source.to_readable_stream(global)?; + let out_readable = ReadableStream { + ptr: webcore::readable_stream::Source::Bytes(out_bytes), + value: out_stream_js, + }; - // SAFETY: sink was just allocated via heap::alloc above; refcount==1. - unsafe { (*sink).response = result }; - // Note (Stacked Borrows): `sink_error` is written via raw pointer - // by the unhandled-rejection handler during `bufferer.run()` and via - // `tmp_sync_error` from `on_finished_buffering`. Use a `Cell` so the - // exported `*mut` (via `Cell::as_ptr`, i.e. `UnsafeCell::get`) carries - // SharedReadWrite provenance — local `.get()` reads do NOT invalidate - // the stored raw pointer the way a `&`/`&mut` reborrow of a plain - // `mut` local would. - let sink_error: core::cell::Cell = core::cell::Cell::new(JSValue::ZERO); - let sink_error_ptr: *mut JSValue = sink_error.as_ptr(); // SAFETY: original is a live *Response passed from begin_transform; its // JS wrapper is on the caller's stack. let input_size = unsafe { (*original).get_body_len() }; - // SAFETY: bun_vm() returns the live VM raw ptr; VM outlives this fn. - let vm: &mut VirtualMachine = global.bun_vm().as_mut(); - // Since we're still using vm.waitForPromise, we have to also override - // the error rejection handler. That way, we can propagate errors to the - // caller. - let scope = vm.unhandled_rejection_scope(); - let prev_unhandled_pending_rejection_to_capture = vm.unhandled_pending_rejection_to_capture; - vm.unhandled_pending_rejection_to_capture = Some(sink_error_ptr); - // SAFETY: sink is a live heap allocation (refcount >= 1); sink_error_ptr - // is non-null (addr of stack local). - unsafe { (*sink).tmp_sync_error = Some(NonNull::new_unchecked(sink_error_ptr)) }; - vm.on_unhandled_rejection = - VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; - // Read the *live* slot at scope exit (Cell shares provenance with the - // raw-pointer writers). - scopeguard::defer! { - sink_error.get().ensure_still_alive(); - // SAFETY: VM outlives this guard (sync stack frame). - let vm = VirtualMachine::get().as_mut(); - vm.unhandled_pending_rejection_to_capture = prev_unhandled_pending_rejection_to_capture; - scope.apply(vm); - } + let sink = bun_core::heap::into_raw(Box::new(BufferOutputSink { + ref_count: Cell::new(1), + global: GlobalRef::from(global), + rewriter: Cell::new(core::ptr::null_mut()), + context, + output: webcore::readable_stream::Strong::init(out_readable, global), + writing: Cell::new(false), + pending_input: Cell::new(Vec::new()), + pending_end: Cell::new(None), + })); + // SAFETY: `sink` is the `heap::into_raw` allocation above; refcount == 1. + let _sink_guard = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; // The handler closures point into `Box`es owned by `(*sink).context`, // which `sink` keeps alive for the rewriter's whole lifetime. @@ -689,9 +629,6 @@ impl BufferOutputSink { // of `(*sink).context` is released at the end of this statement. let (element_content_handlers, document_content_handlers) = unsafe { build_settings(&mut (*sink).context.borrow_mut()) }; - // `SinkRef` carries the raw `sink` (`heap::into_raw` root) so every - // `(*sink).field` access shares its provenance; `run_output_sink` - // reaches the rewriter through a raw pointer, never `&mut *sink`. let rewriter = bun_core::heap::into_raw(Box::new(lol_html::HtmlRewriter::new( lol_html::Settings { element_content_handlers, @@ -711,10 +648,22 @@ impl BufferOutputSink { enable_esi_tags: false, adjust_charset_on_meta_tag: false, }, - SinkRef(sink), + SinkRef(out_bytes), ))); // SAFETY: sink is a live heap allocation (refcount >= 1). - unsafe { (*sink).rewriter = rewriter }; + unsafe { (*sink).rewriter.set(rewriter) }; + + let result = bun_core::heap::into_raw(Box::new(Response::init( + webcore::response::Init { + status_code: 200, + ..Default::default() + }, + webcore::Body::new( + webcore::body::Value::from_readable_stream_without_lock_check(out_readable, global), + ), + BunString::empty(), + false, + ))); // SAFETY: result and original are both live *Response (result allocated // above, original kept alive by caller); no aliasing &mut exists. @@ -726,60 +675,52 @@ impl BufferOutputSink { ); // https://github.com/oven-sh/bun/issues/3334 - // Note: `clone_this` takes `&mut self`, so use the `_mut` - // accessor (original is `*mut Response`). `clone_this` only reads - // `self` (FFI mutates a freshly-allocated clone, not the receiver). if let Some(headers) = (*original).get_init_headers_mut() { let cloned = headers.clone_this(global)?; (*result).set_init_headers(cloned.map(|p| HeadersRef::adopt(p))); } + (*result).set_url((*original).url().clone()); } + // SAFETY: `result` is a live heap allocation; `to_js` transfers + // ownership to the JS wrapper. + let response_js_value = unsafe { (*result).to_js(global) }; + response_js_value.ensure_still_alive(); - // Hold off on cloning until we're actually done. - // SAFETY: (*sink).response == result (set above), live heap allocation. - let response_js_value = unsafe { (*(*sink).response).to_js(&(*sink).global) }; - // SAFETY: sink is a live heap allocation (refcount >= 1). - unsafe { (*sink).response_value.set(global, response_js_value) }; - - // SAFETY: result/original are live *Response (see SAFETY note above). - // `url()` is +0 borrowed-bits; `set_url` takes +1 — `.clone()` to bump. - unsafe { (*result).set_url((*original).url().clone()) }; + // `handler_callback` runs user JS via `wait_for_promise`; capture any + // handler error while `feed`/`finish` drive the rewriter. + // SAFETY: bun_vm() returns the live VM raw ptr; VM outlives this fn. + let vm: &mut VirtualMachine = global.bun_vm().as_mut(); + let scope = vm.unhandled_rejection_scope(); + let prev_capture = vm.unhandled_pending_rejection_to_capture; + let sink_error: Cell = Cell::new(JSValue::ZERO); + vm.unhandled_pending_rejection_to_capture = Some(sink_error.as_ptr()); + vm.on_unhandled_rejection = + VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; + scopeguard::defer! { + sink_error.get().ensure_still_alive(); + let vm = VirtualMachine::get().as_mut(); + vm.unhandled_pending_rejection_to_capture = prev_capture; + scope.apply(vm); + } // SAFETY: original is a live *Response kept alive by caller. let value = unsafe { (*original).get_body_value() }; - // SAFETY: original is a live *Response kept alive by caller; sink live. let owned_readable_stream = - unsafe { (*original).get_body_readable_stream(&(*sink).global) }; - response_js_value.ensure_still_alive(); + // SAFETY: original is a live *Response kept alive by caller. + unsafe { (*original).get_body_readable_stream(global) }; - // SAFETY: sink is a live heap allocation (refcount >= 1). - unsafe { (*sink).ref_() }; - // SAFETY: sink is a live heap allocation; `start_reading_input` may - // synchronously invoke `on_input_end`, which (via the rewriter's output - // sink) re-enters `SinkRef::handle_chunk` through the same root `sink` - // pointer. No `&mut *sink` is formed here. - if let Err(err) = unsafe { Self::start_reading_input(sink, value, owned_readable_stream) } { - // SAFETY: `sink` is a live `heap::into_raw` allocation; release the - // ref taken for the in-flight reader. - unsafe { BufferOutputSink::deref(sink) }; - return Ok((*err).to_error_instance(global)); - } + // SAFETY: sink is a live heap allocation (refcount >= 1). `new` bumps; + // `forget` hands the +1 to whatever calls `finish` (consumed there by + // `ScopedRef::adopt`). + let in_flight = unsafe { bun_ptr::ScopedRef::::new(sink) }; + // SAFETY: sink is a live heap allocation. + unsafe { Self::start_reading_input(sink, value, owned_readable_stream)? }; + in_flight.forget(); - // SAFETY: sink is a live heap allocation (refcount >= 1). Nulling - // `tmp_sync_error` both invalidates the stack-local pointer before it - // goes out of scope and signals to `on_input_end` that any later call - // arrived from the event loop. - unsafe { (*sink).tmp_sync_error = None }; - - // sync error occurs — read via the Cell (shares SharedReadWrite - // provenance with the raw-pointer writers; see Note above). let captured = sink_error.get(); if !captured.is_empty() { captured.ensure_still_alive(); captured.unprotect(); - // Throw directly: the callers gate on `JSValue::to_error()`, which - // only recognises `ErrorInstance`/`Exception`, so an abort reason - // (a DOMException or any user value) would be returned instead. return Err(global.throw_value(captured)); } @@ -787,327 +728,225 @@ impl BufferOutputSink { Ok(response_js_value) } - /// Dispatch the input `Response` body into the rewriter. - /// - /// Materialised bodies (string / blob / buffer / empty) run the rewrite - /// immediately; a file-backed blob schedules an async read; anything - /// carrying a `ReadableStream` hands it to `ResumableSink`, which drives - /// every stream source kind (`Bytes` via native pipe, `JavaScript` / - /// `Direct` / `Blob` / `File` via the JS pump) into - /// `write_request_data` / `write_end_request`. - /// /// # Safety /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0; `(*sink).rewriter` and `(*sink).response` must be set. - /// On `Ok`, the +1 taken for the in-flight reader in `init()` is (or will - /// be) consumed by `on_input_end`; on `Err` the caller releases it. + /// refcount > 0; `(*sink).rewriter` must be set. The +1 taken for the + /// in-flight reader in `init()` is consumed by `finish` on every path. unsafe fn start_reading_input( sink: *mut Self, value: &mut webcore::body::Value, owned_readable_stream: Option, - ) -> Result<(), Box> { + ) -> JsResult<()> { // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). let global = unsafe { (*sink).global }; - value.to_blob_if_possible(); - - let readable_stream: ReadableStream = match value { - webcore::body::Value::Used => { - return Err(Box::new(system_error( - "ERR_STREAM_ALREADY_FINISHED", - "Stream already used, please create a new one", - ))); - } - webcore::body::Value::Empty | webcore::body::Value::Null => { - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, None) }; - return Ok(()); - } - webcore::body::Value::Error(err) => { - let js_err = err.to_js(&global); - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, Some(js_err)) }; - return Ok(()); - } - webcore::body::Value::WTFStringImpl(_) + let readable_stream = if let Some(stream) = owned_readable_stream { + stream + } else { + value.to_blob_if_possible(); + if let webcore::body::Value::WTFStringImpl(_) | webcore::body::Value::InternalBlob(_) - | webcore::body::Value::Blob(_) => { + | webcore::body::Value::Blob(_) = value + { + // Materialised bodies run the rewrite synchronously so that + // `transform(String | ArrayBuffer)` (which reads the output + // body back as a blob before returning) keeps its synchronous + // contract. let mut input = value.use_as_any_blob_allow_non_utf8_string(); - if input.needs_to_read_file() { - if let webcore::AnyBlob::Blob(blob) = &mut input { - struct LoadFileAdapter; - impl webcore::blob::InternalReadFileFn for LoadFileAdapter { - fn call( - sink: *mut BufferOutputSink, - bytes: webcore::blob::read_file::ReadFileResultType, - ) { - // SAFETY: `sink` was set from the live heap allocation - // below and outlives the read (refcount > 0). - unsafe { BufferOutputSink::on_file_read(sink, bytes) }; - } - } - blob.do_read_file_internal::(sink, &global); - return Ok(()); - } + if !input.needs_to_read_file() { + // SAFETY: see fn safety contract. + unsafe { Self::feed(sink, input.slice()) }; + input.detach(); + // SAFETY: see fn safety contract. + unsafe { Self::finish(sink, None) }; + return Ok(()); } + *value = webcore::body::Value::Blob(match input { + webcore::AnyBlob::Blob(b) => b, + _ => unreachable!(), + }); + } + let js_stream = value.to_readable_stream(&global)?; + if js_stream.is_null() { // SAFETY: see fn safety contract. - let _ = unsafe { (*sink).input_buffer.append(input.slice()) }; - input.detach(); - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, None) }; + unsafe { Self::finish(sink, None) }; return Ok(()); } - webcore::body::Value::Locked(locked) => 'brk: { - if let Some(stream) = owned_readable_stream { - break 'brk stream; - } - if let Some(stream) = locked.readable.get(&global) { - break 'brk stream; - } - let js_stream = match value.to_readable_stream(&global) { - Ok(v) => v, - Err(err) => { - let js_err = global.take_exception(err); - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, Some(js_err)) }; - return Ok(()); - } - }; - match ReadableStream::from_js(js_stream, &global) { - Ok(Some(stream)) => break 'brk stream, - _ => { - return Err(Box::new(system_error( - "ERR_STREAM_CANNOT_PIPE", - "Failed to pipe stream", - ))); - } + match ReadableStream::from_js(js_stream, &global)? { + Some(stream) => stream, + None => { + // SAFETY: see fn safety contract. + unsafe { Self::finish(sink, None) }; + return Ok(()); } } }; - *value = webcore::body::Value::Used; - // The caller's in-flight +1 on `BufferOutputSink` keeps `context` valid - // until `write_end_request` fires; the `ResumableSink` allocation - // itself is owned solely by its own lifecycle (pipe ref / JS wrapper). - // Not stored: `on_input_end` runs inside `ResumableSink::end_pipe`'s - // `&mut self`, so a back-pointer this struct could deref from `Drop` - // would retag the sink's root pointer and pop `end_pipe`'s borrow. + if !matches!(value, webcore::body::Value::Error(_)) { + *value = webcore::body::Value::Used; + } + // The in-flight +1 on `BufferOutputSink` keeps `context` valid until + // `write_end_request` fires; the sink's own lifecycle (pipe ref / JS + // wrapper) governs its allocation. let _ = ResumableHTMLRewriterSink::init(&global, readable_stream, sink); Ok(()) } + /// Feed one input chunk to lol-html. + /// + /// A re-entrant call (handler `wait_for_promise` draining the next + /// `Source::Bytes` pipe chunk) spills into `pending_input`; the outer call + /// drains it after `write()` returns. The JS-pump path cannot re-enter + /// (`JSResumableSinkPumpOperation::m_reading` guards the drain loop and the + /// next `resumableIssueRead` is not issued until `sink.write()` returns). + /// /// # Safety - /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0 (the +1 taken in `init()` is consumed by the forwarded - /// `on_input_end`). - unsafe fn on_file_read(sink: *mut Self, bytes: webcore::blob::read_file::ReadFileResultType) { + /// `sink` must be a live `BufferOutputSink` heap allocation (refcount > 0). + unsafe fn feed(sink: *mut Self, bytes: &[u8]) { // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). - let global = unsafe { (*sink).global }; - match bytes { - webcore::blob::read_file::ReadFileResultType::Err(err) => { - let js_err = err.to_error_instance(&global); - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, Some(js_err)) }; + let this = unsafe { &*sink }; + if this.writing.get() { + let mut spill = this.pending_input.take(); + spill.extend_from_slice(bytes); + this.pending_input.set(spill); + return; + } + if this.rewriter.get().is_null() { + return; + } + this.writing.set(true); + if let Err(e) = Self::rewrite(this, bytes) { + Self::fail(this, e); + } + loop { + let spill = this.pending_input.take(); + if spill.is_empty() || this.rewriter.get().is_null() { + break; } - webcore::blob::read_file::ReadFileResultType::Result(data) => { - // SAFETY: every producer sets `buf = heap::alloc(v.into_boxed_slice())` - // (read_file.rs); reclaim ownership here. Dropped at end of scope. - let buf = unsafe { Box::<[u8]>::from_raw(data.buf) }; - // SAFETY: sink is a live heap allocation (refcount > 0). - let _ = unsafe { (*sink).input_buffer.append(&buf) }; - // SAFETY: see fn safety contract. - unsafe { Self::on_input_end(sink, None) }; + if let Err(e) = Self::rewrite(this, &spill) { + Self::fail(this, e); + break; } } + this.writing.set(false); + if let Some(end) = this.pending_end.take() { + // SAFETY: see fn safety contract. + unsafe { Self::finish(sink, end) }; + } } - /// Deliver the buffered input (or an upstream error) to the rewriter. - /// Called at most once per transform; consumes the +1 taken in `init()` - /// for the in-flight reader. - /// - /// # Safety - /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0 (the +1 taken in `init()` is consumed here). - unsafe fn on_input_end(sink: *mut BufferOutputSink, js_err: Option) { - // SAFETY: `sink` was ref'd in `init()` before scheduling this callback; - // refcount > 0 so the allocation is live. `adopt` consumes that +1 on Drop. - let _g = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; - // Note: do not materialise `&mut *sink` here — the rewriter - // write/end calls below re-enter `SinkRef::handle_chunk` - // through the stored raw pointer, which forms - // its own `&mut *sink`. Holding an outer `&mut` across that re-entry - // is aliased-&mut UB. Access fields via raw-pointer place expressions - // instead (mirroring `init()`). - // - // SAFETY: sink was ref'd in init() before scheduling this callback; - // refcount > 0 so the allocation is live. - let global = unsafe { (*sink).global }; - // SAFETY: sink is a live heap allocation (refcount > 0). - let is_async = unsafe { (*sink).tmp_sync_error.is_none() }; - - if let Some(err) = js_err { - err.ensure_still_alive(); - // SAFETY: (*sink).response is the heap Response allocated in init() - // and kept alive by (*sink).response_value (Strong root). - let sink_body_value = unsafe { (*(*sink).response).get_body_value() }; - let sink_ptr_usize = sink as usize; - // If a `.body` readable is already attached, stay `Locked` so - // `to_error_instance` delivers the error to its ByteStream; clearing - // to `Empty` here would strand any pending `reader.read()` forever. - let has_readable = match sink_body_value { - webcore::body::Value::Locked(l) => l.readable.has(), - _ => false, - }; - if !has_readable - && matches!(sink_body_value, webcore::body::Value::Locked(l) - if l.task.map_or(0, |p| p as usize) == sink_ptr_usize && l.promise.is_none()) - { - // No reader and no pending read: normalize to `Empty` so - // `to_error_instance` takes the simple (non-`Locked`) path. - *sink_body_value = webcore::body::Value::Empty; - } else if matches!(sink_body_value, webcore::body::Value::Locked(l) - if l.task.map_or(0, |p| p as usize) == sink_ptr_usize && l.promise.is_some()) - { - if let webcore::body::Value::Locked(l) = sink_body_value { - l.on_receive_value = None; - l.task = None; - } - } - if is_async { - let ref_ = jsc::strong::Optional::create(err, &global); - let _ = sink_body_value - .to_error_instance(webcore::body::ValueError::JSValue(ref_), &global); - // TODO: properly propagate exception upwards - } else { - err.protect(); - Self::write_tmp_sync_error(sink, err); - } - // Do not `end()` the rewriter: that would run `done()`, replacing - // the error just stored on the body with the truncated output. - // `Drop` destroys the rewriter once the sink's refcount hits zero. - return; + /// Drive one `HtmlRewriter::write()` under a handler-error capture scope so + /// a thrown / rejected handler surfaces its original JS error instead of + /// the generic "rewriter has been stopped" lol-html wrapper. + fn rewrite(this: &Self, bytes: &[u8]) -> Result<(), JSValue> { + let global = this.global; + let vm: &mut VirtualMachine = global.bun_vm().as_mut(); + let prev_capture = vm.unhandled_pending_rejection_to_capture; + let captured: Cell = Cell::new(JSValue::ZERO); + vm.unhandled_pending_rejection_to_capture = Some(captured.as_ptr()); + let prev_handler = vm.on_unhandled_rejection; + vm.on_unhandled_rejection = + VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; + scopeguard::defer! { + let vm = VirtualMachine::get().as_mut(); + vm.unhandled_pending_rejection_to_capture = prev_capture; + vm.on_unhandled_rejection = prev_handler; } - // SAFETY: sink is a live heap allocation (refcount > 0). - let bytes = core::mem::replace( - unsafe { &mut (*sink).input_buffer }, - MutableString::init_empty(), - ); - // SAFETY: `sink` is live (refcount > 0, see fn safety contract). - let ret_err = unsafe { Self::run_output_sink(sink, bytes.list.as_slice(), is_async) }; - if let Some(ret_err) = ret_err { - ret_err.ensure_still_alive(); - ret_err.protect(); - Self::write_tmp_sync_error(sink, ret_err); + let rewriter = this.rewriter.get(); + // SAFETY: rewriter heap-allocated by init(), non-null (checked by + // caller), not yet freed. + let result = unsafe { (*rewriter).write(bytes) }; + match result { + Ok(()) => Ok(()), + Err(e) => { + let err = captured.get(); + if !err.is_empty() { + err.ensure_still_alive(); + err.unprotect(); + Err(err) + } else { + Err(create_lolhtml_error(&global, &e)) + } + } } } - /// Note: takes `*mut Self` (not `&mut self`) because - /// `HtmlRewriter::write/end` re-enter - /// `SinkRef::handle_chunk(&mut self)` through the - /// raw `*mut BufferOutputSink` captured at build time. A `&mut self` - /// receiver here would alias that inner `&mut` (Stacked Borrows UB). + /// Close the transform: consume the rewriter with `end()` (emits the final + /// empty chunk, which `SinkRef` forwards as `Done`), or push an upstream + /// error to the output stream. /// /// # Safety /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0; `(*sink).rewriter` and `(*sink).response` must be set. - unsafe fn run_output_sink(sink: *mut Self, bytes: &[u8], is_async: bool) -> Option { - // SAFETY: sink is a live heap allocation (refcount > 0, caller - // invariant). Read fields into locals before the rewriter calls so no - // borrow of `*sink` is live across the re-entrant output sink. - let (global, response, rewriter) = unsafe { - let _ = (*sink).bytes.grow_by(bytes.len()); // OOM/capacity: fire-and-forget - ((*sink).global, (*sink).response, (*sink).rewriter) - }; + /// refcount > 0 (the +1 taken in `init()` is consumed here). + unsafe fn finish(sink: *mut Self, err: Option) { + // SAFETY: `sink` was ref'd in `init()`; `adopt` consumes that +1 on Drop. + let _g = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; + // SAFETY: sink is a live heap allocation (refcount > 0). + let this = unsafe { &*sink }; - // SAFETY: rewriter heap-allocated by init(), not yet freed. - if let Err(e) = unsafe { (*rewriter).write(bytes) } { - // Poisoned: never call `end()` after a failed `write()`. The - // field stays non-null so `Drop` frees the rewriter. - if is_async { - // SAFETY: response kept alive by response_value Strong. - let _ = unsafe { (*response).get_body_value() }.to_error_instance( - webcore::body::ValueError::Message(lol_err_string(&e)), - &global, - ); - // TODO: properly propagate exception upwards - return None; - } else { - return Some(create_lolhtml_error(&global, &e)); - } + if let Some(err) = err { + Self::fail(this, err); + return; } - // `HtmlRewriter::end(self)` consumes the rewriter: null the field - // first so `Drop` does not free it a second time. - // SAFETY: sink is a live heap allocation (refcount > 0). - unsafe { (*sink).rewriter = core::ptr::null_mut() }; + let rewriter = this.rewriter.replace(core::ptr::null_mut()); + if rewriter.is_null() { + return; + } // SAFETY: `rewriter` was heap-allocated by init(); sole owner now. if let Err(e) = unsafe { bun_core::heap::take(rewriter) }.end() { - if is_async { - // SAFETY: response kept alive by response_value Strong. - let _ = unsafe { (*response).get_body_value() }.to_error_instance( - webcore::body::ValueError::Message(lol_err_string(&e)), - &global, - ); - // TODO: properly propagate exception upwards - return None; - } else { - return Some(create_lolhtml_error(&global, &e)); - } + Self::fail(this, create_lolhtml_error(&this.global, &e)); } - - None } - pub fn done(&mut self) { - // SAFETY: self.response is kept alive by self.response_value (Strong - // root) for the lifetime of this sink. - let body_value = unsafe { (*self.response).get_body_value() }; - let mut prev_value = core::mem::replace( - body_value, - webcore::body::Value::InternalBlob(webcore::InternalBlob { - bytes: core::mem::replace(&mut self.bytes, MutableString::init_empty()).list, - was_string: false, - }), - ); - - let _ = webcore::body::Value::resolve(&mut prev_value, body_value, &self.global, None); - // TODO: properly propagate exception upwards + fn output_bytes(&self) -> Option> { + self.output.get(&self.global).and_then(|s| s.ptr.bytes()) } - pub fn write(&mut self, bytes: &[u8]) { - let _ = self.bytes.append(bytes); // OOM/capacity: fire-and-forget + fn fail(this: &Self, err: JSValue) { + err.ensure_still_alive(); + let rewriter = this.rewriter.replace(core::ptr::null_mut()); + if !rewriter.is_null() { + // SAFETY: rewriter heap-allocated by init() and not yet freed. + unsafe { bun_core::heap::destroy(rewriter) }; + } + if let Some(bytes) = this.output_bytes() { + let ref_ = jsc::strong::Optional::create(err, &this.global); + let _ = bytes.on_data(streams::Result::Err(streams::StreamError::JSValue(ref_))); + } } } /// `lol_html::OutputSink` for the rewriter built in [`BufferOutputSink::init`]. -/// Carries a raw `*mut BufferOutputSink` (never a reference) so the rewriter -/// stored on the sink does not self-borrow. -pub struct SinkRef(*mut BufferOutputSink); +/// Writes to the output `ByteStream` (rooted via `BufferOutputSink::output`), +/// not back into its owner, so driving the rewriter never re-enters +/// `BufferOutputSink`. +pub struct SinkRef(*mut ByteStream); impl lol_html::OutputSink for SinkRef { fn handle_chunk(&mut self, chunk: &[u8]) { - // SAFETY: `self.0` is the sink that owns this rewriter (refcount > 0 - // inside `run_output_sink`), and no other `&mut *sink` is live — - // `run_output_sink` reads its fields into locals before the call. - let sink = unsafe { &mut *self.0 }; - // lol-html signals end-of-output with a zero-length final chunk. - if chunk.is_empty() { - sink.done(); + // SAFETY: `self.0` is the `NewSource` payload rooted by + // `BufferOutputSink::output` for as long as the rewriter lives. + // `ByteStream::on_data` takes `&self` (interior-mutable). + let bytes = unsafe { &*self.0 }; + let _ = if chunk.is_empty() { + bytes.on_data(streams::Result::Done) } else { - sink.write(chunk); - } + bytes.on_data(streams::Result::Temporary(bun_ptr::RawSlice::new(chunk))) + }; } } impl Drop for BufferOutputSink { fn drop(&mut self) { - // bytes, input_buffer, context (Rc), response_value (Strong) drop automatically. - if !self.rewriter.is_null() { + let rewriter = self.rewriter.get(); + if !rewriter.is_null() { // SAFETY: rewriter heap-allocated by init() and not yet freed - // (`run_output_sink` nulls the field before consuming it in `end`). - unsafe { bun_core::heap::destroy(self.rewriter) }; + // (`finish`/`fail` null the field before consuming it). + unsafe { bun_core::heap::destroy(rewriter) }; } + self.output.deinit(); } } diff --git a/src/runtime/webcore/ResumableSink.rs b/src/runtime/webcore/ResumableSink.rs index 23c00430644..e74fd695616 100644 --- a/src/runtime/webcore/ResumableSink.rs +++ b/src/runtime/webcore/ResumableSink.rs @@ -38,13 +38,22 @@ pub trait ResumableSinkJs { } /// Trait capturing the per-`Context` callbacks the sink invokes. -// The only -// in-tree impls (FetchTasklet / S3UploadStreamWrapper) mutate self in both -// callbacks (e.g. `detachSink`, `deref`, clearing `endPromise`), so these -// MUST be `&mut self`. +/// +/// Both methods take `*mut Self` (not `&mut self`) per the "borrow = ptr" +/// dispatch rule in src/CLAUDE.md: HTMLRewriter's `write_request_data` drives +/// `lol_html::HtmlRewriter::write`, which runs user async handlers via +/// `vm.wait_for_promise`; on the `Source::Bytes` native-pipe path that nested +/// event loop can deliver the next chunk and re-enter `on_write` on the same +/// context. A `&mut self` receiver would be aliased on the re-entrant call. +/// Impls that do not re-enter (FetchTasklet, S3) dereference once at the top. +/// +/// # Safety +/// `this` is the live heap allocation stored in [`ResumableSink::context`]; +/// callers only invoke these via [`ResumableSink::on_write`] / +/// [`ResumableSink::on_end`]. pub trait ResumableSinkContext { - fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure; - fn write_end_request(&mut self, err: Option); + fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure; + fn write_end_request(this: *mut Self, err: Option); } #[repr(u8)] @@ -122,15 +131,11 @@ impl ResumableSink ResumableSinkBackpressure { - // SAFETY: `context` is a BACKREF to the owning Context (FetchTasklet / - // S3UploadStreamWrapper) which outlives this sink — see LIFETIMES.tsv. - // Dereferenced as `&mut` because impls mutate (detachSink, deref, etc.). - unsafe { (*ctx).write_request_data(bytes) } + Context::write_request_data(ctx, bytes) } #[inline] fn on_end(ctx: *mut Context, err: Option) { - // SAFETY: see on_write. - unsafe { (*ctx).write_end_request(err) } + Context::write_end_request(ctx, err) } pub fn constructor(global: &JSGlobalObject, _frame: &CallFrame) -> JsResult<*mut Self> { @@ -632,12 +637,15 @@ impl_resumable_sink_js!( // (S3UploadStreamWrapper's impl lives next to its struct in s3/client.rs.) impl ResumableSinkContext for FetchTasklet { #[inline] - fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { - FetchTasklet::write_request_data(self, bytes) + fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure { + // SAFETY: `this` is the live context registered in `ResumableSink::init`; + // FetchTasklet does not re-enter the sink from these callbacks. + FetchTasklet::write_request_data(unsafe { &mut *this }, bytes) } #[inline] - fn write_end_request(&mut self, err: Option) { - FetchTasklet::write_end_request(self, err) + fn write_end_request(this: *mut Self, err: Option) { + // SAFETY: see `write_request_data`. + FetchTasklet::write_end_request(unsafe { &mut *this }, err) } } diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index 80271e2b89e..fd3e0541b78 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -706,12 +706,15 @@ impl S3UploadStreamWrapper { impl ResumableSinkContext for S3UploadStreamWrapper { #[inline] - fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { - S3UploadStreamWrapper::write_request_data(self, bytes) + fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure { + // SAFETY: `this` is the live context registered in `ResumableSink::init`; + // S3UploadStreamWrapper does not re-enter the sink from these callbacks. + S3UploadStreamWrapper::write_request_data(unsafe { &mut *this }, bytes) } #[inline] - fn write_end_request(&mut self, err: Option) { - S3UploadStreamWrapper::write_end_request(self, err) + fn write_end_request(this: *mut Self, err: Option) { + // SAFETY: see `write_request_data`. + S3UploadStreamWrapper::write_end_request(unsafe { &mut *this }, err) } } From 886e0f31cc0c9c62eb3505898bef05fd4e74f61b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:49:43 +0000 Subject: [PATCH 06/13] HTMLRewriter: route all input through the JS pump; drop the re-entrancy guard Addresses the five review findings on 15f3adb: - The Source::Bytes native-pipe path could re-enter ResumableSink's own &mut self chain (Wrap::pipe -> on_stream_pipe -> end_pipe) when a handler's wait_for_promise nested the event loop, not just the context layer. Rather than propagating *mut Self through PipeHandler, ResumableSinkContext gains AVOID_NATIVE_PIPE (default false, true for BufferOutputSink) so HTMLRewriter always goes through the JS pump, whose m_reading guard and read-after-write sequencing already prevent re-entry. The trait reverts to &mut self and the writing / pending_input / pending_end fields (and the unrooted JSValue they stored) are gone. - rewrite() shadowed init()'s capture scope, so a sync handler error no longer made transform() throw. rewrite() is gone; create_lolhtml_error already reads vm.unhandled_pending_rejection_to_capture. A HandlerErrorScope RAII guard installs the capture slot around every feed/finish call (init()'s sync path, write_request_data, write_end_request), and a strong::Optional failed field on BufferOutputSink lets init() throw what fail() stored. - finish() now runs under the same capture scope as feed(), so an onDocument.end handler error on the async path surfaces the user's error. - The output Response is wrapped in a scopeguard so a clone_this? failure no longer leaks it and the Strong on its body stream. Other fallout of the ByteStream output landing: - transform(String | ArrayBuffer) reads the output back via get_body_readable_stream().to_any_blob(): Response::to_js migrates Locked.readable to the wrapper's cached stream slot, so use_as_any_blob_allow_non_utf8_string found an empty Strong. - start_reading_input handles Value::Error synchronously so transform() of an already-failed body still throws. - feed() copies the chunk before HtmlRewriter::write(): lol-html parses the first chunk straight from the input slice, and a handler can otherwise mutate bytes it has not tokenized yet. - The 'transform rejects when the upstream body fails' tests are adapted for streaming: chunks delivered before the failure now reach .body, so the assertions read to completion instead of expecting a single rejected read. - The '.body of a transform whose source is still pending' test is un-skipped (#19305 is fixed), and getReader / readableStreamToText are back in the 'every way of reading' suite. --- src/runtime/api/html_rewriter.rs | 346 ++++++++++---------------- src/runtime/webcore/ResumableSink.rs | 46 ++-- src/runtime/webcore/s3/client.rs | 11 +- test/js/workerd/html-rewriter.test.js | 66 ++--- 4 files changed, 191 insertions(+), 278 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 63188581243..32d9ea922a4 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -466,12 +466,13 @@ impl HTMLRewriter { return Ok(out_response_value); }; // SAFETY: out_response is the m_ctx of out_response_value (kept alive - // on the stack via ensure_still_alive above). - let mut blob = unsafe { - (*out_response) - .get_body_value() - .use_as_any_blob_allow_non_utf8_string() - }; + // on the stack via ensure_still_alive above). `to_js` migrated the + // output ByteStream to the wrapper's cached stream slot. + let mut blob = unsafe { (*out_response).get_body_readable_stream(global) } + .and_then(|mut s| s.to_any_blob(global)) + .unwrap_or(webcore::AnyBlob::Blob(Default::default())); + // SAFETY: out_response is live (see above). + unsafe { *(*out_response).get_body_value() = webcore::body::Value::Used }; let _out_guard = scopeguard::guard((out_response_value, out_response), |(v, r)| { // `Response.js.dangerouslySetPtr(v, null)` — null out the JS @@ -535,47 +536,70 @@ impl HTMLRewriter { /// Drives one `HTMLRewriter.transform()` call: pulls input chunks from the /// source body via `ResumableSink`, feeds them to lol-html, and delivers the /// rewritten output to a `ByteStream` that backs the returned `Response` body. -/// -/// The rewriter's `OutputSink` writes to that `ByteStream` (a separate -/// allocation), not back into this struct, so driving the rewriter never -/// re-enters its owner. lol-html itself is still borrowed exclusively during -/// `write()`/`end()`; `writing` + `pending_*` guard the one path -/// (`wait_for_promise` inside a handler on the `Source::Bytes` native pipe) -/// that can deliver the next input chunk while a `write()` is on the stack. #[derive(bun_ptr::CellRefCounted)] pub struct BufferOutputSink { ref_count: Cell, pub global: GlobalRef, rewriter: Cell<*mut lol_html::HtmlRewriter<'static, SinkRef>>, pub context: Rc>, - /// GC root for the output `ByteStream`'s JS wrapper; `SinkRef` writes to - /// its `context` payload via [`output_bytes`](Self::output_bytes). + /// GC root for the output `ByteStream`'s JS wrapper. output: webcore::readable_stream::Strong, - writing: Cell, - pending_input: Cell>, - pending_end: Cell>>, + /// First error pushed via [`fail`](Self::fail), so a synchronous handler + /// error inside `init()` still makes `transform()` throw. + failed: JsCell, } impl ResumableSinkContext for BufferOutputSink { - fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure { - // SAFETY: `this` is the live context registered in `ResumableSink::init`. - unsafe { Self::feed(this, bytes) }; + /// `feed` drives `HtmlRewriter::write`, which runs async handlers via + /// `wait_for_promise`. The JS pump cannot deliver the next chunk until + /// `sink.write()` returns (`m_reading` guard + read-after-write ordering); + /// the native pipe can, so skip it and let the pump read `Source::Bytes`. + const AVOID_NATIVE_PIPE: bool = true; + + fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { + let captured = Cell::new(JSValue::ZERO); + let _scope = HandlerErrorScope::enter(&self.global, &captured); + self.feed(bytes); ResumableSinkBackpressure::WantMore } - fn write_end_request(this: *mut Self, err: Option) { - // SAFETY: `this` is the live context registered in `ResumableSink::init`. - if unsafe { (*this).writing.get() } { - if let Some(err) = err { - err.ensure_still_alive(); - } - // SAFETY: see above. - unsafe { (*this).pending_end.set(Some(err)) }; - return; - } - // SAFETY: `this` is live; the +1 taken for the in-flight reader in - // `init()` is consumed here. - unsafe { Self::finish(this, err) }; + fn write_end_request(&mut self, err: Option) { + let captured = Cell::new(JSValue::ZERO); + let _scope = HandlerErrorScope::enter(&self.global, &captured); + self.finish(err); + // SAFETY: `&mut self` keeps the allocation live; releases the in-flight + // +1 taken in `init()`. `self` is not touched after this. + unsafe { Self::deref(core::ptr::from_mut(self)) }; + } +} + +/// RAII guard that points `vm.unhandled_pending_rejection_to_capture` at a +/// caller-owned cell and installs the quiet rejection handler for its scope. +/// `create_lolhtml_error` reads that slot to surface the user's thrown error. +struct HandlerErrorScope { + prev_capture: Option<*mut JSValue>, + rejection_scope: bun_jsc::virtual_machine::UnhandledRejectionScope, +} + +impl HandlerErrorScope { + fn enter(global: &JSGlobalObject, captured: &Cell) -> Self { + let vm: &mut VirtualMachine = global.bun_vm().as_mut(); + let scope = Self { + prev_capture: vm.unhandled_pending_rejection_to_capture, + rejection_scope: vm.unhandled_rejection_scope(), + }; + vm.unhandled_pending_rejection_to_capture = Some(captured.as_ptr()); + vm.on_unhandled_rejection = + VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; + scope + } +} + +impl Drop for HandlerErrorScope { + fn drop(&mut self) { + let vm = VirtualMachine::get().as_mut(); + vm.unhandled_pending_rejection_to_capture = self.prev_capture; + self.rejection_scope.apply(vm); } } @@ -588,9 +612,6 @@ impl BufferOutputSink { global: &JSGlobalObject, original: *mut Response, ) -> JsResult { - // The output Response body is a ByteStream from the start so `SinkRef` - // never reaches back into this struct and every consumer path - // (`.text()`, `.body.getReader()`, `Bun.serve`) reads the same stream. let source = webcore::readable_stream::NewSource::::new_mut( webcore::readable_stream::NewSource { context: ByteStream::default(), @@ -606,8 +627,7 @@ impl BufferOutputSink { value: out_stream_js, }; - // SAFETY: original is a live *Response passed from begin_transform; its - // JS wrapper is on the caller's stack. + // SAFETY: original is a live *Response kept alive by caller. let input_size = unsafe { (*original).get_body_len() }; let sink = bun_core::heap::into_raw(Box::new(BufferOutputSink { @@ -616,17 +636,12 @@ impl BufferOutputSink { rewriter: Cell::new(core::ptr::null_mut()), context, output: webcore::readable_stream::Strong::init(out_readable, global), - writing: Cell::new(false), - pending_input: Cell::new(Vec::new()), - pending_end: Cell::new(None), + failed: JsCell::new(jsc::strong::Optional::empty()), })); // SAFETY: `sink` is the `heap::into_raw` allocation above; refcount == 1. let _sink_guard = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; - // The handler closures point into `Box`es owned by `(*sink).context`, - // which `sink` keeps alive for the rewriter's whole lifetime. - // SAFETY: sink is a live heap allocation (refcount >= 1); the `RefMut` - // of `(*sink).context` is released at the end of this statement. + // SAFETY: sink is live; the `RefMut` is released at end of statement. let (element_content_handlers, document_content_handlers) = unsafe { build_settings(&mut (*sink).context.borrow_mut()) }; let rewriter = bun_core::heap::into_raw(Box::new(lol_html::HtmlRewriter::new( @@ -650,7 +665,7 @@ impl BufferOutputSink { }, SinkRef(out_bytes), ))); - // SAFETY: sink is a live heap allocation (refcount >= 1). + // SAFETY: sink is a live heap allocation. unsafe { (*sink).rewriter.set(rewriter) }; let result = bun_core::heap::into_raw(Box::new(Response::init( @@ -664,16 +679,18 @@ impl BufferOutputSink { BunString::empty(), false, ))); + let result_guard = scopeguard::guard(result, |r| { + // SAFETY: `r` is the `heap::into_raw` allocation above; sole owner. + Response::finalize(unsafe { Box::from_raw(r) }) + }); - // SAFETY: result and original are both live *Response (result allocated - // above, original kept alive by caller); no aliasing &mut exists. + // SAFETY: result and original are both live *Response. unsafe { (*result).set_init( (*original).get_method(), (*original).get_init_status_code(), (*original).get_init_status_text().clone(), ); - // https://github.com/oven-sh/bun/issues/3334 if let Some(headers) = (*original).get_init_headers_mut() { let cloned = headers.clone_this(global)?; @@ -681,84 +698,71 @@ impl BufferOutputSink { } (*result).set_url((*original).url().clone()); } - // SAFETY: `result` is a live heap allocation; `to_js` transfers - // ownership to the JS wrapper. - let response_js_value = unsafe { (*result).to_js(global) }; + // SAFETY: `result` is live; `to_js` transfers ownership to the wrapper. + let response_js_value = + unsafe { (*scopeguard::ScopeGuard::into_inner(result_guard)).to_js(global) }; response_js_value.ensure_still_alive(); - // `handler_callback` runs user JS via `wait_for_promise`; capture any - // handler error while `feed`/`finish` drive the rewriter. - // SAFETY: bun_vm() returns the live VM raw ptr; VM outlives this fn. - let vm: &mut VirtualMachine = global.bun_vm().as_mut(); - let scope = vm.unhandled_rejection_scope(); - let prev_capture = vm.unhandled_pending_rejection_to_capture; - let sink_error: Cell = Cell::new(JSValue::ZERO); - vm.unhandled_pending_rejection_to_capture = Some(sink_error.as_ptr()); - vm.on_unhandled_rejection = - VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; - scopeguard::defer! { - sink_error.get().ensure_still_alive(); - let vm = VirtualMachine::get().as_mut(); - vm.unhandled_pending_rejection_to_capture = prev_capture; - scope.apply(vm); - } - // SAFETY: original is a live *Response kept alive by caller. let value = unsafe { (*original).get_body_value() }; let owned_readable_stream = // SAFETY: original is a live *Response kept alive by caller. unsafe { (*original).get_body_readable_stream(global) }; - // SAFETY: sink is a live heap allocation (refcount >= 1). `new` bumps; - // `forget` hands the +1 to whatever calls `finish` (consumed there by - // `ScopedRef::adopt`). - let in_flight = unsafe { bun_ptr::ScopedRef::::new(sink) }; + { + let captured = Cell::new(JSValue::ZERO); + let _scope = HandlerErrorScope::enter(global, &captured); + // SAFETY: sink is a live heap allocation; `new` bumps, `forget` + // hands the +1 to `write_end_request` (or `finish` on the sync + // path), which releases it. + let in_flight = unsafe { bun_ptr::ScopedRef::::new(sink) }; + // SAFETY: sink is a live heap allocation. + unsafe { (*sink).start_reading_input(value, owned_readable_stream)? }; + in_flight.forget(); + } + // SAFETY: sink is a live heap allocation. - unsafe { Self::start_reading_input(sink, value, owned_readable_stream)? }; - in_flight.forget(); - - let captured = sink_error.get(); - if !captured.is_empty() { - captured.ensure_still_alive(); - captured.unprotect(); - return Err(global.throw_value(captured)); + if let Some(err) = unsafe { (*sink).failed.with_mut(|f| f.try_swap()) } { + return Err(global.throw_value(err)); } response_js_value.ensure_still_alive(); Ok(response_js_value) } - /// # Safety - /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0; `(*sink).rewriter` must be set. The +1 taken for the - /// in-flight reader in `init()` is consumed by `finish` on every path. - unsafe fn start_reading_input( - sink: *mut Self, + fn start_reading_input( + &mut self, value: &mut webcore::body::Value, owned_readable_stream: Option, ) -> JsResult<()> { - // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). - let global = unsafe { (*sink).global }; + let global = self.global; let readable_stream = if let Some(stream) = owned_readable_stream { stream } else { value.to_blob_if_possible(); + if let webcore::body::Value::Error(err) = value { + let js_err = err.to_js(&global); + self.fail(js_err); + // SAFETY: `&mut self` keeps the allocation live; releases the + // in-flight +1 taken in `init()`. + unsafe { Self::deref(core::ptr::from_mut(self)) }; + return Ok(()); + } if let webcore::body::Value::WTFStringImpl(_) | webcore::body::Value::InternalBlob(_) | webcore::body::Value::Blob(_) = value { - // Materialised bodies run the rewrite synchronously so that - // `transform(String | ArrayBuffer)` (which reads the output - // body back as a blob before returning) keeps its synchronous - // contract. let mut input = value.use_as_any_blob_allow_non_utf8_string(); if !input.needs_to_read_file() { - // SAFETY: see fn safety contract. - unsafe { Self::feed(sink, input.slice()) }; + // Run synchronously so `transform(String | ArrayBuffer)` can + // read the output body back as a blob before returning. + self.feed(input.slice()); input.detach(); - // SAFETY: see fn safety contract. - unsafe { Self::finish(sink, None) }; + self.finish(None); + // SAFETY: `&mut self` keeps the allocation live; releases + // the in-flight +1 taken in `init()`. + unsafe { Self::deref(core::ptr::from_mut(self)) }; return Ok(()); } *value = webcore::body::Value::Blob(match input { @@ -767,16 +771,12 @@ impl BufferOutputSink { }); } let js_stream = value.to_readable_stream(&global)?; - if js_stream.is_null() { - // SAFETY: see fn safety contract. - unsafe { Self::finish(sink, None) }; - return Ok(()); - } match ReadableStream::from_js(js_stream, &global)? { Some(stream) => stream, None => { - // SAFETY: see fn safety contract. - unsafe { Self::finish(sink, None) }; + self.finish(None); + // SAFETY: see above. + unsafe { Self::deref(core::ptr::from_mut(self)) }; return Ok(()); } } @@ -785,118 +785,37 @@ impl BufferOutputSink { if !matches!(value, webcore::body::Value::Error(_)) { *value = webcore::body::Value::Used; } - // The in-flight +1 on `BufferOutputSink` keeps `context` valid until - // `write_end_request` fires; the sink's own lifecycle (pipe ref / JS - // wrapper) governs its allocation. - let _ = ResumableHTMLRewriterSink::init(&global, readable_stream, sink); + let _ = ResumableHTMLRewriterSink::init(&global, readable_stream, core::ptr::from_mut(self)); Ok(()) } - /// Feed one input chunk to lol-html. - /// - /// A re-entrant call (handler `wait_for_promise` draining the next - /// `Source::Bytes` pipe chunk) spills into `pending_input`; the outer call - /// drains it after `write()` returns. The JS-pump path cannot re-enter - /// (`JSResumableSinkPumpOperation::m_reading` guards the drain loop and the - /// next `resumableIssueRead` is not issued until `sink.write()` returns). - /// - /// # Safety - /// `sink` must be a live `BufferOutputSink` heap allocation (refcount > 0). - unsafe fn feed(sink: *mut Self, bytes: &[u8]) { - // SAFETY: sink is a live heap allocation (refcount > 0, caller invariant). - let this = unsafe { &*sink }; - if this.writing.get() { - let mut spill = this.pending_input.take(); - spill.extend_from_slice(bytes); - this.pending_input.set(spill); - return; - } - if this.rewriter.get().is_null() { + fn feed(&self, bytes: &[u8]) { + let rewriter = self.rewriter.get(); + if rewriter.is_null() { return; } - this.writing.set(true); - if let Err(e) = Self::rewrite(this, bytes) { - Self::fail(this, e); - } - loop { - let spill = this.pending_input.take(); - if spill.is_empty() || this.rewriter.get().is_null() { - break; - } - if let Err(e) = Self::rewrite(this, &spill) { - Self::fail(this, e); - break; - } - } - this.writing.set(false); - if let Some(end) = this.pending_end.take() { - // SAFETY: see fn safety contract. - unsafe { Self::finish(sink, end) }; + // lol-html parses a first chunk straight from the input slice, so a + // handler that mutates/transfers the user's buffer mid-`write()` would + // corrupt tokens past the current position. + let owned: Vec = bytes.to_vec(); + // SAFETY: rewriter heap-allocated by init(), non-null, not yet freed. + if let Err(e) = unsafe { (*rewriter).write(&owned) } { + self.fail(create_lolhtml_error(&self.global, &e)); } } - /// Drive one `HtmlRewriter::write()` under a handler-error capture scope so - /// a thrown / rejected handler surfaces its original JS error instead of - /// the generic "rewriter has been stopped" lol-html wrapper. - fn rewrite(this: &Self, bytes: &[u8]) -> Result<(), JSValue> { - let global = this.global; - let vm: &mut VirtualMachine = global.bun_vm().as_mut(); - let prev_capture = vm.unhandled_pending_rejection_to_capture; - let captured: Cell = Cell::new(JSValue::ZERO); - vm.unhandled_pending_rejection_to_capture = Some(captured.as_ptr()); - let prev_handler = vm.on_unhandled_rejection; - vm.on_unhandled_rejection = - VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value; - scopeguard::defer! { - let vm = VirtualMachine::get().as_mut(); - vm.unhandled_pending_rejection_to_capture = prev_capture; - vm.on_unhandled_rejection = prev_handler; - } - - let rewriter = this.rewriter.get(); - // SAFETY: rewriter heap-allocated by init(), non-null (checked by - // caller), not yet freed. - let result = unsafe { (*rewriter).write(bytes) }; - match result { - Ok(()) => Ok(()), - Err(e) => { - let err = captured.get(); - if !err.is_empty() { - err.ensure_still_alive(); - err.unprotect(); - Err(err) - } else { - Err(create_lolhtml_error(&global, &e)) - } - } - } - } - - /// Close the transform: consume the rewriter with `end()` (emits the final - /// empty chunk, which `SinkRef` forwards as `Done`), or push an upstream - /// error to the output stream. - /// - /// # Safety - /// `sink` must be a live `BufferOutputSink` heap allocation with - /// refcount > 0 (the +1 taken in `init()` is consumed here). - unsafe fn finish(sink: *mut Self, err: Option) { - // SAFETY: `sink` was ref'd in `init()`; `adopt` consumes that +1 on Drop. - let _g = unsafe { bun_ptr::ScopedRef::::adopt(sink) }; - // SAFETY: sink is a live heap allocation (refcount > 0). - let this = unsafe { &*sink }; - + fn finish(&self, err: Option) { if let Some(err) = err { - Self::fail(this, err); + self.fail(err); return; } - - let rewriter = this.rewriter.replace(core::ptr::null_mut()); + let rewriter = self.rewriter.replace(core::ptr::null_mut()); if rewriter.is_null() { return; } // SAFETY: `rewriter` was heap-allocated by init(); sole owner now. if let Err(e) = unsafe { bun_core::heap::take(rewriter) }.end() { - Self::fail(this, create_lolhtml_error(&this.global, &e)); + self.fail(create_lolhtml_error(&self.global, &e)); } } @@ -904,31 +823,32 @@ impl BufferOutputSink { self.output.get(&self.global).and_then(|s| s.ptr.bytes()) } - fn fail(this: &Self, err: JSValue) { + fn fail(&self, err: JSValue) { err.ensure_still_alive(); - let rewriter = this.rewriter.replace(core::ptr::null_mut()); + let rewriter = self.rewriter.replace(core::ptr::null_mut()); if !rewriter.is_null() { // SAFETY: rewriter heap-allocated by init() and not yet freed. unsafe { bun_core::heap::destroy(rewriter) }; } - if let Some(bytes) = this.output_bytes() { - let ref_ = jsc::strong::Optional::create(err, &this.global); + if !self.failed.get().has() { + self.failed + .with_mut(|f| *f = jsc::strong::Optional::create(err, &self.global)); + } + if let Some(bytes) = self.output_bytes() { + let ref_ = jsc::strong::Optional::create(err, &self.global); let _ = bytes.on_data(streams::Result::Err(streams::StreamError::JSValue(ref_))); } } } -/// `lol_html::OutputSink` for the rewriter built in [`BufferOutputSink::init`]. -/// Writes to the output `ByteStream` (rooted via `BufferOutputSink::output`), -/// not back into its owner, so driving the rewriter never re-enters -/// `BufferOutputSink`. +/// Writes the rewriter's output to the `ByteStream` rooted by +/// `BufferOutputSink::output`, never back into its owner. pub struct SinkRef(*mut ByteStream); impl lol_html::OutputSink for SinkRef { fn handle_chunk(&mut self, chunk: &[u8]) { // SAFETY: `self.0` is the `NewSource` payload rooted by - // `BufferOutputSink::output` for as long as the rewriter lives. - // `ByteStream::on_data` takes `&self` (interior-mutable). + // `BufferOutputSink::output`; `on_data` takes `&self`. let bytes = unsafe { &*self.0 }; let _ = if chunk.is_empty() { bytes.on_data(streams::Result::Done) @@ -942,14 +862,12 @@ impl Drop for BufferOutputSink { fn drop(&mut self) { let rewriter = self.rewriter.get(); if !rewriter.is_null() { - // SAFETY: rewriter heap-allocated by init() and not yet freed - // (`finish`/`fail` null the field before consuming it). + // SAFETY: heap-allocated by init(), not yet freed. unsafe { bun_core::heap::destroy(rewriter) }; } self.output.deinit(); } } - // ──────────────────────── DocumentHandler ──────────────────────────────── pub struct DocumentHandler { diff --git a/src/runtime/webcore/ResumableSink.rs b/src/runtime/webcore/ResumableSink.rs index e74fd695616..bd841828a25 100644 --- a/src/runtime/webcore/ResumableSink.rs +++ b/src/runtime/webcore/ResumableSink.rs @@ -38,22 +38,16 @@ pub trait ResumableSinkJs { } /// Trait capturing the per-`Context` callbacks the sink invokes. -/// -/// Both methods take `*mut Self` (not `&mut self`) per the "borrow = ptr" -/// dispatch rule in src/CLAUDE.md: HTMLRewriter's `write_request_data` drives -/// `lol_html::HtmlRewriter::write`, which runs user async handlers via -/// `vm.wait_for_promise`; on the `Source::Bytes` native-pipe path that nested -/// event loop can deliver the next chunk and re-enter `on_write` on the same -/// context. A `&mut self` receiver would be aliased on the re-entrant call. -/// Impls that do not re-enter (FetchTasklet, S3) dereference once at the top. -/// -/// # Safety -/// `this` is the live heap allocation stored in [`ResumableSink::context`]; -/// callers only invoke these via [`ResumableSink::on_write`] / -/// [`ResumableSink::on_end`]. pub trait ResumableSinkContext { - fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure; - fn write_end_request(this: *mut Self, err: Option); + /// Skip the `Source::Bytes` native-pipe fast path and always drive the + /// stream through the JS pump. Set by contexts whose `write_request_data` + /// nests the event loop (HTMLRewriter's `wait_for_promise`), which could + /// re-enter the native pipe callback; the JS pump's `m_reading` guard and + /// read-after-write sequencing prevent that. + const AVOID_NATIVE_PIPE: bool = false; + + fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure; + fn write_end_request(&mut self, err: Option); } #[repr(u8)] @@ -131,11 +125,14 @@ impl ResumableSink ResumableSinkBackpressure { - Context::write_request_data(ctx, bytes) + // SAFETY: `context` is a BACKREF to the owning Context which outlives + // this sink. Dereferenced as `&mut` because impls mutate. + unsafe { (*ctx).write_request_data(bytes) } } #[inline] fn on_end(ctx: *mut Context, err: Option) { - Context::write_end_request(ctx, err) + // SAFETY: see on_write. + unsafe { (*ctx).write_end_request(err) } } pub fn constructor(global: &JSGlobalObject, _frame: &CallFrame) -> JsResult<*mut Self> { @@ -187,7 +184,9 @@ impl ResumableSink ResumableSinkBackpressure { - // SAFETY: `this` is the live context registered in `ResumableSink::init`; - // FetchTasklet does not re-enter the sink from these callbacks. - FetchTasklet::write_request_data(unsafe { &mut *this }, bytes) + fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { + FetchTasklet::write_request_data(self, bytes) } #[inline] - fn write_end_request(this: *mut Self, err: Option) { - // SAFETY: see `write_request_data`. - FetchTasklet::write_end_request(unsafe { &mut *this }, err) + fn write_end_request(&mut self, err: Option) { + FetchTasklet::write_end_request(self, err) } } diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index fd3e0541b78..80271e2b89e 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -706,15 +706,12 @@ impl S3UploadStreamWrapper { impl ResumableSinkContext for S3UploadStreamWrapper { #[inline] - fn write_request_data(this: *mut Self, bytes: &[u8]) -> ResumableSinkBackpressure { - // SAFETY: `this` is the live context registered in `ResumableSink::init`; - // S3UploadStreamWrapper does not re-enter the sink from these callbacks. - S3UploadStreamWrapper::write_request_data(unsafe { &mut *this }, bytes) + fn write_request_data(&mut self, bytes: &[u8]) -> ResumableSinkBackpressure { + S3UploadStreamWrapper::write_request_data(self, bytes) } #[inline] - fn write_end_request(this: *mut Self, err: Option) { - // SAFETY: see `write_request_data`. - S3UploadStreamWrapper::write_end_request(unsafe { &mut *this }, err) + fn write_end_request(&mut self, err: Option) { + S3UploadStreamWrapper::write_end_request(self, err) } } diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index b6a2392c738..d64d10f435f 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -180,9 +180,6 @@ describe("HTMLRewriter", () => { // Must reject with the upstream connection error, and must never // resolve with the truncated document. expect(await text).toEqual(rejectedWithConnectionError); - // The body is now in its error state. A second read must report the - // same failure, not resolve as an empty "successful" document. - expect(await settle(transformed.text())).toEqual(rejectedWithConnectionError); }); }); @@ -196,16 +193,22 @@ describe("HTMLRewriter", () => { }); it(".body on the transformed response is an errored stream", async () => { + // The rewrite streams, so bytes that arrive before the failure are + // delivered; read to completion and assert the stream ends in an error + // instead of closing cleanly as a truncated "successful" document. + async function readAll(reader) { + while (true) { + const r = await settle(reader.read()); + if (r.rejected) return r; + if (r.value.done) return r; + } + } await withPartialBodyServer(async (url, release) => { const res = await fetch(url); const transformed = rewriter().transform(res); - const text = settle(transformed.text()); + const reader = transformed.body.getReader(); release(); - // Barrier: once this has rejected, the body is in its error state. - expect(await text).toEqual(rejectedWithConnectionError); - // Reading `.body` must reject with the same upstream error instead of - // closing cleanly as an empty "successful" document. - expect(await settle(transformed.body.getReader().read())).toEqual(rejectedWithConnectionError); + expect(await readAll(reader)).toEqual(rejectedWithConnectionError); }); }); @@ -213,12 +216,15 @@ describe("HTMLRewriter", () => { await withPartialBodyServer(async (url, release) => { const res = await fetch(url); const transformed = rewriter().transform(res); - // Start the read BEFORE the upstream fails. This is the one shape - // (readable attached, no pending promise) where the error must reach - // the attached stream; discarding it would strand this read forever. - const read = settle(transformed.body.getReader().read()); + const reader = transformed.body.getReader(); + // Start the read BEFORE the upstream fails so it is pending when the + // error arrives. The first read may resolve with the chunk that was + // rewritten before the failure; the stream must eventually reject. + let read = settle(reader.read()); release(); - expect(await read).toEqual(rejectedWithConnectionError); + let r = await read; + while (!r.rejected && !r.value.done) r = await settle(reader.read()); + expect(r).toEqual(rejectedWithConnectionError); }); }); @@ -226,13 +232,10 @@ describe("HTMLRewriter", () => { await withPartialBodyServer(async (url, release) => { const res = await fetch(url); const transformed = rewriter().transform(res); - const text = settle(transformed.text()); + const clone = transformed.clone(); release(); - // Barrier: the body is now in its error state. - expect(await text).toEqual(rejectedWithConnectionError); - // Cloning a failed body must produce a failed body, not an empty one - // that reads back as a complete (and empty) document. - expect(await settle(transformed.clone().text())).toEqual(rejectedWithConnectionError); + expect(await settle(transformed.text())).toEqual(rejectedWithConnectionError); + expect(await settle(clone.text())).toEqual(rejectedWithConnectionError); }); }); @@ -387,18 +390,22 @@ describe("HTMLRewriter", () => { expect(await text).toBe("

bye

"); }); - it("every promise-returning reader on the transformed response", async () => { - // `.body.getReader()` is covered by the `.todo` below: the ResumableSink - // pump delivers chunks from a microtask, so at the instant `transform()` - // returns the body is still `Locked`, which is the pre-existing #19305 - // output-side bug. Readers that return a promise await a turn first and - // are unaffected. + it("every way of reading the transformed response", async () => { const read = { text: response => response.text(), arrayBuffer: async response => new TextDecoder().decode(await response.arrayBuffer()), bytes: async response => new TextDecoder().decode(await response.bytes()), blob: response => response.blob().then(blob => blob.text()), json: response => response.json().then(value => JSON.stringify(value)), + getReader: async response => { + const reader = response.body.getReader(); + const parts = []; + for (let chunk = await reader.read(); !chunk.done; chunk = await reader.read()) { + parts.push(new TextDecoder().decode(chunk.value)); + } + return parts.join(""); + }, + readableStreamToText: response => Bun.readableStreamToText(response.body), }; const html = '

hi

there

'; @@ -545,12 +552,7 @@ describe("HTMLRewriter", () => { } }); - // Resolves with "" instead: the `.body` getter builds a ByteStream the - // producer is never told about, so done() closes it empty. Pre-existing and - // not specific to JS sources — a fetch body that is still mid-stream when - // transform() returns does the same thing on main (#19305, and #6068 for - // the Bun.serve shape, which hangs). Un-skip once the output side is fixed. - it.todo(".body of a transform whose source is still pending", async () => { + it(".body of a transform whose source is still pending", async () => { const { promise: gate, resolve: openGate } = Promise.withResolvers(); const body = new ReadableStream({ async start(controller) { From 154613c53a65f2374193f5393b5d247d5dd014a3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:16:27 +0000 Subject: [PATCH 07/13] handler_callback: drop the unbalanced gcProtect() on a captured exception handler_callback wrote the thrown exception into the HandlerErrorScope capture cell and then gcProtect()ed it; create_lolhtml_error reads that slot back and zeros it without unprotecting. The capture cell is a stack local on a conservatively-scanned frame that lives from the store through the read, so the protect provided no GC safety and leaked one Exception (and its Error) per transform on the throwing-handler path. Verified by the new leak test: 200 iterations pinned exactly 200 Error instances pre-fix. --- src/runtime/api/html_rewriter.rs | 19 +++++-------------- test/js/workerd/html-rewriter.test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 32d9ea922a4..ff3e37caed2 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -1144,36 +1144,27 @@ where ) { Ok(v) => v, Err(_) => { - // If there's an exception in the scope, capture it for later retrieval if let Some(exc) = scope.exception() { let exc_value = JSValue::from_cell(exc.as_ptr()); - // Store the exception in the VM's unhandled rejection capture - // mechanism if it's available (this is the same mechanism used - // by BufferOutputSink) + // `err_ptr` is a stack Cell owned by `HandlerErrorScope`'s + // caller; that frame is conservatively scanned until + // `create_lolhtml_error` reads it back, so no `protect()`. if let Some(err_ptr) = vm().unhandled_pending_rejection_to_capture { - // SAFETY: VM-owned pointer set by BufferOutputSink::init. + // SAFETY: VM-owned pointer set by `HandlerErrorScope`. unsafe { *err_ptr = exc_value }; - exc_value.protect(); } } - // Clear the exception from the scope to prevent assertion failures scope.clear_exception(); - // Return true to indicate failure to LOLHTML, which will cause the - // write operation to fail and the error handling logic to take over. return true; } }; - // Check if there's an exception that was thrown but not caught by the error union if let Some(exc) = scope.exception() { let exc_value = JSValue::from_cell(exc.as_ptr()); - // Store the exception in the VM's unhandled rejection capture mechanism if let Some(err_ptr) = vm().unhandled_pending_rejection_to_capture { - // SAFETY: VM-owned pointer set by BufferOutputSink::init. + // SAFETY: VM-owned pointer set by `HandlerErrorScope`. unsafe { *err_ptr = exc_value }; - exc_value.protect(); } - // Clear the exception to prevent assertion failures scope.clear_exception(); return true; } diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index d64d10f435f..12442b6d7a0 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -467,6 +467,32 @@ describe("HTMLRewriter", () => { await expect(transformed.text()).rejects.toThrow(TypeError); }); + it("does not leak a handler's thrown error", async () => { + const { heapStats } = require("bun:jsc"); + const once = async () => { + const rw = new HTMLRewriter().on("p", { + element() { + throw new Error("boom"); + }, + }); + await rw.transform(new Response(streamOf(encode("

x

")))).text().catch(() => {}); + }; + const settle = async () => { + for (let i = 0; i < 3; i++) { + Bun.gc(true); + await Bun.sleep(1); + } + }; + for (let i = 0; i < 10; i++) await once(); + await settle(); + const before = heapStats().objectTypeCounts.Error ?? 0; + for (let i = 0; i < 200; i++) await once(); + await settle(); + // Pre-fix: the Exception cell was gcProtect()ed in handler_callback and + // never unprotected, pinning one Error per transform (grew by ~400). + expect((heapStats().objectTypeCounts.Error ?? 0) - before).toBeLessThan(20); + }); + it("reusing the transformed response's source stream throws", async () => { const response = new Response(streamOf(encode("

hi

"))); expect(await rewriter().transform(response).text()).toBe("

bye

"); From e4ac595d046112afd8ddeb192e40d7d8f169ff37 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:47:09 +0000 Subject: [PATCH 08/13] ResumableSink: cancel the source once the context is done write_request_data returned WantMore unconditionally, so after fail() destroyed the rewriter the JS pump kept reading the source stream into a dead context until the source closed on its own. For a never-closing pull()-based source that means write_end_request never fires and the in-flight +1 (plus the sink's self-rooting js_this Strong, the reader and the source) stay alive forever after .text() has already rejected. js_write now treats ResumableSinkBackpressure::Done as a request to cancel the pump (cancel() releases the reader, fires on_end, and detaches the JS wrapper). write_request_data returns Done once the rewriter is gone; finish() early-returns if fail() already ran so the cancel-driven write_end_request is a no-op. FetchTasklet and S3 already return Done only after their own cancel()/abort paths have set status = Done, so the new js_write behaviour is idempotent for them. Covered by the new 'cancels the source stream once a handler throws' test, which hung forever pre-fix. Also moved the heapStats import to module scope per test/CLAUDE.md. --- src/runtime/api/html_rewriter.rs | 11 ++++++++++- src/runtime/webcore/ResumableSink.rs | 5 ++++- test/js/workerd/html-rewriter.test.js | 26 +++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index ff3e37caed2..1ebb43c1f97 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -560,7 +560,13 @@ impl ResumableSinkContext for BufferOutputSink { let captured = Cell::new(JSValue::ZERO); let _scope = HandlerErrorScope::enter(&self.global, &captured); self.feed(bytes); - ResumableSinkBackpressure::WantMore + if self.rewriter.get().is_null() { + // `fail()` destroyed the rewriter; stop the pump so the source + // is cancelled and `write_end_request` fires. + ResumableSinkBackpressure::Done + } else { + ResumableSinkBackpressure::WantMore + } } fn write_end_request(&mut self, err: Option) { @@ -805,6 +811,9 @@ impl BufferOutputSink { } fn finish(&self, err: Option) { + if self.failed.get().has() { + return; + } if let Some(err) = err { self.fail(err); return; diff --git a/src/runtime/webcore/ResumableSink.rs b/src/runtime/webcore/ResumableSink.rs index bd841828a25..ac60a7c350c 100644 --- a/src/runtime/webcore/ResumableSink.rs +++ b/src/runtime/webcore/ResumableSink.rs @@ -329,7 +329,10 @@ impl ResumableSink {} + ResumableSinkBackpressure::Done => { + this.cancel(JSValue::UNDEFINED); + return Ok(JSValue::FALSE); + } ResumableSinkBackpressure::WantMore => { this.status = Status::Started; } diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index 12442b6d7a0..09c47b917cc 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -1,3 +1,4 @@ +import { heapStats } from "bun:jsc"; import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { once } from "events"; import fs from "fs"; @@ -467,8 +468,31 @@ describe("HTMLRewriter", () => { await expect(transformed.text()).rejects.toThrow(TypeError); }); + it("cancels the source stream once a handler throws", async () => { + let pulls = 0; + let cancelled = false; + const body = new ReadableStream({ + pull(c) { + pulls++; + c.enqueue(encode("

x

")); + }, + cancel() { + cancelled = true; + }, + }); + const rw = new HTMLRewriter().on("p", { + element() { + throw new Error("boom"); + }, + }); + await expect(rw.transform(new Response(body)).text()).rejects.toThrow("boom"); + // The pump must stop instead of reading the never-closing source + // forever; a couple of extra pulls queued before cancel lands is fine. + expect(pulls).toBeLessThan(5); + expect(cancelled).toBe(true); + }); + it("does not leak a handler's thrown error", async () => { - const { heapStats } = require("bun:jsc"); const once = async () => { const rw = new HTMLRewriter().on("p", { element() { From f074428afec68478a58b08ba600e89b1e52fae4f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:17:08 +0000 Subject: [PATCH 09/13] Bun.write: read a stream-backed Locked body instead of waiting forever Bun.write(path, response) with a Response whose body was Locked with a readable (Locked { readable: Strong(stream) }) set on_receive_value and waited for a producer to call Value::resolve(). For bodies that are streams from the start (new Response(readableStream), and now HTMLRewriter.transform()'s ByteStream output), nothing ever fires that callback and the write hangs forever. Reproducible on main with Bun.write(path, new Response(new ReadableStream(...))). When the Locked body already has a readable (cached on the JS wrapper or in locked.readable), read it via readableStreamToBytes and feed the resolved bytes to WriteFileWaitFromLockedValueTask::then as an InternalBlob. A rejected stream reaches then() as Value::Error. Fixes the 'Bun.write(output.html, HTMLRewriter.transform(Bun.file))' test that regressed when HTMLRewriter's output body became a ByteStream. --- src/jsc/bindings/ZigGlobalObject.cpp | 4 ++ src/jsc/bindings/ZigGlobalObject.h | 4 +- src/jsc/bindings/headers.h | 2 + src/runtime/webcore/Blob.rs | 19 ++++++++++ src/runtime/webcore/blob/write_file.rs | 52 +++++++++++++++++++++++++- 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 1e1fe0a1ca1..9e3a4298596 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4040,6 +4040,10 @@ GlobalObject::PromiseFunctions GlobalObject::promiseHandlerID(Zig::FFIFunction h return GlobalObject::PromiseFunctions::Bun__TestScope__Describe2__bunTestThen; } else if (handler == Bun__TestScope__Describe2__bunTestCatch) { return GlobalObject::PromiseFunctions::Bun__TestScope__Describe2__bunTestCatch; + } else if (handler == Bun__WriteFileLocked__onStreamResolved) { + return GlobalObject::PromiseFunctions::Bun__WriteFileLocked__onStreamResolved; + } else if (handler == Bun__WriteFileLocked__onStreamRejected) { + return GlobalObject::PromiseFunctions::Bun__WriteFileLocked__onStreamRejected; } else if (handler == Bun__onResolveEntryPointResult) { return GlobalObject::PromiseFunctions::Bun__onResolveEntryPointResult; } else if (handler == Bun__onRejectEntryPointResult) { diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index ab79b3a1780..d4dc19ba41a 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -392,6 +392,8 @@ class GlobalObject : public Bun::GlobalScope { jsFunctionOnLoadObjectResultReject, Bun__TestScope__Describe2__bunTestThen, Bun__TestScope__Describe2__bunTestCatch, + Bun__WriteFileLocked__onStreamResolved, + Bun__WriteFileLocked__onStreamRejected, Bun__onResolveEntryPointResult, Bun__onRejectEntryPointResult, Bun__NodeHTTPRequest__onResolve, @@ -411,7 +413,7 @@ class GlobalObject : public Bun::GlobalScope { Bun__HTTPRequestContextDebugH3__onResolve, Bun__HTTPRequestContextDebugH3__onResolveStream, }; - static constexpr size_t promiseFunctionsSize = 40; + static constexpr size_t promiseFunctionsSize = 42; static PromiseFunctions promiseHandlerID(SYSV_ABI EncodedJSValue (*handler)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1)); diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index d0a11b5ebfe..b4b78b5a78c 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -776,6 +776,8 @@ BUN_DECLARE_HOST_FUNCTION(Bun__HTTPRequestContextDebugTLS__onResolveStream); BUN_DECLARE_HOST_FUNCTION(Bun__TestScope__Describe2__bunTestThen); BUN_DECLARE_HOST_FUNCTION(Bun__TestScope__Describe2__bunTestCatch); +BUN_DECLARE_HOST_FUNCTION(Bun__WriteFileLocked__onStreamResolved); +BUN_DECLARE_HOST_FUNCTION(Bun__WriteFileLocked__onStreamRejected); BUN_DECLARE_HOST_FUNCTION(Bun__CronJob__onPromiseResolve); BUN_DECLARE_HOST_FUNCTION(Bun__CronJob__onPromiseReject); diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index ec3d76a5564..b04a290d94c 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -5244,6 +5244,25 @@ pub fn write_file_internal( let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { unreachable!() }; + // A body backed by a ReadableStream has no producer to + // fire `on_receive_value`; read the stream ourselves. + if let Some(readable) = + get_stream(global_this).or_else(|| locked.readable.get(global_this)) + { + let bytes_promise = + global_this.readable_stream_to_bytes(readable.value); + // SAFETY: re-borrow after `readable_stream_to_bytes`. + *(unsafe { &mut *body_value }) = BodyValue::Used; + bytes_promise.then( + global_this, + task, + write_file_mod::write_file_locked_on_stream_resolved_shim, + write_file_mod::write_file_locked_on_stream_rejected_shim, + ); + // SAFETY: `task` heap-allocated above; consumed by + // the `then` reactions. + return Ok(ControlFlow::Break(unsafe { (*task).promise.value() })); + } locked.task = Some(task.cast::()); locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 1f6795af372..01ca6d2276b 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -9,7 +9,7 @@ use bun_core::ZigString; use bun_io::{self as io, IntrusiveIoRequest as _}; use bun_jsc::ZigStringJsc as _; use bun_jsc::node_path::PathOrFileDescriptor; -use bun_jsc::{self as jsc, JSGlobalObject, JSPromise, JSValue, JsTerminated, SystemError}; +use bun_jsc::{self as jsc, CallFrame, JSGlobalObject, JSPromise, JSValue, JsTerminated, SystemError}; use bun_sys::{self as sys, Fd}; use bun_threading::{IntrusiveWorkTask as _, WorkPool, WorkPoolTask}; @@ -1326,6 +1326,33 @@ impl WriteFileWaitFromLockedValueTask { // TODO: properly propagate exception upwards } + /// `.then` reaction for a `Locked` body that already has a readable: the + /// body was read via `readableStreamToBytes`; wrap the resolved bytes as + /// an `InternalBlob` and hand them to [`then`](Self::then). + fn on_stream_resolved(global: &JSGlobalObject, callframe: &jsc::CallFrame) -> JSValue { + let args = callframe.arguments_old::<2>(); + let this = args.ptr[args.len - 1].as_promise_ptr::(); + let mut value = match args.ptr[0].as_array_buffer(global) { + Some(buf) => body::Value::InternalBlob(body::InternalBlob { + bytes: buf.slice().to_vec(), + was_string: false, + }), + None => body::Value::Empty, + }; + let _ = Self::then(NonNull::new(this).unwrap(), &mut value); + JSValue::UNDEFINED + } + + fn on_stream_rejected(global: &JSGlobalObject, callframe: &jsc::CallFrame) -> JSValue { + let args = callframe.arguments_old::<2>(); + let this = args.ptr[args.len - 1].as_promise_ptr::(); + let mut value = body::Value::Error(body::ValueError::JSValue( + jsc::strong::Optional::create(args.ptr[0], global), + )); + let _ = Self::then(NonNull::new(this).unwrap(), &mut value); + JSValue::UNDEFINED + } + /// # Safety /// `this` must point to a live Box-allocated `WriteFileWaitFromLockedValueTask`. /// On every arm except `body::Value::Locked`, the allocation is consumed. @@ -1427,3 +1454,26 @@ impl WriteFileWaitFromLockedValueTask { Ok(()) } } + +bun_jsc::jsc_host_abi! { + #[unsafe(export_name = "Bun__WriteFileLocked__onStreamResolved")] + pub(crate) unsafe fn write_file_locked_on_stream_resolved_shim( + global: *mut JSGlobalObject, + callframe: *mut CallFrame, + ) -> JSValue { + let (global, callframe) = + (bun_opaque::opaque_deref(global), bun_opaque::opaque_deref(callframe)); + WriteFileWaitFromLockedValueTask::on_stream_resolved(global, callframe) + } +} +bun_jsc::jsc_host_abi! { + #[unsafe(export_name = "Bun__WriteFileLocked__onStreamRejected")] + pub(crate) unsafe fn write_file_locked_on_stream_rejected_shim( + global: *mut JSGlobalObject, + callframe: *mut CallFrame, + ) -> JSValue { + let (global, callframe) = + (bun_opaque::opaque_deref(global), bun_opaque::opaque_deref(callframe)); + WriteFileWaitFromLockedValueTask::on_stream_rejected(global, callframe) + } +} From 277698392a348fba240bca019701be86a45856ba Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:03:48 +0000 Subject: [PATCH 10/13] Bun.write: handle a non-promise / throwing readableStreamToBytes result readable_stream_to_bytes enters JS and can return JSValue::ZERO with a pending exception (or a non-promise value). The previous revision called .then() on the result unconditionally, which in debug asserts on .asCell() and in release silently no-ops, leaking the heap-allocated WriteFileWaitFromLockedValueTask (its JSPromiseStrong root and file_blob) with a promise that never settles. Wrap the call in from_js_host_call and, if the result is not a promise, reject the task's promise with the exception (or the returned value) and free the task. --- src/runtime/webcore/Blob.rs | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index b04a290d94c..55c35516b16 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -5249,19 +5249,32 @@ pub fn write_file_internal( if let Some(readable) = get_stream(global_this).or_else(|| locked.readable.get(global_this)) { - let bytes_promise = - global_this.readable_stream_to_bytes(readable.value); + let bytes_promise = bun_jsc::from_js_host_call(global_this, || { + global_this.readable_stream_to_bytes(readable.value) + }); // SAFETY: re-borrow after `readable_stream_to_bytes`. *(unsafe { &mut *body_value }) = BodyValue::Used; - bytes_promise.then( - global_this, - task, - write_file_mod::write_file_locked_on_stream_resolved_shim, - write_file_mod::write_file_locked_on_stream_rejected_shim, - ); - // SAFETY: `task` heap-allocated above; consumed by - // the `then` reactions. - return Ok(ControlFlow::Break(unsafe { (*task).promise.value() })); + // SAFETY: `task` heap-allocated above; sole owner. + let promise = unsafe { (*task).promise.value() }; + match bytes_promise { + Ok(p) if p.as_any_promise().is_some() => p.then( + global_this, + task, + write_file_mod::write_file_locked_on_stream_resolved_shim, + write_file_mod::write_file_locked_on_stream_rejected_shim, + ), + other => { + // SAFETY: `task` heap-allocated above; sole owner. + let task = unsafe { bun_core::heap::take(task) }; + task.file_blob.detach(); + let err = match other { + Err(err) => global_this.take_exception(err), + Ok(v) => v, + }; + task.promise.get().reject(global_this, Ok(err))?; + } + } + return Ok(ControlFlow::Break(promise)); } locked.task = Some(task.cast::()); locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); From c08a95eaf25874e768d48bbfb01d36f90132ed40 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:14:52 +0000 Subject: [PATCH 11/13] rebase: adapt WriteFileLocked stream reactions to CallFrame::argument(n) arguments_old::() was removed on main. --- src/runtime/webcore/blob/write_file.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 01ca6d2276b..bdcb73c6d53 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -1330,9 +1330,8 @@ impl WriteFileWaitFromLockedValueTask { /// body was read via `readableStreamToBytes`; wrap the resolved bytes as /// an `InternalBlob` and hand them to [`then`](Self::then). fn on_stream_resolved(global: &JSGlobalObject, callframe: &jsc::CallFrame) -> JSValue { - let args = callframe.arguments_old::<2>(); - let this = args.ptr[args.len - 1].as_promise_ptr::(); - let mut value = match args.ptr[0].as_array_buffer(global) { + let this = callframe.argument(1).as_promise_ptr::(); + let mut value = match callframe.argument(0).as_array_buffer(global) { Some(buf) => body::Value::InternalBlob(body::InternalBlob { bytes: buf.slice().to_vec(), was_string: false, @@ -1344,10 +1343,9 @@ impl WriteFileWaitFromLockedValueTask { } fn on_stream_rejected(global: &JSGlobalObject, callframe: &jsc::CallFrame) -> JSValue { - let args = callframe.arguments_old::<2>(); - let this = args.ptr[args.len - 1].as_promise_ptr::(); + let this = callframe.argument(1).as_promise_ptr::(); let mut value = body::Value::Error(body::ValueError::JSValue( - jsc::strong::Optional::create(args.ptr[0], global), + jsc::strong::Optional::create(callframe.argument(0), global), )); let _ = Self::then(NonNull::new(this).unwrap(), &mut value); JSValue::UNDEFINED From 293173d6baaa830536b49503b76c0c7b2ad8e248 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:17:06 +0000 Subject: [PATCH 12/13] [autofix.ci] apply automated fixes --- src/runtime/api/html_rewriter.rs | 3 ++- src/runtime/webcore/blob/write_file.rs | 4 +++- test/js/workerd/html-rewriter.test.js | 5 ++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/html_rewriter.rs b/src/runtime/api/html_rewriter.rs index 1ebb43c1f97..c9cf5779c30 100644 --- a/src/runtime/api/html_rewriter.rs +++ b/src/runtime/api/html_rewriter.rs @@ -791,7 +791,8 @@ impl BufferOutputSink { if !matches!(value, webcore::body::Value::Error(_)) { *value = webcore::body::Value::Used; } - let _ = ResumableHTMLRewriterSink::init(&global, readable_stream, core::ptr::from_mut(self)); + let _ = + ResumableHTMLRewriterSink::init(&global, readable_stream, core::ptr::from_mut(self)); Ok(()) } diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index bdcb73c6d53..3ff2b75cad1 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -9,7 +9,9 @@ use bun_core::ZigString; use bun_io::{self as io, IntrusiveIoRequest as _}; use bun_jsc::ZigStringJsc as _; use bun_jsc::node_path::PathOrFileDescriptor; -use bun_jsc::{self as jsc, CallFrame, JSGlobalObject, JSPromise, JSValue, JsTerminated, SystemError}; +use bun_jsc::{ + self as jsc, CallFrame, JSGlobalObject, JSPromise, JSValue, JsTerminated, SystemError, +}; use bun_sys::{self as sys, Fd}; use bun_threading::{IntrusiveWorkTask as _, WorkPool, WorkPoolTask}; diff --git a/test/js/workerd/html-rewriter.test.js b/test/js/workerd/html-rewriter.test.js index 09c47b917cc..fbad2fe7653 100644 --- a/test/js/workerd/html-rewriter.test.js +++ b/test/js/workerd/html-rewriter.test.js @@ -499,7 +499,10 @@ describe("HTMLRewriter", () => { throw new Error("boom"); }, }); - await rw.transform(new Response(streamOf(encode("

x

")))).text().catch(() => {}); + await rw + .transform(new Response(streamOf(encode("

x

")))) + .text() + .catch(() => {}); }; const settle = async () => { for (let i = 0; i < 3; i++) { From 7353853d255170c1acfab3019faf4eae058c7139 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:40:23 +0000 Subject: [PATCH 13/13] ci: retrigger gate (release build infra failure; local release 92/92 pass)