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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/jsc/array_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,8 @@ impl ArrayBuffer {

pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> ArrayBuffer {
ArrayBuffer {
len: u32::try_from(bytes.len()).expect("int cast") as usize,
byte_len: u32::try_from(bytes.len()).expect("int cast") as usize,
len: bytes.len(),
byte_len: bytes.len(),
typed_array_type,
ptr: bytes.as_mut_ptr(),
..Default::default()
Expand All @@ -408,8 +408,8 @@ impl ArrayBuffer {
// this is an FFI hand-off, not a leak.
let ptr = bun_core::heap::into_raw(bytes).cast::<u8>();
ArrayBuffer {
len: u32::try_from(len).expect("int cast") as usize,
byte_len: u32::try_from(len).expect("int cast") as usize,
len,
byte_len: len,
typed_array_type,
ptr,
..Default::default()
Expand Down Expand Up @@ -876,6 +876,20 @@ impl MarkedArrayBuffer {
})
}

/// For in-place writes: re-read from `value` when `self.buffer.ptr` is an
/// owned snapshot (see `StringOrBuffer::array_buffer_into`).
Comment thread
robobun marked this conversation as resolved.
#[inline]
pub fn live_array_buffer(&self, global: &JSGlobalObject) -> ArrayBuffer {
if self.owns_buffer && self.buffer.value != JSValue::ZERO {
return self
.buffer
.value
.as_array_buffer(global)
.unwrap_or_default();
}
self.buffer
}

pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer {
MarkedArrayBuffer {
buffer: ArrayBuffer::from_bytes(bytes, typed_array_type),
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/api/MarkdownObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ impl PinnedView {
let Some(b) = buffer.buffer() else {
return Ok(None);
};
if b.owns_buffer {
return Ok(None);
}
match b.buffer.value.as_pinned_arraybuffer(global) {
Some(pinned) => Ok(Some(Self(pinned))),
None => Err(global.throw_out_of_memory()),
Expand Down
10 changes: 5 additions & 5 deletions src/runtime/crypto/CryptoHasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ impl CryptoHasher {

if let Some(string_or_buffer) = output {
if let StringOrBuffer::Buffer(buffer) = &string_or_buffer {
let ab = buffer.buffer;
let ab = buffer.live_array_buffer(global);
return Self::hash_to_bytes(global, &mut evp, input, Some(ab));
}
// `inline else => |*str|` — every non-buffer arm yields a string-like
Expand Down Expand Up @@ -687,7 +687,7 @@ impl CryptoHasher {
) -> JsResult<JSValue> {
if let Some(string_or_buffer) = output {
if let StringOrBuffer::Buffer(buffer) = &string_or_buffer {
let ab = buffer.buffer;
let ab = buffer.live_array_buffer(global);
return this.digest_to_bytes(global, Some(ab));
}
// `defer str.deinit()` — handled by Drop.
Expand Down Expand Up @@ -927,7 +927,7 @@ impl CryptoHasherZig {
) -> JsResult<JSValue> {
if let Some(string_or_buffer) = output {
if let StringOrBuffer::Buffer(buffer) = &string_or_buffer {
let ab = buffer.buffer;
let ab = buffer.live_array_buffer(global);
return Self::hash_by_name_inner_to_bytes::<A>(global, input, Some(ab));
}
let Some(encoding) = Encoding::from(string_or_buffer.slice()) else {
Expand Down Expand Up @@ -1374,7 +1374,7 @@ impl<H: StaticHasher> StaticCryptoHasher<H> {

if let Some(string_or_buffer) = output {
if let StringOrBuffer::Buffer(buffer) = &string_or_buffer {
let ab = buffer.buffer;
let ab = buffer.live_array_buffer(global);
return Self::hash_to_bytes(global, input, Some(ab));
}
let Some(encoding) = Encoding::from(string_or_buffer.slice()) else {
Expand Down Expand Up @@ -1464,7 +1464,7 @@ impl<H: StaticHasher> StaticCryptoHasher<H> {
}
if let Some(string_or_buffer) = output {
if let StringOrBuffer::Buffer(buffer) = &string_or_buffer {
let ab = buffer.buffer;
let ab = buffer.live_array_buffer(global);
return this.digest_to_bytes(global, Some(ab));
}
let Some(encoding) = Encoding::from(string_or_buffer.slice()) else {
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3891,7 +3891,9 @@ pub mod args {
}
}
}
if arguments.will_be_async && matches!(args.buffer, StringOrBuffer::Buffer(_)) {
if arguments.will_be_async
&& matches!(&args.buffer, StringOrBuffer::Buffer(b) if !b.owns_buffer)
{
if let Some(pinned) = bv.as_pinned_arraybuffer(ctx) {
args.buffer = StringOrBuffer::Buffer(Buffer {
buffer: pinned,
Expand Down
90 changes: 64 additions & 26 deletions src/runtime/node/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,28 @@
Self::EncodedSlice(_encoded) => {
// ZigStringSlice has Drop; cleanup is implicit.
}
Self::Buffer(_) => {}
Self::Buffer(buffer) => {
buffer.destroy();
}
Comment thread
robobun marked this conversation as resolved.
}
}
}

#[cold]
#[inline(never)]
fn snapshot_resizable(global: &JSGlobalObject, ab: &jsc::ArrayBuffer) -> Buffer {
let bytes = ab.byte_slice();
let mut owned = if bytes.is_empty() {
Buffer::EMPTY
} else {
global.vm().report_extra_memory(bytes.len());
bun_core::handle_oom(Buffer::from_string(bytes))
};
owned.buffer.value = ab.value;
owned.buffer.typed_array_type = ab.typed_array_type;
owned
}

impl bun_jsc::Unprotect for BlobOrStringOrBuffer {
/// JS-side half of cleanup — owned
/// payloads are released by `Drop` (which runs next when held in a
Expand Down Expand Up @@ -356,7 +373,9 @@
if buffer.buffer.value != JSValue::ZERO {
return Ok(buffer.buffer.value);
}
Ok(buffer.to_node_buffer(ctx))
let js = buffer.to_node_buffer(ctx);
buffer.owns_buffer = false;
Ok(js)
}
}
}
Expand All @@ -371,6 +390,40 @@
}
}

/// `pin()` guards `transfer()` but not `ArrayBuffer.prototype.resize()`,
/// so resizable non-shared inputs are snapshotted (growable SAB only grows
/// in-place; captured extent stays readable). `snapshot_volatile = false`
/// opts out for callers that run no more user JS before reading.

Check warning on line 396 in src/runtime/node/types.rs

View check run for this annotation

Claude / Claude Code Review

VectorArrayBuffer (fs.writev/readv) has the same resize() hole; not listed as an exclusion

🟡 `VectorArrayBuffer::from_js` (same file, used by `fs.writev`/`fs.readv` via `FdVectorIo::from_js` at node_fs.rs:2817) has the identical `resize()` hole this PR fixes for `StringOrBuffer`: `Bun__JSArray__collectBufferSpans` pins each element then captures `(view->vector(), view->byteLength())` into an iovec with no resizable check, so `buf.buffer.resize(0)` after `fs.writev(fd, [buf], cb)` returns mprotects the pages `PROT_NONE` and the threadpool `writev` SIGSEGVs — the direct analog of the `f
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
#[inline]
fn array_buffer_into(
out: &mut Self,
global: &JSGlobalObject,
value: JSValue,
is_async: bool,
snapshot_volatile: bool,
) {
let ab = value.as_array_buffer(global).unwrap_or_default();
let buffer = if snapshot_volatile && ab.resizable && !ab.shared {
snapshot_resizable(global, &ab)
Comment thread
robobun marked this conversation as resolved.
} else if is_async {
Buffer::from_js_pinned(global, value).unwrap_or(Buffer {
buffer: ab,
owns_buffer: false,
pinned: false,
})
} else {
Buffer {
buffer: ab,
owns_buffer: false,
pinned: false,
}
};
if is_async {
buffer.buffer.value.protect();
}
*out = Self::Buffer(buffer);
}

/// Out-param core of [`from_js_maybe_async`]. Writes the decoded payload
/// directly into `*out` and returns
/// `Ok(true)` on success, `Ok(false)` if `value` is not a string/buffer
Expand Down Expand Up @@ -437,18 +490,7 @@
| JSType::BigInt64Array
| JSType::BigUint64Array
| JSType::DataView => {
let buffer = if is_async {
Buffer::from_js_pinned(global, value)
.unwrap_or_else(|| Buffer::from_array_buffer(global, value))
} else {
Buffer::from_array_buffer(global, value)
};

if is_async {
buffer.buffer.value.protect();
}

*out = Self::Buffer(buffer);
Self::array_buffer_into(out, global, value, is_async, true);
Ok(true)
}
_ => Ok(false),
Expand Down Expand Up @@ -484,15 +526,18 @@
Self::from_js_with_encoding_maybe_async(global, value, encoding, false, true)
}

/// Out-param convenience wrapper — see [`from_js_with_encoding_maybe_async_into`].
/// Out-param wrapper for `NodeHTTPResponse`; it evaluates encoding/callback
/// before capture and spills resizable tails itself (`snapshot_volatile=false`).
Comment thread
robobun marked this conversation as resolved.
#[inline]
pub fn from_js_with_encoding_into(
out: &mut StringOrBuffer,
global: &JSGlobalObject,
value: JSValue,
encoding: Encoding,
) -> JsResult<bool> {
Self::from_js_with_encoding_maybe_async_into(out, global, value, encoding, false, true)
Self::from_js_with_encoding_maybe_async_into(
out, global, value, encoding, false, true, false,
)
}

/// Out-param core of [`from_js_with_encoding_maybe_async`]. Writes into
Expand All @@ -506,18 +551,10 @@
encoding: Encoding,
is_async: bool,
allow_string_object: bool,
snapshot_volatile: bool,
) -> JsResult<bool> {
if value.is_cell() && value.js_type().is_array_buffer_like() {
let buffer = if is_async {
Buffer::from_js_pinned(global, value)
.unwrap_or_else(|| Buffer::from_array_buffer(global, value))
} else {
Buffer::from_array_buffer(global, value)
};
if is_async {
buffer.buffer.value.protect();
}
*out = Self::Buffer(buffer);
Self::array_buffer_into(out, global, value, is_async, snapshot_volatile);
return Ok(true);
}

Expand Down Expand Up @@ -570,6 +607,7 @@
encoding,
is_async,
allow_string_object,
true,
)? {
Ok(Some(out))
} else {
Expand Down
9 changes: 1 addition & 8 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4032,15 +4032,8 @@ impl FormDataContext<'_> {
let js_err = err.to_js(global_this);
let _ = global_this.throw_value(js_err);
}
Ok(mut result) => {
Ok(result) => {
joiner.push_cloned(result.slice());
// StringOrBuffer::Drop is a no-op for Buffer; release
// the readFile allocation explicitly.
if let crate::node::types::StringOrBuffer::Buffer(buf) =
&mut result
{
buf.destroy();
}
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1814,16 +1814,11 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
body.detach();
return Ok(rejected_value);
}
Ok(mut result) => {
Ok(result) => {
body.detach();
body = HTTPRequestBody::AnyBlob(blob::Any::from_owned_slice(
result.slice().to_vec(),
));
// StringOrBuffer::Drop is a no-op for Buffer; release the
// readFile allocation now that the bytes are copied out.
if let crate::node::types::StringOrBuffer::Buffer(buf) = &mut result {
buf.destroy();
}
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions test/js/bun/md/md-render-callback.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

const Markdown = Bun.markdown;

Expand Down Expand Up @@ -426,4 +427,20 @@ describe("Bun.markdown buffer input", () => {
input.buffer.transfer();
expect((input.buffer as ArrayBuffer).detached).toBe(true);
});

test("a resizable input is read at call time even if an option getter resizes it to 0", async () => {
const script = `
const bytes = new TextEncoder().encode("# Hello\\n\\nworld\\n" + Buffer.alloc(1 << 16, 0x20).toString());
const input = new Uint8Array(new ArrayBuffer(bytes.length, { maxByteLength: bytes.length }));
input.set(bytes);
const fixed = Bun.markdown.html(Buffer.from(bytes), { autolinks: false });
const out = Bun.markdown.html(input, { get autolinks() { input.buffer.resize(0); return false; } });
console.log(out === fixed ? "OK" : "MISMATCH " + JSON.stringify(out.slice(0, 200)));
`;
await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: bunEnv, stdout: "pipe", stderr: "pipe" });
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout.trim()).toBe("OK");
expect(exitCode).toBe(0);
});
});
Loading
Loading