Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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 @@ impl Drop for StringOrBuffer {
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 @@ impl StringOrBuffer {
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 @@ impl StringOrBuffer {
}
}

/// `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.
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 @@ impl StringOrBuffer {
| 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 @@ impl StringOrBuffer {
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 @@ impl StringOrBuffer {
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 @@ impl StringOrBuffer {
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
21 changes: 21 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,24 @@ 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 () => {
// `StringOrBuffer::from_js` used to borrow the view; the `autolinks` getter
// runs before the parser touches the bytes and can `resize(0)` the backing
// store out from under the borrow, mprotecting the trimmed pages. The
// funnel now snapshots resizable non-shared inputs.
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