fix(resolver): skip EACCES/EPERM on ancestor directories in dir_info … - #6
Merged
springmin merged 1 commit intoJun 8, 2026
Conversation
…walk On OHOS, opening ancestor directories like '/' or '/storage/' returns EACCES due to sandbox restrictions, even when child directories are fully accessible. This caused dir_info_cached_miss to return Ok(None) immediately upon encountering the first EACCES in the bottom-up walk, making all read_dir_info calls fail - including for the cwd and $HOME fallback. The downstream InstallFailed error was then silently swallowed by handle_root_error, producing exit(1) with zero stderr output. Fix: when open_dir_absolute_z returns EACCES/EPERM for an ancestor, continue 'queue_walk to skip that entry and proceed to process the remaining child directories. This allows dir_info to be built from the deepest accessible ancestor upward, which is sufficient for package.json resolution, module resolution, and the bun <dir> / bun . entry point. The previous code already treated EACCES/EPERM as non-loggable (suppressed the 'Cannot read directory' message) but still returned Ok(None), making the error fatal. Now it is also non-fatal for the ancestor-walk case. Verified: bun . now resolves package.json main field and executes the entry point on OHOS, producing correct stdout and exit 0.
springmin
pushed a commit
that referenced
this pull request
Jun 18, 2026
…letes mid-read (oven-sh#31959) [publish images] Fixes a use-after-free in the HTTP client's proxy tunnel close path (Sentry BUN-2VY8, ~10 events/day on Windows release builds; reproduces deterministically under ASAN on all platforms). ## Repro `fetch()` through an HTTP CONNECT proxy to an HTTPS origin, where the origin's final response bytes and its TLS `close_notify` reach the client in a single TCP batch (origin writes the response and immediately closes). The regression test builds exactly that: a local CONNECT proxy that holds origin-to-client bytes after the handshake and flushes session tickets + response + close_notify in one write. On an unfixed ASAN build: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8 thread T11 (HTTP Client) #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 bun_http::proxy_tunnel::on_close src/http/ProxyTunnel.rs:525 #2 SSLWrapper<*mut HTTPClient>::trigger_close_callback src/uws/lib.rs:802 #3 SSLWrapper<*mut HTTPClient>::handle_reading src/uws/lib.rs:1022 #4 SSLWrapper<*mut HTTPClient>::handle_traffic #5 SSLWrapper<*mut HTTPClient>::receive_data #6 ProxyTunnel::receive src/http/ProxyTunnel.rs:751 freed by: AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:813 HTTPClient::send_progress_update_without_stage_check src/http/lib.rs:3793 ``` ## Cause 1. `handle_reading` processes the batch: `SSL_read` returns the body bytes, the next `SSL_read` hits `close_notify` (`SSL_ERROR_ZERO_RETURN`), which sets `received_ssl_shutdown` and `sent_ssl_shutdown` before flushing the already-decrypted bytes through the data callback. 2. The data callback completes the response. The done path runs `close_proxy_tunnel(true)` -> `ProxyTunnel::shutdown()` -> `SSLWrapper::shutdown(true)`, which hits the already-shut-down early return (`sent_ssl_shutdown || fatal_error`) and returns **without setting `closed_notified`**. The result callback then frees the `ThreadlocalAsyncHTTP` embedding the `HTTPClient`, the exact pointer stored in the wrapper's `handlers.ctx`. 3. Control returns to `handle_reading`. Its liveness guard (`ssl.is_none() || closed_notified()`) passes because neither is set, so `trigger_close_callback()` invokes `on_close(handlers.ctx)` on the freed client. When the allocation has been recycled, `on_close` can ref or close a different request's tunnel instead of faulting. ## Fix `src/uws/lib.rs`: when `SSLWrapper::shutdown(fast_shutdown=true)` takes the already-shut-down early return, fire `trigger_close_callback()` (idempotent via `closed_notified`) so the wrapper is marked closed before the owner detaches and frees `handlers.ctx`. A fast shutdown is a full teardown, and the normal fast-shutdown path already fires the close callback unconditionally; this only closes the gap where the SSL-level shutdown had already happened. Graceful `shutdown(false)` (node:tls half-close via UpgradedDuplex / WindowsNamedPipe) is unchanged, so reads after a sent `close_notify` keep working. ## Verification New test in `test/js/bun/http/proxy.test.ts` (`test.skipIf(!isASAN)`, the UAF is only deterministic under ASAN): fails on an unfixed ASAN debug build with the heap-use-after-free above, passes with the fix. Full `proxy.test.ts` (46 tests) plus `node-tls-connect`, `node-tls-upgrade`, `node-tls-duplex-close-throw-uaf`, `node-tls-socket-allow-half-open-option`, `node-tls-server`, `fetch-tls-cert`, and `node-https-checkServerIdentity` suites pass. ## Note on the asan-lane CI failure (oven-sh#32144) The intermittent LeakSanitizer failure on the x64-asan shard (deferred napi finalizers parked on a never-drained cleanup-hook list at `bun test` exit) is being fixed in oven-sh#32146, which carries the same `global_exit()` drain plus a hooks-only guard that skips pending `napi_wrap` finalizers on undrained-loop exits. A subset version of that fix was briefly on this branch (e59bc1d) but without the hooks-only guard it made `test/js/third_party/duckdb/duckdb-basic-usage.test.ts` SEGV at exit on the asan lane (build 62135), exactly the failure mode oven-sh#32146's guard prevents, so it was reverted (61f9e70). This PR is scoped to the proxy-tunnel UAF; its asan lane can still intermittently hit the pre-existing oven-sh#32144 leak until oven-sh#32146 lands. ## Related PRs - oven-sh#30606 addresses the same crash signature but patches only the `.zig` reference files, which are no longer compiled; this PR fixes the shipping Rust implementation. - oven-sh#31952 fixes the same UAF by calling a new `mark_close_notified()` helper from `ProxyTunnel::shutdown` (silently setting the flag at one call site, with `close_raw` exempted). This PR instead closes the gap inside `SSLWrapper::shutdown(true)` itself, so every fast-shutdown caller (`ProxyTunnel::shutdown`, `ProxyTunnel::close_raw`, `UpgradedDuplex::close`, `WebSocketProxyTunnel::shutdown`) gets the same "no callbacks after teardown" guarantee without new wrapper API or a shutdown/close_raw asymmetry. The close callback is fired rather than suppressed, so the error teardown path keeps delivering `on_close` -> `close_and_fail` exactly once (idempotent via `closed_notified`). Test here is a deterministic single-shot repro (the test proxy reassembles TLS records and flushes tickets + response + close_notify in one write) rather than an iteration loop. --------- Co-authored-by: Ciro Spaciari MacBook <ciro@anthropic.com>
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…sweep (oven-sh#32729) ### Crash ``` ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping vendor/WebKit/Source/JavaScriptCore/runtime/JSCell.cpp(179) : bool JSC::JSCell::validateIsNotSweeping() const ``` Backtrace (from a release-asan build with asserts): ``` #3 JSC::JSCell::validateIsNotSweeping() #4 JSC::JSCell::classInfo() const #5 WTF::uncheckedDowncast<WebCore::JSResumableFetchSink>(JSValue const&) #6 ResumableFetchSinkPrototype__ondrainSetCachedValue #7 bun_runtime::webcore::fetch::fetch_tasklet::FetchTasklet::ignore_remaining_response_body #8 JSC::WeakBlock::sweep() <- inside GC sweep (Weak finalizer) #9 JSC::WeakSet::sweep() #10 JSC::PreciseAllocation::sweep() #12 JSC::Heap::finalize() oven-sh#21 JSC::LocalAllocator::allocateSlowCase oven-sh#23 JSC::ErrorInstance::create <- ordinary allocation kicked off GC ``` Found by the syscall fault-injection fuzzer's client-side grammar scenario (fetch/node:http with abort + transient errno on the client socket). Reproduces ~4/5 under `BUN_JSC_collectContinuously=1`. ### Cause `FetchTasklet::on_response_finalize` is the `WeakRefOwner<FetchResponse>::finalize` callback and runs inside `WeakBlock::sweep` while `MutatorState == Sweeping`. When the response body is `Locked` without a pending promise or stream it calls `ignore_remaining_response_body()`, which called: - `ResumableSink::detach_js()`: writes the sink wrapper's cached `ondrain` / `oncancel` / `stream` slots via the generated `ResumableFetchSinkPrototype__*SetCachedValue` helpers. Each does `uncheckedDowncast<JSResumableFetchSink>(thisValue)`, which reaches `JSCell::classInfo()` and then issues a write barrier on the wrapper cell. - `clear_stream_handlers()`: reaches `ReadableStreamTag__tagged` -> `object->inherits<JSReadableStream>()` (guarded today, but one boolean away). Calling `classInfo()` on any cell while the mutator is sweeping is forbidden: the cell's `Structure` may already have been swept. Assert builds catch it; release builds corrupt the heap. ### Fix Thread a `from_finalizer` flag through `ignore_remaining_response_body`. When `true` (the `on_response_finalize` caller) skip `detach_js()` and `clear_stream_handlers()`; only native state is touched. The sink's JS-side detach still happens from `clear_sink()` in `FetchTasklet::deinit()`, which runs as an event-loop `ConcurrentTask` outside any sweep, so nothing leaks. The `on_stream_cancelled_callback` caller (reader `.cancel()`, runs from JS on the event loop) passes `false` and keeps the immediate detach. Also corrects the `ResumableSink::detach_js` doc comment that claimed finalizer safety. ### Verification New test at `test/js/web/fetch/fetch-response-finalizer-sweep.test.ts`: a child process under `BUN_JSC_collectContinuously=1` does 12 iterations of `fetch()` with a user-constructed `ReadableStream` body (so the sink takes the JS route with a Strong `js_this`) against a raw TCP server that sends headers + a partial chunked body and never terminates it, then drops the `Response` unconsumed and runs `Bun.gc(true)`. Without the fix (`bun bd`, src/ stashed): ``` exitCode: 134 stderr: ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping ``` With the fix: `stdout: "ok"`, `exitCode: 0`. `test/js/web/fetch/fetch-backpressure.test.ts` (exercises the `on_stream_cancelled_callback` path) passes unchanged. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…type (oven-sh#32738) ### Repro ```js using listener = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: { data() {} } }); await Bun.connect({ hostname: "127.0.0.1", port: listener.port, socket: { open(s) { s.setTypeOfService({}); } }, }); ``` On any assert build (debug, release-asan): ``` ASSERTION FAILED: isInt32() #4 JSC__JSValue__toInt32 #5 TCPSocketPrototype__setTypeOfService #6 WebCore::TCPSocketPrototype__setTypeOfServiceCallback ``` On plain release the assert compiles out and the NaN-boxed bits of the object handle are passed to `setsockopt(IP_TOS)` as the TOS byte. ### Cause `set_type_of_service` in `src/runtime/socket/socket_body.rs` called `args.ptr[0].to_int32()` on the raw argument. For non-numeric values that falls through to the C++ `JSC__JSValue__toInt32`, which is `JSC::JSValue::asInt32()` (the unchecked accessor that asserts `isInt32()`), not a coercing conversion. The `node:net` wrapper validates `tos` in JS before calling the handle, but the Bun-native `Bun.connect` socket exposes this prototype method directly with no JS validation layer. ### Fix Route the argument through `validate_integer_range` with `min: 0, max: 255, field_name: "tos"`, the same pattern the sibling `setKeepAlive` already uses for `initialDelay`. Non-numbers now throw `ERR_INVALID_ARG_TYPE`, out-of-range integers throw `ERR_OUT_OF_RANGE`, and non-integral numbers throw `ERR_INVALID_ARG_TYPE`, matching the `node:net` surface. I audited the other numeric setters on the TCPSocket/TLSSocket prototype (`timeout`, `setMaxSendFragment`, `write` offset/length) and the remaining `to_int32()` / `to_int64()` callers in `src/runtime/socket/`: each is already gated by `is_number()` / `is_any_int()` or routes through `coerce`. `setTypeOfService` was the only unguarded one. ### Verification New test in `test/js/bun/net/socket.test.ts` spawns a subprocess that calls `setTypeOfService` on a connected `Bun.connect` socket with `{}`, `"x"`, `-1`, `256`, `1.5`, and `0x10`, and asserts the error code for each plus that `getTypeOfService()` returns an integer. Without the fix the subprocess aborts (exit 134) on the first call; with the fix all seven checks pass. `test/js/node/test/parallel/test-net-socket-tos.js` continues to pass. Found by the bun-sys-fuzz API-grammar layer.
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ed (oven-sh#33016) A backend message that fails the connection can share a TCP read with messages that follow it. `PostgresRequest::on_data`'s message loop had no bail-out once `fail()` had run, so the trailing messages in that read kept being dispatched against the already-failed connection. ### Repro A mock backend that answers the StartupMessage with one write carrying two messages: ``` R int32(8) int32(99) Authentication, unrecognized type Z int32(5) 'I' ReadyForQuery ``` ```ts const sql = new SQL({ url: `postgres://u@127.0.0.1:${port}/db`, max: 1, idleTimeout: 1, connectionTimeout: 5 }); await sql`select 1`.catch(() => {}); await Bun.sleep(1600); ``` ### Cause The unrecognized `Authentication` type calls `fail()`, which sets the status to `Failed`, closes the socket, and rejects the pending requests, but the message loop keeps going and dispatches the `ReadyForQuery` from the same read. That calls `set_status(Status::Connected)`, which has no guard against leaving `Failed`, so the dead connection is flipped back to `Connected` and the `on_data` epilogue re-arms its idle timer. uSockets frees a closed `us_socket_t` at the end of the event-loop iteration, so when the timer later fires, `ref_and_close` reads the freed socket: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 at 0x71f2125605d2 thread T0 #0 us_socket_is_closed packages/bun-usockets/src/socket.c:143:21 #4 PostgresSQLConnection::ref_and_close src/sql_jsc/postgres/PostgresSQLConnection.rs:1528:31 #5 PostgresSQLConnection::fail_with_js_value src/sql_jsc/postgres/PostgresSQLConnection.rs:726:14 #6 PostgresSQLConnection::fail_fmt src/sql_jsc/postgres/PostgresSQLConnection.rs:749:14 #7 PostgresSQLConnection::on_connection_timeout src/sql_jsc/postgres/PostgresSQLConnection.rs:557:14 #8 __bun_fire_timer src/runtime/dispatch.rs:1020:35 0x71f2125605d2 is located 18 bytes inside of 104-byte region freed by thread T0 here: #2 us_internal_free_closed_sockets packages/bun-usockets/src/loop.c:305:9 ``` ### Fix - `PostgresRequest::on_data`: the message loop returns once the connection's status is `Failed`. `fail()` is terminal; nothing after it in the same read should be handled (a `DataRow`, `CommandComplete`, or `ErrorResponse` in that position would be just as wrong as the `ReadyForQuery`). - `PostgresSQLConnection::set_status`: refuses to transition out of `Failed`. The transition function owns that invariant; every other consumer of `Status` (the timer interval, `update_has_pending_activity`, the idempotency check in `fail_with_js_value`) already assumes `Failed` is terminal. ### Verification `test/js/sql/postgres-failed-connection-resurrection.test.ts` runs a fixture against the mock backend above and lets it outlive the idle-timer window. Without the fix the fixture dies with the ASan report above; with it the fixture exits 0. Gated to ASan builds because the bug is a read of freed memory, which release lanes do not detect. The postgres fault-injection and integration suites still pass locally (90 tests across `test/js/sql/postgres-*.test.ts`, `sql*.test.ts`, `tls-sql.test.ts`). ### Related - oven-sh#32861 detaches the stored socket handle in `on_close` / `on_connect_error` so nothing can dereference the freed `us_socket_t` regardless of how the stale read is reached. It removes the last step of this chain from the other end; this PR stops the failed connection from being resurrected at all. - oven-sh#30950 guards the JS pool's `handleConnected` against the reverse ordering within one read (a legitimately queued `onconnect` microtask arriving after a synchronous `onclose`).
springmin
pushed a commit
that referenced
this pull request
Jul 2, 2026
…h#33186) ### Repro ```sh printf '{"name":"x","version":"1.0.0"}' > package.json bun pm pkg set 'contributors[0]=alice' ``` On a release build (1.4.0 and current `main`) this exits 0 and writes freed heap bytes into `package.json` as the property key: ```json { "name": "x", "version": "1.0.0", "P\x01\x00\x00\x00tors": { "\x00": "alice" } } ``` Depending on what was in the freed allocation the result is often not valid JSON at all. Any `bun pm pkg set` key path containing `[index]` hits it. Under ASAN it is a deterministic `heap-use-after-free`: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 #0 bun_js_printer::write_pre_quoted_string_inner src/js_printer/lib.rs:1014 #7 PmPkgCommand::save_package_json src/runtime/cli/pm_pkg_command.rs:909 freed by thread T0 here: #7 <Box<[u8]> as Drop>::drop #12 PmPkgCommand::set_value src/runtime/cli/pm_pkg_command.rs:661 previously allocated by thread T0 here: #10 <Box<[u8]> as From<&[u8]>>::from #11 PmPkgCommand::parse_key_path src/runtime/cli/pm_pkg_command.rs:583 ``` <details> <summary>full ASAN report</summary> ``` ================================================================= ==16563==ERROR: AddressSanitizer: heap-use-after-free on address 0x73423c7c0670 at pc 0x00000f583cc5 bp 0x7fff2667e950 sp 0x7fff2667e948 READ of size 1 at 0x73423c7c0670 thread T0 #0 0x00000f583cc4 in _RINvCs59Hqei94dXF_14bun_js_printer29write_pre_quoted_string_innerINtB2_16StdWriterAdapterQINtB2_6WriterNtB2_12BufferWriterEEKVNtNtB2_8Encoding4Utf8UECsgBGN0jRPILJ_11bun_bundler /workspace/bun/src/js_printer/lib.rs:1014:79 #1 0x00000ebda439 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_utf8 /workspace/bun/src/js_printer/lib.rs:2641:21 #2 0x00000ebdb7a7 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_e_string /workspace/bun/src/js_printer/lib.rs:4546:22 #3 0x00000ebdb238 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_literal_e_string /workspace/bun/src/js_printer/lib.rs:3018:18 #4 0x00000ebd2be2 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_property /workspace/bun/src/js_printer/lib.rs:4807:34 #5 0x00000ebc0166 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_expr /workspace/bun/src/js_printer/lib.rs:3962:38 #6 0x00000ee574c1 in bun_js_printer::print_json::<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>> /workspace/bun/src/js_printer/lib.rs:8071:13 #7 0x00000c05c270 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::save_package_json /workspace/bun/src/runtime/cli/pm_pkg_command.rs:909:25 #8 0x00000c060cfb in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:330:13 #9 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #10 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 #11 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 #12 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 #13 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 #14 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 #15 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 oven-sh#16 0x77223ccc7d64 in __libc_start_main csu/../csu/libc-start.c:360:3 oven-sh#17 0x0000099d1d1d in __wrap___libc_start_main /workspace/bun/build/debug/../../src/jsc/bindings/workaround-missing-symbols.cpp:487:12 0x73423c7c0670 is located 0 bytes inside of 12-byte region [0x73423c7c0670,0x73423c7c067c) freed by thread T0 here: #0 0x000007ae192a in free crtstuff.c #1 0x00000bb3c5a7 in <std::alloc::System as core::alloc::global::GlobalAlloc>::dealloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:48:18 #2 0x00000bb3be9a in __rustc::__rust_dealloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b05f in alloc::alloc::dealloc_nonnull /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:128:14 #4 0x0000125872fe in <alloc::alloc::Global>::deallocate_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:229:22 #5 0x000012586364 in <alloc::alloc::Global>::deallocate_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:344:9 #6 0x00001258d79c in <alloc::alloc::Global as core::alloc::Allocator>::deallocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:462:23 #7 0x000012582946 in <alloc::boxed::Box<[u8]> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1956:24 #8 0x000012572e44 in core::ptr::drop_in_place::<alloc::boxed::Box<[u8]>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #9 0x000011f8d429 in core::ptr::drop_in_place::<[alloc::boxed::Box<[u8]>]> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #10 0x00000ef6b73a in <alloc::vec::Vec<alloc::boxed::Box<[u8]>> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:4258:13 #11 0x00000ef69e64 in core::ptr::drop_in_place::<alloc::vec::Vec<alloc::boxed::Box<[u8]>>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #12 0x00000c061846 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:661:5 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #15 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 oven-sh#16 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 oven-sh#17 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 oven-sh#18 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 oven-sh#19 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 oven-sh#20 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 previously allocated by thread T0 here: #0 0x000007ae1bc8 in malloc crtstuff.c #1 0x00000bb3c520 in <std::alloc::System as core::alloc::global::GlobalAlloc>::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:14:22 #2 0x00000bb3be30 in __rustc::__rust_alloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b335 in alloc::alloc::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:101:9 #4 0x000012586b81 in <alloc::alloc::Global>::alloc_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:210:73 #5 0x0000125862b6 in <alloc::alloc::Global>::alloc_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:332:9 #6 0x00001258d86a in <alloc::alloc::Global as core::alloc::Allocator>::allocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:449:14 #7 0x00001257dbd3 in <alloc::boxed::Box<[u8]>>::try_clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:881:29 #8 0x00001257da49 in <alloc::boxed::Box<[u8]>>::clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:840:15 #9 0x00001257d3f4 in <alloc::boxed::Box<[u8]>>::clone_from_ref /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:793:9 #10 0x000012581e34 in <alloc::boxed::Box<[u8]> as core::convert::From<&[u8]>>::from /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed/convert.rs:77:9 #11 0x00000c05a47f in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::parse_key_path /workspace/bun/src/runtime/cli/pm_pkg_command.rs:583:37 #12 0x00000c061608 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:643:30 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 ``` </details> ### Cause `parse_key_path` returned a `Vec<Box<[u8]>>`, and `set_value` / `set_nested` inserted those boxed segments into the manifest AST by reference: `E::Object::put` constructs `EString::init(key)`, whose documented contract is that `key` is arena-owned (it records the slice, it does not copy it). The vector is a local of `set_value`, so it dropped before `exec_set` reached `save_package_json`, and the JSON printer then read the dangling keys. The non-bracket path in `set_value` did not have the bug: it borrowed its segments straight out of the argv key, which outlives the whole command. The bracket path differed only by the unnecessary boxing. ### Fix `parse_key_path` now returns `Vec<&[u8]>`. Every segment is a literal sub-slice of the input key, so nothing ever needed owning. With the boxing gone, `set_value`'s separate non-bracket branch and its `set_nested_simple` helper (which existed only to avoid the allocation) were exact duplicates of the bracket path, so they are deleted and all keys route through `parse_key_path` + `set_nested`. `set_nested_simple`'s trailing `root.put(current_key, nested)` was a no-op: `ExprData::EObject` is a `StoreRef` handle, so mutating the copy returned by `root.get()` already mutates the stored object, and the put re-stores the same handle. Dropping it with the function changes nothing (and the prior bracket path, `set_nested`, never had it). Intentionally not changed here: `set 'contributors[0]=alice'` produces `"contributors": {"0": "alice"}`, an object keyed by the digit string, rather than the array npm's `pkg set` creates, and `set 'array[]=x'` still errors with `InvalidPath` instead of appending. Both are the npm compat gap tracked in oven-sh#22035, which is separate from the memory safety of the key names and is not closed by this PR. ### Verification New test in `test/cli/install/bun-pm-pkg.test.ts` reparses the written file and asserts the exact object. Without the fix it fails on release (`SyntaxError: JSON Parse error: Invalid escape character x`) and on the ASAN debug build (the child aborts on the use-after-free). With the fix the full `bun-pm-pkg.test.ts` suite passes (74 pass, 0 fail).
springmin
pushed a commit
that referenced
this pull request
Jul 2, 2026
…en-sh#33242) ### What After a 3xx redirect, `handle_response_metadata` rewrites per-hop request state on the HTTP-thread clone of the `AsyncHTTP`: - `client.url` (and `connected_url`) become a self-borrow into `client.redirect`, a `Vec<u8>` the clone owns and frees in the final-callback teardown (`AsyncHTTP::on_async_http_callback_raw`). - On a cross-origin hop, `Authorization`/`Proxy-Authorization`/`Cookie`/`Host` are removed from `client.header_entries` in place. - The method may be downgraded to GET. `NetworkTask::notify`'s bitwise copy-back (`ptr::write(real, ptr::read(async_http))`) carries all of that into the JS-thread `AsyncHTTP`. When `bun install` retries the task after a retryable failure (5xx or a connection reset on the redirect target), the re-scheduled request therefore: 1. connects through the freed redirect buffer (use after free), and 2. if the redirect was cross-origin, goes out without `Authorization`, so an authorized registry answers 401. ASAN (debug build), deterministic on the first try: ``` ERROR: AddressSanitizer: heap-use-after-free ... thread T1 (HTTP Client) READ of size 1 #0 bun_core::fmt::parse_int::<u16> src/bun_core/fmt.rs:929 #1 <bun_url::URL>::get_port src/url/lib.rs:470 #2 <bun_url::URL>::get_port_auto src/url/lib.rs:474 #3 <bun_http::http_thread::HttpThread>::connect src/http/HTTPThread.rs:602 #4 <bun_http::HTTPClient>::start_ src/http/lib.rs:2635 #6 <bun_http::async_http::AsyncHTTP>::on_start src/http/AsyncHTTP.rs:893 freed by thread T1 (HTTP Client): <bun_http::async_http::AsyncHTTP>::on_async_http_callback_raw src/http/AsyncHTTP.rs:774 previously allocated by thread T1 (HTTP Client): <bun_http::HTTPClient>::handle_response_metadata src/http/lib.rs:5038 ``` On a release build the same sequence does not crash, but the retries never reach the server (each one connects through freed memory) and the install fails. ### Repro A scripted registry where the manifest URL 302-redirects and the redirect target answers a 500 once, then the real packument: ``` GET /BaR -> 302 Location: /redirected/BaR GET /redirected/BaR -> 500 on the first hit, then the packument GET /BaR-0.0.2.tgz -> tarball ``` `bun install` against it aborts under ASAN and fails on release. Any 301/302/307/308 and 1- or 2-hop chains hit the same path. With an authorized registry that redirects cross-origin (the common Artifactory / CodeArtifact / GitHub Packages shape), the retry also loses `Authorization`; that variant fails with `GET <registry>/BaR - 401` even once the URL is fixed. ### Fix `src/http/AsyncHTTP.rs`: the `!has_more` teardown block already releases every clone-owned allocation. Before freeing `client.redirect`, restore the per-hop state that a re-scheduled attempt must not inherit: - `client.url` back to the caller-owned pre-redirect URL (`AsyncHTTP.url`, which borrows memory valid for the original's whole lifetime), and `client.connected_url` (which `connect` derives from it) to default. - `client.header_entries` back to the untouched `AsyncHTTP.request_headers`. The list is bitwise-shared with the JS-thread original, so it must not be dropped or reallocated on the HTTP thread; it was cloned from `request_headers` at init and only ever shrinks, so `clear_retaining_capacity()` + `append_list_assume_capacity()` restores it in place. - `client.method` back to `AsyncHTTP.method`. Nothing that crosses back to the JS thread references clone-freed memory anymore, and a retried request restarts from the original URL with the original headers instead of the last redirect hop's, which is what the install-level retry is meant to do. ### Tests `test/cli/install/bun-install-retry.test.ts`: - `retries a manifest whose redirect target 500s once` - `retries a tarball whose redirect target 500s once` (the sibling retry site in `runTasks`) - `retries an authorized manifest whose cross-origin redirect target 500s once` (also asserts the cross-origin hop itself still does NOT carry `Authorization`, so the spec-mandated strip is unchanged) All three fail on the unfixed build (ASAN abort under `bun bd`, install error with `USE_SYSTEM_BUN=1`). The third additionally fails with a 401 if only the URL is restored and not the headers, so each restore is load-bearing. `test/js/web/fetch/fetch-redirect.test.ts` and `fetch-url-after-redirect.test.ts` still pass, so `response.url` after a redirect is unaffected (it comes from the owned `metadata.url` copy, not from `client.url`). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 15, 2026
…ry rewrite (oven-sh#34271) `test/js/bun/util/filesystem_router.test.ts` went red on alpine x64 in build [73276](https://buildkite.com/bun/bun/builds/73276): the `reload() while Bun.build() resolves the same directory` subprocess segfaulted in `bust_dir_cache_recursive`, inlined from `NonNull::new`. ## Cause `RealFS::entries_at` (`src/resolver/lib.rs`) replaces a cached `DirEntry` in place when the caller's resolver generation is newer than the cached listing's. The replacement at `*e_ptr = new_entry` drops the old `DirEntry`, which drops its `data: StringHashMap<*mut Entry>` and frees the hashmap's bucket allocation. The function's comment says `entries_mutex held by caller`, but that is only true on one of the five paths that reach it: `dir_info_uncached`, when entered from `dir_info_cached_miss`. The other callers (`finalize_result`, `handle_esm_resolution`, `load_index_with_extension`, `Transpiler::run_env_loader`) all reach `entries_at` after `dir_info_cached_maybe_log` has already returned and released both `RESOLVER_MUTEX` and `entries_mutex`. `FileSystemRouter::reload()` and `RouteLoader::load` iterate the same `DirEntry.data` map under `entries_mutex` (the snapshot pattern oven-sh#33056 introduced for exactly this kind of concurrent rewrite). With `entries_at`'s rewrite unsynchronized, a `Bun.build()` on the bundler thread can drop the map while `reload()` on the JS thread is mid-iteration. The generation mismatch is what makes `entries_at` enter its rewrite branch, so the window only opens once the bundle thread has processed at least one batch (it bumps its own generation after every queue drain); every subsequent `Bun.build()` then re-reads any directory that `reload()` just refreshed to generation 0. ASAN catches it as a heap-use-after-free with the two sides of the race laid out exactly: ``` READ of size 16 (thread T0): #6 HashMap::values #7 StringHashMap<*mut Entry>::values src/collections/array_hash_map.rs:1864 #8 FileSystemRouter::bust_dir_cache_recursive src/runtime/api/filesystem_router.rs:395 #9 FileSystemRouter::bust_dir_cache src/runtime/api/filesystem_router.rs:451 #10 FileSystemRouter::reload src/runtime/api/filesystem_router.rs:476 freed by thread T11 (Bundler): #11 drop_in_place<bun_resolver::fs_full::DirEntry> #12 bun_resolver::fs::RealFS::entries_at src/resolver/lib.rs:1639 #13 DirInfo::get_entries_ref src/resolver/dir_info.rs:266 #14 Resolver::finalize_result src/resolver/resolver.rs:1714 #15 Resolver::resolve_and_auto_install src/resolver/resolver.rs:1485 ... oven-sh#23 BundleThread::generate_in_new_thread src/bundler/BundleThread.rs:276 previously allocated by thread T0: oven-sh#17 HashMap::reserve oven-sh#18 Resolver::dir_info_cached_miss src/resolver/resolver.rs:4591 oven-sh#19 Resolver::dir_info_cached_maybe_log src/resolver/resolver.rs:4201 oven-sh#20 Resolver::read_dir_info src/resolver/resolver.rs:4118 oven-sh#21 FileSystemRouter::reload src/runtime/api/filesystem_router.rs:492 ``` (The use side is sometimes `RouteLoader::load` at `src/router/lib.rs:816` instead; same map, same lock.) This has been the shape of `entries_at` since the Rust port; oven-sh#33056 narrowed the race by snapshotting under the lock but assumed the rewrite side already held it. ## Fix `entries_at` now takes `entries_mutex` itself, matching `read_directory_with_iterator` which already does. The one call path that reaches it with the lock already held (`dir_info_cached_miss` -> `dir_info_uncached` -> `parent_.get_entries_ref`) routes through a new `entries_at_locked` / `get_entries_ref_locked` pair so the non-recursive mutex is not re-entered. That path is the only one that passes a non-`None` parent to `dir_info_uncached`; the other caller (`dir_info_for_resolution`) passes `None`, so the parent branch containing the accessor never runs there. ## Test The existing concurrency test now awaits one `Bun.build()` first, so the bundle thread's generation is already past zero when the concurrent rounds start, and then runs forty reload/build rounds instead of one. That is the shape that reaches the stale-generation rewrite at all; the original single-round fixture usually completes with every build still on generation 0. The race is scheduling-dependent. Pinning the fixture to a single core reproduces the ASAN use-after-free on roughly 3 in 10 runs against an unpatched debug build and 0 in 15 with this change; with all 16 cores available the unpatched build reproduces at roughly 1 in 30. The assertions are otherwise the same as before, so the test continues to cover the behavior oven-sh#33056 added. Also ran the full `filesystem_router.test.ts`, `test/bundler/bun-build-api.test.ts` (including the thousands-of-builds test that exercises the generation path heavily), `test/js/bun/resolve/resolve.test.ts`, `test/cli/hot/hot.test.ts`, `test/cli/watch/watch.test.ts`, `test/bake/framework-router.test.ts`, and `bun run rust:check-all` (10/10 targets). <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/util/filesystem_router.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Jul 18, 2026
…n worker terminate (oven-sh#34455) ## What Fixes a heap-use-after-free when a Worker with an in-flight `dns.lookup()` / `dns.resolve*()` is terminated. Surfaced by Node's upstream `test/parallel/test-worker-dns-terminate.js` (being vendored in oven-sh#34441), on the debian 13 x64-asan lane: ``` ==11356==ERROR: AddressSanitizer: heap-use-after-free on address 0x12ce0a3af168 READ of size 4 at 0x12ce0a3af168 thread T6 (Worker) #0 FilePoll::unregister src/io/posix_event_loop.rs:951 #1 FilePoll::deinit_possibly_defer src/io/posix_event_loop.rs:428 #2 FilePoll::deinit_with_vm src/io/posix_event_loop.rs:448 #3 Resolver::on_dns_socket_state src/runtime/dns_jsc/dns.rs:4894 #6 ares_conn_sock_state_cb_update vendor/cares/src/lib/ares_conn.c:36 freed by thread T6 (Worker): drop_in_place<Box<posix_event_loop::Store>> (RareData field drop) VirtualMachine::destroy src/jsc/VirtualMachine.rs:4453 WebWorker::shutdown src/jsc/web_worker.rs:1299 ``` ## Repro ```js const { Worker } = require('worker_threads'); const w = new Worker(` const dns = require('dns'); dns.lookup('nonexistent.org', () => {}); require('worker_threads').parentPort.postMessage('0'); `, { eval: true }); w.on('message', () => w.terminate()); ``` ## Cause `WebWorker::shutdown()` runs, in order: `WebWorker__teardownJSCVM` (frees the `JSGlobalObject`), then `VirtualMachine::destroy()` which drops `rare_data` (frees the `FilePoll` hive `Store`) and finally calls `deinit_runtime_state` which drops `RuntimeState`. That last drop runs `GlobalData::drop` which calls `ares_destroy()` on the per-VM c-ares channel. `ares_destroy()` synchronously fires every pending query callback with `ARES_EDESTRUCTION` and then the socket-state callback for each fd it closes. Those callback chains re-enter: - `Resolver::on_dns_socket_state` -> `FilePoll::deinit_with_vm` on the already-freed hive slot (the ASAN trace above) - `GetAddrInfoRequest::on_cares_complete` -> `DNSLookup::process_get_addr_info` -> `reject_later(global_this)` on the freed `JSGlobalObject` (bmalloc-backed so ASAN misses it) - `ResolveInfoRequest::on_cares_complete` -> `request_completed()` -> `remove_timer()` -> `(*runtime_state()).timer` with the TLS already nulled (null deref) ## Fix Add a `RuntimeHooks::close_dns_for_terminate` slot that runs `Resolver::close_channel_for_terminate()` from `WebWorker::shutdown()` (and the `BUN_DESTRUCT_VM_ON_EXIT` main-thread path) right after `close_all_socket_groups`, while JSC, `RareData.file_polls`, the event loop, and `runtime_state` are all still live. The method also removes the resolver's c-ares timeout timer, which `GetAddrInfoRequest`'s EDESTRUCTION path never unwinds. `GlobalData::drop` still handles the channel if the early hook never ran (it sees `channel == None` when it did). This matches Node's model: `Worker::Exit` -> `CleanupHandles()` closes every handle wrap (including `ChannelWrap`) before disposing the Isolate. ## Verification New ASAN-gated test in `test/js/web/workers/worker-terminate-lifetime.test.ts` spawns four workers that each start a `dns.lookup()` + `dns.resolve4()` and terminates them mid-flight. - **fail-before** (`git stash -- src/ && bun bd test ...`): null-deref panic / ASAN heap-use-after-free - **pass-after**: clean exit 0 across 10 consecutive runs Also verified `test/js/node/dns/` and `test/js/bun/dns/` pass/fail counts are unchanged vs. main, and `bun run rust:check-all` is clean on all targets. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/workers/worker-terminate-lifetime.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Jul 23, 2026
oven-sh#35255) `test/js/bun/http/serve-protocols.test.ts` has been going red on main (build 78445 darwin-x64 hard, 78462 debian-11-aarch64 hard, plus 78300/78419 with retries), always as ``` error: HTTP3StreamReset fetching "https://127.0.0.1:<port>/echo" ✗ Bun.serve over http/3 > POST echo 1000000 bytes [20169.37ms] ``` Reproduced on Linux by looping the file: the h3 subset alone fails about 17 of 100 runs. ## Cause The ten concurrent h3 tests share one lsquic client engine and one unconnected UDP socket. `bsd_create_udp_socket()` sets `IP_RECVERR` on every UDP socket (for `node:dgram`'s error surfacing, oven-sh#28827), including QUIC's. When a finished test `proc.kill()`s its server, the client session is still in the engine and keeps scheduling retransmits / `NEW_CONNECTION_ID` to an unbound port. With `IP_RECVERR` on, the resulting ICMP port-unreachable is queued on the shared socket and the next `sendmmsg` returns `-1 ECONNREFUSED`, even though that call is sending a datagram to a live peer. `us_quic_packets_out` reports that as a short return, lsquic clears `ENPUB_CAN_SEND` for the whole engine and only its one-second `resume_sending_at` failsafe re-enables it. With several dead sessions generating ICMPs, every failsafe retry fails the same way and the live 1MB upload never advances; the 20s in CI is two idle-timeout rounds through `retry_or_fail`. While tracing that I also found an unsigned underflow in lsquic's `send_batch` requeue loop: when the first unsent spec in a batch coalesces multiple packets (`pack_off[0] == 0`, `iovlen > 1`), `end = &batch->packets[off - 1]` indexes with `UINT_MAX` and only the last packet of the coalesced group is returned to the connection. The earlier ones are the INIT ACK and the HSK CRYPTO carrying the client Finished, so the peer can never complete the handshake. This is the same hang reached from a different direction (real EAGAIN backpressure instead of stale ICMP). ## Fix - `IP_RECVERR` is now opt-in via `LIBUS_UDP_LINUX_RECVERR`, set by `us_create_udp_socket` when a `recv_error_cb` is provided. `node:dgram` always passes one and keeps the option; QUIC passes `NULL` and no longer gets it. This matches libuv's `UV_UDP_LINUX_RECVERR` gating that `bsd.c` already cited. - `us_quic_packets_out()` retries once on a non-`EAGAIN`/`ENOBUFS` send failure before reporting a short return, so a stale `sk_err` that does surface cannot pause the engine. Both the `sendmmsg` and per-packet paths now go through `US_FAULT_CHECK(US_FAULT_SENDMSG, ...)` so the short-return path is reachable from tests. - `patches/lsquic/requeue-unsent-coalesced.patch` rewrites the requeue loop's bounds as `[off, off+count)` so every packet in an unsent coalesced datagram is returned to the connection. The same underflow is present in upstream lsquic master; I will open a PR there separately. - `serve-protocols.test.ts` now stops each fixture server gracefully on stdin close (`server.stop(true)`), matching `serve-http3.test.ts`, so the pooled client session sees `CONNECTION_CLOSE` instead of leaving the engine retransmitting to unbound ports. - `test/js/web/fetch/fetch-http3-syscall-fault.test.ts` injects `EAGAIN` on the coalesced handshake datagram (the `pack_off[0]==0`, `iovlen>1` spec the lsquic patch fixes), a one-shot `ECONNREFUSED` that the retry-once branch consumes, and a burst of `EAGAIN` that the `on_drain` path recovers from. ## Verification Release build, looped: | | before | after | | --- | --- | --- | | `serve-protocols -t "http/3"` | 17/100 fail | 2/100 fail | | `serve-protocols` (full) | 7/100 fail | 4/200 fail | Debug+ASAN: `serve-protocols`, `serve-http3` (46), `fetch-http3-client` (52), `fetch-http3-adversarial` (29), `fetch-http3-syscall-fault` (3) and `dgram.test.ts` all pass, 211 tests total. The residual ~1-2% is a separate pre-existing bug (the client's 36-byte HSK CRYPTO is buffered but never flushed when `drain_send_body` writes the whole 1MB body synchronously from `on_stream_open`); I've handed that off as its own issue. With CI's retry it is well under the flake threshold. ### Gate note The fault-injection hook that makes the new test deterministic lives in `packages/bun-usockets/src/quic.c`, so `git stash -- src/ packages/` removes it along with the fix and the fault never fires. The lsquic piece lives in `patches/` and `scripts/`, which the stash does not touch. That means a single stashed run passes (no fault, no stall) and a single unstashed run passes (fault fires, fix handles it), and the gate cannot distinguish them mechanically. The 300-iteration probe above is the evidence; the fault-injection tests pin the behavior going forward. <details> <summary>lsquic debug trace of the stall</summary> ``` engine: packets out returned 0 (out of 1) [C919…] event: unsent packet #15 ACK_FREQUENCY, size 36 [C919…] sendctl: packet #15 has been delayed engine: send_packets_out: sent 0 packets … <- no "can send again"; nothing for 1s engine: failsafe activated: resume sending packets again after timeout engine: packets out returned 0 (out of 10) <- fails again, live conn's oven-sh#207 included ``` and for the underflow, a batch with `pack_off[0]=0`, `iovlen[0]=3`: ``` engine: packets out returned 0 (out of 2) event: unsent packet #3 ACK PADDING, size 1059 event: unsent packet #4 ACK CRYPTO, size 87 event: unsent packet #5 NEW_CONNECTION_ID, size 54 event: unsent packet #6 STREAM, size 114 sendctl: packet #6 has been delayed sendctl: packet #5 has been delayed … <- #3 and #4 never requeued [WARN] sendctl: send history gap 2 - 5 ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/fetch-http3-syscall-fault.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 29, 2026
…rier (oven-sh#36337) `JSNativeStreamSourceAdapter::m_controller` was a `JSC::Weak<JSReadableStreamDefaultController>`. When the native pull promise is rejected (socket fault on a fetch body) the adapter is queued as the `onNativePullRejected` reaction context, which roots the **adapter** but not the **controller**: the adapter's only edge to it was the `Weak`. `FetchTasklet` releases both native `Strong<>`s to the body stream before that microtask drains, so a GC in between can leave the entire consumer graph (`controller -> stream -> reader -> pipe op -> destination -> writer -> readyPromise`) white. The subsequent error cascade then enqueues the pipe's writes-drained shutdown deferral against a corpse `op`, and `performPipeShutdownAction(AbortDestination)` dereferences a swept `readyPromise`: ``` ASSERTION FAILED: result JSObject.h(583) JSGlobalObject *JSC::JSObject::realm() const #5 JSC::JSObject::realm() #6 JSC::JSPromise::rejectPromise #7 JSC::JSPromise::reject #8 Bun::WebStreams::writableStreamDefaultWriterEnsureReadyPromiseRejected #9 Bun::WebStreams::writableStreamStartErroring #10 Bun::WebStreams::writableStreamAbort #11 WebCore::performPipeShutdownAction (AbortDestination) #12 WebCore::JSStreamPipeToOperation::onWritesFinishedForShutdown ``` On builds without the assert the same path is a silent write into freed/reused promise memory. ## Fix Hold `m_controller` as a visited internal field so a queued adapter roots the controller directly. The edge is cleared on every terminal path (`nativeSourcePullRejected`, `nativeSourceCallClose`, `nativeSourceCancel`); `controller->algorithmContext` is cleared by `readableStreamDefaultControllerClearAlgorithms`, so the abandoned case is an ordinary intra-heap cycle mark-sweep collects. `NewSource::this_jsvalue` is only `Strong` during FileReader I/O, where pinning the consumer graph is the correct behavior anyway. With the `Weak` gone the adapter no longer needs a destructor, so it is now a `JSInternalFieldObjectImpl<5>`: the five JSValue members (handle, pendingView, closer, drainValue, controller) are internal fields visited by the base class, with typed accessors at call sites. The scalar members (chunkSize, flag bitfield, text-decode state) stay as plain members. ## Verification `native-source-onclose-leak.test.ts` (the partial-read + `releaseLock` abandonment tests for Blob/fetch/File sources) continues to pass, confirming the cycle does not pin. `streams.test.js`, `pipeTo-signal-leak.test.ts`, `compression.test.ts`, `blob.test.ts` all pass. The crash itself is 0/1800 standalone; it reproduces ~1/3 only under a fault-injected tracer replay. `pipeTo-shutdown-gc.test.ts` exercises the shape (native body source, socket fault mid-stream, fire-and-forget `pipeTo` under `collectContinuously`, `AbortDestination` shutdown arm) as a regression surface. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/streams/pipeTo-shutdown-gc.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 6, 2026
…llback (oven-sh#36986) ## What `test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts` (the crypto-generateKeyPair fixture) fails on every Linux x64-asan run since oven-sh#36598 landed (builds [89023](https://buildkite.com/bun/bun/builds/89023), [89031](https://buildkite.com/bun/bun/builds/89031)): ``` direct leak of 24b in run (src/runtime/node/node_crypto_binding.rs:85:21) +34 more SUMMARY: AddressSanitizer: 1480 byte(s) leaked in 35 allocation(s). #6 EVP_PKEY_keygen vendor/boringssl/crypto/evp/evp_ctx.cc #7 Bun::KeyPairJobCtx::runTask src/jsc/bindings/node/crypto/CryptoGenKeyPair.cpp:23 #8 Bun__RsaKeyPairJobCtx__runTask src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp:26 ``` ## Cause The 11 extern crypto job ctxs (generateKeyPair x5, sign/verify, diffieHellman, hkdf, generatePrime, checkPrime, generateKey) completed by invoking the JS callback from inside C++ `runFromJS` while the ctx was still alive; the ctx was freed only after `then()` returned. A callback that never returns (the fixture calls `process.exit(0)` inside it) stranded everything the ctx still owned: the generated `EVP_PKEY`, `KeyObjectData` refs, `BIGNUM`s. The leak is pre-existing; oven-sh#36598 made it observable by routing `OPENSSL_malloc` through libc under ASAN. Whether LSan reported the other job types too was codegen luck (their pointers happened to be reachable by the conservative stack scan); `generateKeyPair`'s `EVP_PKEY` sits behind two FastMalloc indirections and was reported deterministically. ## Fix Make it structurally impossible for a job ctx to hold native resources across user JS: the native side never sees the callback. - `runFromJS` keeps its name (the JS-thread half, paired with the work-pool half `runTask`) but no longer receives the callback. It returns `JSCallbackArgs`, a small by-value type whose constructors are the only producers, so bodies read `return { err };` or `return { jsNull(), publicKey, privateKey };`. The extern "C" shims copy it through a typed out-pointer (C linkage cannot return a class type); the Rust side consumes it as a slice. - The Rust `extern_crypto_job!` plumbing does, in order: run `runFromJS` to produce the arguments, free the ctx (`ctx_deinit`), invoke the callback. The invariant lives in one place and applies to every job type. - Shutdown release: a completion task enqueued but not yet dispatched when `process.exit()` runs (exit racing the work pool) used to be re-queued at shutdown, stranding the ctx the same way. `AnyTaskJob` now carries an erased release entry and the shutdown release frees the job without running its completion. A completion posted after the final drain is not recoverable without joining the work pool (which would block exit); `test-crypto-op-during-process-exit.js` stays in `no-validate-leaksan.txt` for that sliver, now with an accurate comment. - The caught-export-exception paths encoded the `JSC::Exception` cell itself, so the callback's err argument was not the thrown Error (not `instanceof Error`, no `code`). They now use `Exception::value()`, matching node: JWK export of an unsupported curve surfaces `ERR_CRYPTO_JWK_UNSUPPORTED_CURVE`. No behavior change otherwise: - `Bun__EventLoop__runCallback{1,2,3}` were Rust's `EventLoop::run_callback` exported to C++. The plumbing now calls `run_callback` directly: same enter/exit bracketing, same pending-exception gate, same unhandled-exception reporting, same synchronous timing. This made `runCallback1`/`runCallback3` dead (the crypto bodies were their last callers), so their exports and declarations are deleted; `runCallback2` stays for the webview backends. - Callback arity is preserved per path (observable via `arguments.length`): error paths pass 1 arg, results 2, generateKeyPair success 3. - Exception paths are preserved: a throw out of argument production skips the callback and reports unhandled, as before. Each `runFromJS` checks its `ThrowScope` after every call that can throw (`RETURN_IF_EXCEPTION`), since the check that used to happen inside the nested `runCallbackN` call now happens after the C++ scope destructs; `BUN_JSC_validateExceptionChecks` verifies this on the asan lane. - The produced `JSValue`s live on the `then()` stack frame between production and invocation, which JSC's conservative scan covers; they are JS-heap values, so freeing the ctx first cannot invalidate them. - Perf: same number of FFI crossings, no allocation added. The Rust-native crypto jobs (pbkdf2, scrypt, random) already had the ordering property: they resolve promises or queue the callback via nextTick, so their ctx drops before user JS runs. The synchronous-callback extern jobs were the gap. ## Verification New tests in `crypto.key-objects.test.ts`: - `isASAN`-gated leak suite: children run with `BUN_DESTRUCT_VM_ON_EXIT=1` and `detect_leaks=1` (the asan lane's configuration) and call `process.exit(0)` from the callback of each job type: generateKeyPair (KeyObject and encrypted PEM outputs), sign, diffieHellman, hkdf, checkPrime, generateKey, plus an exit-before-completion-dispatch case (busy-spin so the queued completion is never dispatched). - An export-error test: `generateKeyPair('ec', { namedCurve: 'secp224r1', ...jwk encodings })` asserts the callback err is `instanceof Error` with code `ERR_CRYPTO_JWK_UNSUPPORTED_CURVE` (matches node; fails on main, which passes the Exception cell). Results: - unfixed build (src stashed): both generateKeyPair leak tests fail with the exact CI signature (`Direct leak of 24 byte(s)` in `EVP_PKEY_keygen` via `KeyPairJobCtx::runTask`) - fixed build: all pass, including under `BUN_JSC_validateExceptionChecks=1`, and ec/ed25519 keypair and verify probes run leak-clean as well - `AsyncLocalStorage-tracking.test.ts`: 74 pass, 0 fail (all async-context crypto fixtures, against both bun and node) - `crypto.test.ts` (369), `crypto.key-objects.test.ts` (117), and 37 node parallel files (`test-crypto-keygen*`, `test-crypto-sign-verify`, `test-crypto-hkdf`, `test-crypto-dh-stateless`, `test-crypto-*prime*`) all pass The break landed with oven-sh#36598 (which made the leak visible); oven-sh#36657 proposed clearing individual ctx fields before the callback, and this PR supersedes that approach with the ordering guarantee in the job plumbing instead of per-field resets. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts test/js/node/crypto/crypto.key-objects.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 6, 2026
…llback (oven-sh#36986) ## What `test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts` (the crypto-generateKeyPair fixture) fails on every Linux x64-asan run since oven-sh#36598 landed (builds [89023](https://buildkite.com/bun/bun/builds/89023), [89031](https://buildkite.com/bun/bun/builds/89031)): ``` direct leak of 24b in run (src/runtime/node/node_crypto_binding.rs:85:21) +34 more SUMMARY: AddressSanitizer: 1480 byte(s) leaked in 35 allocation(s). #6 EVP_PKEY_keygen vendor/boringssl/crypto/evp/evp_ctx.cc #7 Bun::KeyPairJobCtx::runTask src/jsc/bindings/node/crypto/CryptoGenKeyPair.cpp:23 #8 Bun__RsaKeyPairJobCtx__runTask src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp:26 ``` ## Cause The 11 extern crypto job ctxs (generateKeyPair x5, sign/verify, diffieHellman, hkdf, generatePrime, checkPrime, generateKey) completed by invoking the JS callback from inside C++ `runFromJS` while the ctx was still alive; the ctx was freed only after `then()` returned. A callback that never returns (the fixture calls `process.exit(0)` inside it) stranded everything the ctx still owned: the generated `EVP_PKEY`, `KeyObjectData` refs, `BIGNUM`s. The leak is pre-existing; oven-sh#36598 made it observable by routing `OPENSSL_malloc` through libc under ASAN. Whether LSan reported the other job types too was codegen luck (their pointers happened to be reachable by the conservative stack scan); `generateKeyPair`'s `EVP_PKEY` sits behind two FastMalloc indirections and was reported deterministically. ## Fix Make it structurally impossible for a job ctx to hold native resources across user JS: the native side never sees the callback. - `runFromJS` keeps its name (the JS-thread half, paired with the work-pool half `runTask`) but no longer receives the callback. It returns `JSCallbackArgs`, a small by-value type whose constructors are the only producers, so bodies read `return { err };` or `return { jsNull(), publicKey, privateKey };`. The extern "C" shims copy it through a typed out-pointer (C linkage cannot return a class type); the Rust side consumes it as a slice. - The Rust `extern_crypto_job!` plumbing does, in order: run `runFromJS` to produce the arguments, free the ctx (`ctx_deinit`), invoke the callback. The invariant lives in one place and applies to every job type. - Shutdown release: a completion task enqueued but not yet dispatched when `process.exit()` runs (exit racing the work pool) used to be re-queued at shutdown, stranding the ctx the same way. `AnyTaskJob` now carries an erased release entry and the shutdown release frees the job without running its completion. A completion posted after the final drain is not recoverable without joining the work pool (which would block exit); `test-crypto-op-during-process-exit.js` stays in `no-validate-leaksan.txt` for that sliver, now with an accurate comment. - The caught-export-exception paths encoded the `JSC::Exception` cell itself, so the callback's err argument was not the thrown Error (not `instanceof Error`, no `code`). They now use `Exception::value()`, matching node: JWK export of an unsupported curve surfaces `ERR_CRYPTO_JWK_UNSUPPORTED_CURVE`. No behavior change otherwise: - `Bun__EventLoop__runCallback{1,2,3}` were Rust's `EventLoop::run_callback` exported to C++. The plumbing now calls `run_callback` directly: same enter/exit bracketing, same pending-exception gate, same unhandled-exception reporting, same synchronous timing. This made `runCallback1`/`runCallback3` dead (the crypto bodies were their last callers), so their exports and declarations are deleted; `runCallback2` stays for the webview backends. - Callback arity is preserved per path (observable via `arguments.length`): error paths pass 1 arg, results 2, generateKeyPair success 3. - Exception paths are preserved: a throw out of argument production skips the callback and reports unhandled, as before. Each `runFromJS` checks its `ThrowScope` after every call that can throw (`RETURN_IF_EXCEPTION`), since the check that used to happen inside the nested `runCallbackN` call now happens after the C++ scope destructs; `BUN_JSC_validateExceptionChecks` verifies this on the asan lane. - The produced `JSValue`s live on the `then()` stack frame between production and invocation, which JSC's conservative scan covers; they are JS-heap values, so freeing the ctx first cannot invalidate them. - Perf: same number of FFI crossings, no allocation added. The Rust-native crypto jobs (pbkdf2, scrypt, random) already had the ordering property: they resolve promises or queue the callback via nextTick, so their ctx drops before user JS runs. The synchronous-callback extern jobs were the gap. ## Verification New tests in `crypto.key-objects.test.ts`: - `isASAN`-gated leak suite: children run with `BUN_DESTRUCT_VM_ON_EXIT=1` and `detect_leaks=1` (the asan lane's configuration) and call `process.exit(0)` from the callback of each job type: generateKeyPair (KeyObject and encrypted PEM outputs), sign, diffieHellman, hkdf, checkPrime, generateKey, plus an exit-before-completion-dispatch case (busy-spin so the queued completion is never dispatched). - An export-error test: `generateKeyPair('ec', { namedCurve: 'secp224r1', ...jwk encodings })` asserts the callback err is `instanceof Error` with code `ERR_CRYPTO_JWK_UNSUPPORTED_CURVE` (matches node; fails on main, which passes the Exception cell). Results: - unfixed build (src stashed): both generateKeyPair leak tests fail with the exact CI signature (`Direct leak of 24 byte(s)` in `EVP_PKEY_keygen` via `KeyPairJobCtx::runTask`) - fixed build: all pass, including under `BUN_JSC_validateExceptionChecks=1`, and ec/ed25519 keypair and verify probes run leak-clean as well - `AsyncLocalStorage-tracking.test.ts`: 74 pass, 0 fail (all async-context crypto fixtures, against both bun and node) - `crypto.test.ts` (369), `crypto.key-objects.test.ts` (117), and 37 node parallel files (`test-crypto-keygen*`, `test-crypto-sign-verify`, `test-crypto-hkdf`, `test-crypto-dh-stateless`, `test-crypto-*prime*`) all pass The break landed with oven-sh#36598 (which made the leak visible); oven-sh#36657 proposed clearing individual ctx fields before the callback, and this PR supersedes that approach with the ordering guarantee in the job plumbing instead of per-field resets. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts test/js/node/crypto/crypto.key-objects.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 16, 2026
) ### Problem - mordant's `tuple_wants_struct` flags `color::parse_hash_color` in `src/css/css_parser.rs:5730`: it returns `(u8, u8, u8, f32)`, and both callers (`CssColor::parse` in `src/css/values/color.rs:408`, `TokenList` hash tokens in `src/css/properties/custom.rs:653`) unpack it as `(r, g, b, a)`. The channel names live only at the call sites, and since the first three fields are all `u8`, a swapped pair still compiles. - Both callers then immediately build an `RGBA` out of the tuple, so the function already had a named type for its result; `custom.rs` also converted each byte to a float and `RGBA::from_floats` converted it straight back. ### Fix - `parse_hash_color` / `parse_hash_color_impl` build the `RGBA` (`red`/`green`/`blue`/`alpha`, all `u8`) themselves and return it; the callers wrap it in `CssColor::Rgba`. `OPAQUE` becomes the `u8` alpha `255` it was being converted to. - No behavior change. The hex digits were already parsed as bytes; the only thing removed is the byte -> f32 -> byte round trip through `RGBA::new` / `RGBA::from_floats`, which is the identity for all 256 values (checked with f32 arithmetic), and `OPAQUE` (`1.0`) mapped to `255` the same way. - The baseline entry `tuple_wants_struct:src/css/css_parser.rs` is removed from `mordant-baseline.toml`. The `[bun_css]` section was regenerated with `MORDANT_BASELINE_WRITE=1 cargo dylint --all -p bun_css --no-deps` (the per-crate form of `bun run rust:mordant:baseline`); that line is the only difference, the `values/color.rs = 5` entry is unchanged. - No test is added. The change has no observable effect (the `RGBA` produced for every hash is byte for byte what it was), so there is no test that fails before it and passes after it; the check that does flip is the mordant run, which reports the `css_parser.rs` finding over the trimmed baseline without this change and is clean with it. The existing coverage below is what guards the refactor. - Verified with `bun bd`: - `bun bd test test/js/bun/css/css.test.ts`: 1142 pass. This covers both callers with every hash length: 8-digit hashes inside custom properties (`--bar: ... #01010116` round-trips through `custom.rs`), `#6` / `#ffffff80` / 3- and 6-digit hashes in regular properties through `color.rs`. - `bun bd test test/js/bun/css/color.test.ts` (`Bun.color`): 993 pass. - `bun bd test test/bundler/css/`: 169 pass. - `cargo check -p bun_css`, `cargo clippy -p bun_css --no-deps` and `cargo dylint --all -p bun_css --no-deps` (mordant) are clean; the latter writes no `target/mordant/over-baseline.txt`. ### Background - `css_parser::color` is the port of the `cssparser` crate's color helpers; `parse_hash_color` parses the text after `#` (3, 4, 6 or 8 hex digits) into channels. `RGBA` (`src/css/values/color.rs`) is the crate's byte-per-channel color and is what `CssColor::Rgba` stores, which is why both callers were converting to it. - `mordant-baseline.toml` is the ratchet for the mordant lint pack: it records the per-(lint, file) finding counts that predate the job so CI only fails on new findings. Fixing a site means removing (or decrementing) its entry; mordant's write mode rewrites one section per compiled crate, so regenerating with only `bun_css` compiled touches only that section. --------- Co-authored-by: Alistair Smith <hi@alistair.sh>
springmin
pushed a commit
that referenced
this pull request
Aug 16, 2026
…wo parallel vecs (oven-sh#39145) ### Problem - `LOLHTMLContext` in `src/runtime/api/html_rewriter.rs` keeps two vecs, `selectors` and `element_handlers`, that describe one thing: entry `i` of each is the selector and the handler object from the same `rewriter.on(selector, handlers)` call. - The pairing is only held up by convention: `on_()` pushes to both, `build_settings()` zips them back together, and a doc comment plus an invariant comment explain it. The mordant `parallel_vecs` lint flags this (the one baselined finding for this file). ### Fix - Add `ElementHandlerEntry { selector, handler: Box<ElementHandler> }` and store `element_handlers: Vec<ElementHandlerEntry>`. One vec, one push in `on_()`, and `build_settings()` destructures each entry instead of zipping. - No behavior change: the same values are pushed in the same order, the handler is still boxed (the lol-html closures built in `build_settings()` hold raw pointers into the box, so it must not move when the vec reallocates), and the body of the `build_settings()` loop is unchanged. The `#[expect(clippy::vec_box)]` comes off `element_handlers` because it is no longer a `Vec<Box<_>>`; `document_handlers` keeps its own. - Remove the `parallel_vecs:src/runtime/api/html_rewriter.rs` line from `mordant-baseline.toml`. - Tests, in `test/js/workerd/html-rewriter.test.js` (`on() registrations`), pin down the two things this storage has to get right. They pass before and after this change, since it is a refactor: - Many selectors registered on one rewriter, with two rejected `on()` calls in the middle, each still run the handlers they were registered with, on two transforms of the same rewriter. - `on()` called from inside a handler, often enough to reallocate the registry while lol-html is still calling the handlers registered before the transform started: the running transform is unaffected and the next one picks the additions up. With the `Box` removed from `ElementHandlerEntry` this test fails under ASAN with a heap-use-after-free (report in the details below), so the boxing is now covered rather than only commented. - Verified: - `bun bd test` on `test/js/workerd/html-rewriter.test.js` (165 tests, including the new ones), `html-rewriter-end-error.test.ts`, `html-rewriter-leak.test.ts`, `test/js/web/html/html-rewriter-doctype.test.ts` and the HTMLRewriter regression tests: all pass. - `cargo clippy -p bun_runtime --no-deps`: clean. - `cargo dylint --all -p bun_runtime` with this baseline: nothing over the baseline. The same command with the baseline line removed but the source change stashed reports exactly the one `parallel_vecs` finding for this file, so the removed line is the one this change fixes. - Regenerating the baseline with `MORDANT_BASELINE_WRITE=1` also drops two entries this PR does not touch (`always_unwrapped_option:src/install/PackageInstall.rs`, `narrowed_two_ways:src/runtime/node/node_crypto_binding.rs`); those findings were already fixed on main by other changes and are left for a separate cleanup. ### Background - `HTMLRewriter.on(selector, handlers)` parses the CSS selector with lol-html and wraps the JS handler object in an `ElementHandler` (the protected `element`/`comments`/`text` callbacks). Nothing is handed to lol-html at that point; registrations are collected in `LOLHTMLContext`, which is shared by the rewriter and every transform it starts, because `transform()` can run more than once. - `build_settings()` runs at transform time and turns each registration into a `(selector, ElementContentHandlers)` pair for lol-html. Its closures capture a `NonNull<ElementHandler>` pointing into the heap allocation owned by the `Box`, which is why the handler has to stay boxed even though clippy would normally suggest otherwise. An `on()` call after a transform has started (for example from inside a handler) pushes onto the same vec, which is what makes the reallocation case reachable from JS. - `mordant-baseline.toml` is the ratchet for the mordant lint pack run by the Rust lints workflow: it records the accepted number of findings per (lint, file), and CI reports anything above those counts. Removing the line here means a reintroduction of the pattern in this file would be reported. <details> <summary>ASAN report from the new test with the Box removed from ElementHandlerEntry</summary> ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8 #3 <ElementHandler as HandlerLike>::global src/runtime/api/html_rewriter.rs #4 handler_callback::<ElementHandler, Element, ...> src/runtime/api/html_rewriter.rs #5 ElementHandler::on_element src/runtime/api/html_rewriter.rs #6 build_settings::{closure#0} src/runtime/api/html_rewriter.rs #8 lol_html ContentHandlersDispatcher::handle_start_tag freed by thread T0 here: #13 RawVec<ElementHandlerEntry>::grow_one #15 Vec<ElementHandlerEntry>::push oven-sh#16 HTMLRewriter::on_ src/runtime/api/html_rewriter.rs ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/workerd/html-rewriter.test.js <!-- robobun:evidence:end --> --------- Co-authored-by: Alistair Smith <hi@alistair.sh>
springmin
pushed a commit
that referenced
this pull request
Sep 1, 2026
…h#36545) ## Repro Under a debug/ASAN build: ```js const result = Bun.spawnSync(["cat"], { stdin: "ignore" }); const copy = { ...result }; ``` ``` ASSERTION FAILED: hasInlineStorage() JSObject.h(437) : PropertyStorage JSC::JSObject::inlineStorage() frame #4: JSC::JSObject::inlineStorage frame #5: tryCreateObjectViaCloning (ObjectConstructorInlines.h:281) frame #6: globalFuncCloneObject (JSGlobalObjectFunctions.cpp:1006) ``` The same assert fires for a spread of an `S3Client.list()` result or one of its `contents` entries, for `const x = structuredClone({}); x.a = 1; ({ ...x })`, and for `const m = new (require("node:module").Module)("x"); m.exports.a = 1; ({ ...m.exports })`. Release builds are unaffected: the pointer returned by `inlineStorage()` is fed into a copy loop bounded by `inlineCapacity`, which is zero, so nothing is dereferenced. ## Cause Several bindings call `constructEmptyObject(globalObject, objectPrototype(), 0)` (either as a literal `0` or as a computed size that can be `0`), which yields a `JSFinalObject` with `inlineCapacity() == 0` and therefore `hasInlineStorage() == false`. Every property added to such an object lands out of line in the butterfly. The object-spread fast path `tryCreateObjectViaCloning` in the current WebKit build calls `source->inlineStorage()` unconditionally to seed `createWithButterflyCopyingInlineStorage`, and `inlineStorage()` asserts `hasInlineStorage()` in debug builds. No object produced by JavaScript has zero inline capacity (`{}` gets `JSFinalObject::defaultInlineCapacity`, and `ObjectAllocationProfile` asserts `inlineCapacity > 0`), so only host-created objects with an explicit `0` hint reach this state. ## Fix Treat a zero hint as "unsized" and route it through `constructEmptyObject(globalObject)`, which produces the same `defaultInlineCapacity` structure as a JS `{}` literal and gives the first handful of properties inline slots instead of forcing a butterfly. - `JSC__JSValue__createEmptyObject` and `JSC__JSObject__create` (the two Rust FFI entry points) gain an `if (!initialCapacity)` branch. This covers every Rust caller of `JSValue::create_empty_object(_, 0)` (25 call sites on main, `Bun.spawnSync` among them) and `JSObject::create_with_initializer(_, _, 0)`. The non-zero branch now clamps the `size_t` hint to `maxInlineCapacity` before it narrows to `unsigned`, so a hint that is a multiple of 2^32 cannot narrow to zero either. - The direct C++ callers that pass `0` (literal or computed) switch to the default-capacity overload: `SQLClient.cpp` row fallback, `ZigGlobalObject.cpp` worker serialized-env when empty (Windows only on current main), `bindings.cpp` `SystemError` `info`, `NodeModuleModule.cpp` `new Module().exports`, the two `SerializedScriptValue.cpp` fast-path deserializer sites (`structuredClone({})` / `structuredClone([{}])`), the `_NativeModule.h` `INIT_NATIVE_MODULE` macro (applied to the uncached construction), the `JSEnvironmentVariableMap.cpp` `process.env` target when `count == 0`, and the `JSONRowsToJS.cpp` object builder when the parsed object has no properties (reaches user code through `Bun.JSONC.parse`, `Bun.TOML.parse`, JSON5, and XML; this file landed on main after the first audit and review caught it). Two sites from the first version of this PR are gone after the merge with main: `ProcessBindingUV.cpp` got the same fix in oven-sh#34660, and the two `NodeHTTP.cpp` `assignHeaders*` sites were removed when request headers became lazy. The remaining computed-capacity `constructEmptyObject(.*objectPrototype(),` sites in `src/` are `fromEntries` (already guarded), `JSFetchHeaders.cpp` (early-returns on `size == 0`), `JSDOMFormData.cpp` / `JSURLSearchParams.cpp` (`size + 1`), and `JSSQLStatement.cpp` (`initializeColumnNames` returns early when `count < 1`). Everything else passes a literal nonzero constant. ## Verification New spread tests guarding each confirmed-aborting surface: - `test/js/bun/spawn/spawnSync.test.ts`: subprocess spreads the `Bun.spawnSync` result. - `test/js/bun/s3/s3-list-objects.test.ts`: subprocess spreads the top-level `list()` result, a `contents` entry, its `owner`, and a `commonPrefixes` entry. - `test/js/node/module/node-module-module.test.js`: subprocess spreads `new Module().exports` after a put. - `test/js/web/structured-clone-fastpath.test.ts`: spreads `structuredClone({})` and `structuredClone([{}])[0]` after adding a property. - `test/js/node/process-binding.test.ts`: spreads `process.binding("uv")` (the fix for this one is already on main, the test pins it). - `test/js/sql/sql.test.ts`: spreads the wide-row result on the null-structure fallback path. - `test/js/bun/jsonc/jsonc.test.ts`: spreads a parsed nested empty `{}` (JSONC) and an empty TOML table after adding a property. Each aborts (or, for the subprocess tests, produces empty stdout) against main's `src/` and passes with the fix. The assertion only exists in debug builds of JavaScriptCore, so these tests cannot fail against a release binary. The remaining sites are defensive rather than confirmed aborts and ship without a dedicated test: - `SystemError` `info`: its `DontDelete` properties bail `checkStructureForClone`. - `JSC__JSObject__create`: its sole caller early-returns through the guarded helper when the count is 0. - Worker serialized-env: the object is consumed before user code can spread it. - `INIT_NATIVE_MODULE(0)` (`node:constants`): 216 default-attribute properties push the structure to `Dictionary`, which bails the fast path. - `JSEnvironmentVariableMap.cpp` `process.env` target: unconditional `CustomAccessor` puts for TZ et al. bail `checkStructureForClone`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/module/node-module-module.test.js, test/js/bun/spawn/spawnSync.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Sep 14, 2026
…back closes the session (oven-sh#42647) ### Problem - A server `RST_STREAM` can make the HTTP/2 fetch client read and free a stream twice. A debug ASAN build of main (09bb546) reports `AddressSanitizer: heap-use-after-free` in `ClientSession::handle_data` (`src/http/h2_client/ClientSession.rs:848`, the `s.state != StreamState::Closed` read). The stream was freed by `drop_stream` in `ClientSession::fail_streams` (`ClientSession.rs:943`). A release build has no report, only heap corruption. - It needs a request with a custom TLS context (for example `tls: { serverName }`) whose entry left the 60-entry context cache. That request then holds the last ref. Its terminal callback drops the context, the drop closes the session's socket, and `on_close` runs `fail_streams` inside the deliver loop. ### Fix - While `delivering` is set, `fail_streams` fails each client and leaves the streams in `streams` and `by_http_id`. The deliver loop already removes a stream that has no client, through `remove_stream`, so each stream is freed once. - Outside the loop `fail_streams` behaves as before. - Correct because the loop's tail is safe on a session that is already closed: `maybe_release` returns at the `registry_index` sentinel, and a write to the closed socket writes nothing. - Verified: `test/js/web/fetch/fetch-http2-client.test.ts` (two new ASAN-only tests, 67 pass). With `src/` at main the RST test fails. Also `fetch-http2-leak.test.ts` (7 pass) and `fetch-http2-adversarial.test.ts` (20 pass). ### Background - `ClientSession` is one HTTP/2 connection of the fetch client. `streams` maps a stream id to a heap `Stream`. Each `Stream` points at the `HTTPClient` of one `fetch()`. - `handle_data` parses frames, then delivers each ready stream to its client in a loop, with `delivering` set. A terminal delivery runs the result callback at once, on the HTTP thread. - Custom TLS contexts are refcounted. The cache holds one ref and each request holds one. <details><summary>Notes</summary> This is the part of oven-sh#31788 that main still needs. oven-sh#31788 was closed as stale, and all five of its `src/` files conflict with main now. - Its first defect, `abort_by_http_id` with no ref on the session, is fixed on main: since oven-sh#37870 every entry point goes through `ClientSession::enter`, which holds a `RefPtr`. The abort test from oven-sh#31788 passes on main. It is included here because main has no test that evicts a TLS context. - Its third part, a `ctx` field on `PendingConnect`, has no reproduction and is not included. ASAN report on main, trimmed: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 thread T2 (HTTP Client) #0 <State as PartialEq>::eq src/http/h2_client/Stream.rs:104 #2 ClientSession::handle_data src/http/h2_client/ClientSession.rs:848 #4 ClientSession::enter src/http/h2_client/ClientSession.rs:242 #6 Handler<true>::on_data src/http/HTTPContext.rs:1386 freed by thread T2 (HTTP Client) here: #12 drop_stream src/http/h2_client/ClientSession.rs:211 #13 ClientSession::fail_streams src/http/h2_client/ClientSession.rs:943 ``` The two tests share one helper. The child holds a stream open on a context made by `serverName`, runs 61 more TLS configs to evict it, writes `evicted` to stderr, and then ends the held request: one test aborts it, the other has the server send `RST_STREAM(CANCEL)`. Both check that a later `fetch()` still works. They run only under ASAN and carry a 30 s timeout, because each child makes 62 TLS handshakes and a regression needs time to print its report. Without the longer timeout the failing run on main shows a timeout at 5 s, not the report. The tests are serial on purpose: each one fills the context cache, which would evict the context that a concurrent test depends on. One older test in the same file changed: `concurrent requests multiplex on one h2 session`. Its server held each stream for 100 ms and the test expected 8 open at once. On a loaded machine it saw 7 (2 of 9 runs of the file). The server now answers when the eighth stream is open, so the test uses no timer. After the change the file passed 4 of 4 runs on the same loaded machine. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1309.07ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [1215.67ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1497.63ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [730.67ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [2353.52ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [2514.64ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [927.77 ... (truncated) release without fix: 2 skipped bun test v1.4.3-canary.1 (f92be71) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [110.34ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [115.04ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response trailers are consumed without breaking the body [99.30ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > connection-specific request headers are stripped before HPACK [111.10ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > cold-start: parallel requests coalesce onto one TLS connect [135.66ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [147.00ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [160.06ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > keep-alive: sequential requests reuse one h2 session [139.51ms] (pass) fetch() over H ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1532.08ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [1265.37ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1731.76ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [501.02ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [1802.42ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1743.84ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent ReadableStream uploads route each chunk to its own stream ... (truncated) release with fix: 2 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 710ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output) �[1m�[92m Compiling�[0m bun_clap v0.0.0 (/workspace/bun/src/clap) �[1m�[92m Compiling�[0m bu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/http/h2_client/ClientSession.rs | 12 ++- test/js/web/fetch/fetch-http2-client.test.ts | 118 ++++++++++++++++++++++++--- 2 files changed, 117 insertions(+), 13 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/http/h2_client/ClientSession.rs 2 1 24 test/js/web/fetch/fetch-http2-client.test.ts 1 2 24 ``` </details> <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OHOS sandbox restrictions cause open() to return EACCES on ancestor directories like / and /storage/, which made the resolver's dir_info walk fail entirely, causing bun . and bun run to
silently exit with code 1 and no error output; this fix skips unreadable ancestors and continues processing child directories, restoring normal directory resolution.