From 7f4bcd3fe7b74f64b9474f3007ea2603132e6991 Mon Sep 17 00:00:00 2001 From: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> Date: Sun, 31 May 2026 19:45:32 -0400 Subject: [PATCH] text-decoder: snapshot shared buffers before decoding --- src/runtime/webcore/TextDecoder.rs | 56 +++++++--- src/runtime/webcore/encoding.classes.ts | 5 - test/js/web/encoding/text-decoder.test.js | 130 ++++++++++++++++++++++ 3 files changed, 171 insertions(+), 20 deletions(-) diff --git a/src/runtime/webcore/TextDecoder.rs b/src/runtime/webcore/TextDecoder.rs index aa9e7039f8ec..ddfd3093d22a 100644 --- a/src/runtime/webcore/TextDecoder.rs +++ b/src/runtime/webcore/TextDecoder.rs @@ -1,7 +1,5 @@ use crate::webcore::EncodingLabel; -use crate::webcore::jsc::{ - self as jsc, CallFrame, JSGlobalObject, JSUint8Array, JSValue, JsResult, -}; +use crate::webcore::jsc::{self as jsc, CallFrame, JSGlobalObject, JSValue, JsResult}; use bun_core::AllocError; use bun_core::{OwnedString, strings}; use core::cell::Cell; @@ -231,7 +229,18 @@ impl TextDecoder { } if let Some(ab) = arguments[0].as_array_buffer(global_this) { - array_buffer = ab; + // Snapshot SharedArrayBuffer / growable SharedArrayBuffer / + // resizable ArrayBuffer inputs before borrowing: another agent can + // mutate shared memory (or the owner can resize the backing store) + // while Rust holds the `&[u8]`, which is undefined behavior under + // Rust's shared-slice aliasing rules. The copy runs in C++, so Rust + // never reads the shared source through a slice. Fixed, unshared + // inputs keep the zero-copy borrowed path. + array_buffer = if ab.shared || ab.resizable { + snapshot_shared_array_buffer(global_this, &ab)? + } else { + ab + }; break 'input_slice array_buffer.slice(); } @@ -248,17 +257,6 @@ impl TextDecoder { } } - /// DOMJIT fast path for `decode(typedArray)` called with no options object. - /// A no-options decode is flushing per WHATWG Encoding, matching the slow - /// path in `decode()` when `stream` is absent. - pub fn decode_without_type_checks( - &self, - global_this: &JSGlobalObject, - uint8array: &mut JSUint8Array, - ) -> JsResult { - self.decode_slice::(global_this, uint8array.slice()) - } - fn decode_slice( &self, global_this: &JSGlobalObject, @@ -560,3 +558,31 @@ impl TextDecoder { Ok(bun_core::heap::into_raw(TextDecoder::new(decoder))) } } + +/// Copy a `SharedArrayBuffer`, growable `SharedArrayBuffer`, or resizable +/// `ArrayBuffer` input into a fresh, non-shared `ArrayBuffer` so the decoder +/// never constructs a `&[u8]` over memory another agent can mutate. The copy is +/// performed in C++ (`Bun__createArrayBufferForCopy`), so Rust never reads the +/// shared source bytes through a slice. The returned `ArrayBuffer` owns the +/// copy's JS value, keeping it alive for the duration of the borrow. +fn snapshot_shared_array_buffer( + global_this: &JSGlobalObject, + array_buffer: &jsc::ArrayBuffer, +) -> JsResult { + let copy = bun_jsc::from_js_host_call(global_this, || unsafe { + Bun__createArrayBufferForCopy(global_this, array_buffer.ptr.cast(), array_buffer.byte_len) + })?; + copy.as_array_buffer(global_this).ok_or_else(|| { + global_this.throw_invalid_arguments(format_args!( + "Failed to snapshot shared ArrayBuffer for TextDecoder.decode", + )) + }) +} + +unsafe extern "C" { + fn Bun__createArrayBufferForCopy( + global_this: *const JSGlobalObject, + ptr: *const core::ffi::c_void, + len: usize, + ) -> JSValue; +} diff --git a/src/runtime/webcore/encoding.classes.ts b/src/runtime/webcore/encoding.classes.ts index b26078ccc1fa..d56d83c6d369 100644 --- a/src/runtime/webcore/encoding.classes.ts +++ b/src/runtime/webcore/encoding.classes.ts @@ -23,11 +23,6 @@ export default [ decode: { fn: "decode", length: 1, - - DOMJIT: { - returns: "JSString", - args: ["JSUint8Array"], - }, }, }, }), diff --git a/test/js/web/encoding/text-decoder.test.js b/test/js/web/encoding/text-decoder.test.js index e0d412027cf1..bf9c6748069c 100644 --- a/test/js/web/encoding/text-decoder.test.js +++ b/test/js/web/encoding/text-decoder.test.js @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; import { gc as gcTrace, isASAN, withoutAggressiveGC } from "harness"; +import { Worker } from "node:worker_threads"; const getByteLength = str => { // returns the byte length of an utf8 string @@ -658,3 +659,132 @@ it.each(["utf-16le", "utf-16be"])("TextDecoder(%s).decode() should not leak the // runs higher under bun-asan even with the fix; widen the threshold there. expect(deltaMiB).toBeLessThan(isASAN ? 128 : 48); }); + +describe("SharedArrayBuffer input", () => { + // decode() snapshots shared/resizable inputs before reading them, so another + // agent can't mutate the bytes underneath the decoder. These cases also pin + // the behavior-preserving contract: SharedArrayBuffer stays accepted (Node + // accepts it too) and fixed unshared inputs keep the zero-copy path. + it("decodes a raw SharedArrayBuffer", () => { + const sab = new SharedArrayBuffer(6); + new Uint8Array(sab).set([0x41, 0x42, 0x43, 0x44, 0x45, 0x46]); + expect(new TextDecoder().decode(sab)).toBe("ABCDEF"); + }); + + it("decodes a Uint8Array over a SharedArrayBuffer", () => { + const sab = new SharedArrayBuffer(6); + new Uint8Array(sab).set([0x41, 0x42, 0x43, 0x44, 0x45, 0x46]); + expect(new TextDecoder().decode(new Uint8Array(sab))).toBe("ABCDEF"); + }); + + it("decodes only the view range of an offset SharedArrayBuffer view", () => { + const sab = new SharedArrayBuffer(6); + new Uint8Array(sab).set([0x41, 0x42, 0x43, 0x44, 0x45, 0x46]); + expect(new TextDecoder().decode(new Uint8Array(sab, 2, 3))).toBe("CDE"); + }); + + it("decodes UTF-16LE over a SharedArrayBuffer view", () => { + const sab = new SharedArrayBuffer(6); + new Uint8Array(sab).set([0x61, 0x00, 0x62, 0x00, 0x63, 0x00]); + expect(new TextDecoder("utf-16le").decode(new Uint8Array(sab))).toBe("abc"); + }); + + it("decodes UTF-16BE over a SharedArrayBuffer view", () => { + const sab = new SharedArrayBuffer(6); + new Uint8Array(sab).set([0x00, 0x61, 0x00, 0x62, 0x00, 0x63]); + expect(new TextDecoder("utf-16be").decode(new Uint8Array(sab))).toBe("abc"); + }); + + it("decodes a zero-length SharedArrayBuffer to an empty string", () => { + expect(new TextDecoder().decode(new SharedArrayBuffer(0))).toBe(""); + const sab = new SharedArrayBuffer(8); + expect(new TextDecoder().decode(new Uint8Array(sab, 4, 0))).toBe(""); + }); + + const supportsGrowableSharedArrayBuffer = (() => { + try { + return new SharedArrayBuffer(8, { maxByteLength: 16 }).growable === true; + } catch { + return false; + } + })(); + + it.skipIf(!supportsGrowableSharedArrayBuffer)("decodes a growable SharedArrayBuffer", () => { + const gsab = new SharedArrayBuffer(6, { maxByteLength: 16 }); + new Uint8Array(gsab).set([0x41, 0x42, 0x43, 0x44, 0x45, 0x46]); + expect(new TextDecoder().decode(gsab)).toBe("ABCDEF"); + }); + + const supportsResizableArrayBuffer = (() => { + try { + return new ArrayBuffer(8, { maxByteLength: 16 }).resizable === true; + } catch { + return false; + } + })(); + + it.skipIf(!supportsResizableArrayBuffer)("decodes a resizable ArrayBuffer", () => { + const rab = new ArrayBuffer(6, { maxByteLength: 16 }); + new Uint8Array(rab).set([0x41, 0x42, 0x43, 0x44, 0x45, 0x46]); + expect(new TextDecoder().decode(rab)).toBe("ABCDEF"); + }); + + it("is safe while a worker concurrently mutates the SharedArrayBuffer", async () => { + const sab = new SharedArrayBuffer(64); + new Uint8Array(sab).fill(0x41); + // state[0]: 0 = wait, 1 = run, 2 = stop; state[1]: worker write counter. + const state = new Int32Array(new SharedArrayBuffer(8)); + + const worker = new Worker( + ` + const { workerData } = require("node:worker_threads"); + const bytes = new Uint8Array(workerData.sab); + const state = new Int32Array(workerData.state); + + while (Atomics.load(state, 0) === 0) Atomics.wait(state, 0, 0); + + let i = 0; + while (Atomics.load(state, 0) === 1) { + bytes[0] = i & 1 ? 0x41 : 0xc3; + bytes[1] = i & 1 ? 0x42 : 0x28; + Atomics.add(state, 1, 1); + i++; + } + `, + { eval: true, workerData: { sab, state: state.buffer } }, + ); + + // If the worker fails to start or throws, surface it instead of polling + // forever below. + let workerError; + worker.on("error", err => { + workerError = err; + }); + + Atomics.store(state, 0, 1); + Atomics.notify(state, 0); + + try { + // Wait until the worker is actively mutating before decoding. Poll the + // atomic write counter without blocking the test runner's main thread. + while (Atomics.load(state, 1) < 50) { + if (workerError) throw workerError; + await Bun.sleep(0); + } + + const decoder = new TextDecoder("utf-8", { fatal: false }); + for (let i = 0; i < 4000; i++) { + // Before the fix this reads a &[u8] over the mutating shared store and + // crashes the UTF-8 materializer; the snapshot makes every read stable. + expect(typeof decoder.decode(new Uint8Array(sab))).toBe("string"); + } + } finally { + // Always stop and join the worker, even if an assertion above threw. + Atomics.store(state, 0, 2); + await worker.terminate(); + } + + // The worker really did mutate concurrently with the decode loop. + expect(Atomics.load(state, 1)).toBeGreaterThan(50); + }); +});