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
37 changes: 19 additions & 18 deletions src/runtime/api/bun/subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,34 +1119,35 @@ impl Subprocess<'_> {
did_update_has_pending_activity = true;
}

match status {
Status::Exited(exited) => {
let _ = promise
.as_any_promise()
.unwrap()
.resolve(global_this, JSValue::js_number(exited.code as f64));
// TODO: properly propagate exception upwards
}
let settled = match status {
Status::Exited(exited) => promise
.as_any_promise()
.unwrap()
.resolve(global_this, JSValue::js_number(exited.code as f64)),
Status::Err(err) => {
let js_err = err.to_js(global_this);
let _ = promise
promise
.as_any_promise()
.unwrap()
.reject_with_async_stack(global_this, js_err);
// TODO: properly propagate exception upwards
}
Status::Signaled(signaled) => {
let _ = promise.as_any_promise().unwrap().resolve(
global_this,
JSValue::js_number(128u8.wrapping_add(*signaled) as f64),
);
// TODO: properly propagate exception upwards
.reject_with_async_stack(global_this, js_err)
}
Status::Signaled(signaled) => promise.as_any_promise().unwrap().resolve(
global_this,
JSValue::js_number(128u8.wrapping_add(*signaled) as f64),
),
_ => {
// crash in debug mode
#[cfg(debug_assertions)]
unreachable!();
#[cfg(not(debug_assertions))]
Ok(())
}
};
if settled.is_err() {
// A failed settle leaves an exception pending on the VM;
// the exit callback below must not run with it (the
// error label erases Thrown, so probe the VM instead).
global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown);
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/runtime/valkey_jsc/js_valkey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,8 +1165,11 @@ impl JSValkeyClient {
.expect("unreachable");
let len = start - cur.len();
let msg = &buf[..len];
let _ = self.client_fail(msg, protocol::RedisError::IdleTimeout);
// TODO: properly propagate exception upwards
if let Err(e) = self.client_fail(msg, protocol::RedisError::IdleTimeout) {
// A failed settle inside `fail` leaves an exception pending
// on the VM; this timer callback has nowhere to bubble it.
self.global_object.report_active_exception_as_unhandled(e);
}
}
valkey::Status::Disconnected | valkey::Status::Connecting => {
use std::io::Write;
Expand All @@ -1180,8 +1183,10 @@ impl JSValkeyClient {
.expect("unreachable");
let len = start - cur.len();
let msg = &buf[..len];
let _ = self.client_fail(msg, protocol::RedisError::ConnectionTimeout);
// TODO: properly propagate exception upwards
if let Err(e) = self.client_fail(msg, protocol::RedisError::ConnectionTimeout) {
// See the idle-timeout arm above.
self.global_object.report_active_exception_as_unhandled(e);
}
}
}
}
Expand Down
16 changes: 11 additions & 5 deletions src/runtime/webcore/ByteStream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,11 +620,17 @@ impl ByteStream {

if let Some(mut action) = self.buffer_action.replace(None) {
let global = self.parent_const().global_this();
// TODO: properly propagate exception upwards
let _ = action.reject(
global,
&streams::StreamError::AbortReason(jsc::CommonAbortReason::UserAbort),
);
if action
.reject(
global,
&streams::StreamError::AbortReason(jsc::CommonAbortReason::UserAbort),
)
.is_err()
{
// A failed settle leaves an exception pending on the VM (the
// error label erases Thrown, so probe the VM instead).
global.report_active_exception_as_unhandled(jsc::JsError::Thrown);
}
self.buffer_action.set(None);
}
}
Expand Down
39 changes: 32 additions & 7 deletions src/runtime/webcore/blob/copy_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,14 @@ impl<'a> CopyFile<'a> {
if let Some(store) = self.store.take() {
drop(store); // deref()
}
promise.reject(global_this, Ok(instance))
if promise.reject(global_this, Ok(instance)).is_err() {
// The settle error label erases Thrown, so probe the VM: report a
// pending non-termination exception instead of letting the task
// channel treat it as termination, and keep real termination as
// the `Err` that unwinds the tick loop.
return jsc::task::report_error_or_terminate(global_this, jsc::JsError::Thrown);
}
Ok(())
}

pub(crate) fn then(&mut self, promise: &mut JSPromise) -> Result<(), jsc::JsTerminated> {
Expand All @@ -147,10 +154,17 @@ impl<'a> CopyFile<'a> {
return self.reject(promise);
}

promise.resolve(
self.global_this,
JSValue::js_number_from_uint64(self.read_len as u64),
)
if promise
.resolve(
self.global_this,
JSValue::js_number_from_uint64(self.read_len as u64),
)
.is_err()
{
// See `reject` above.
return jsc::task::report_error_or_terminate(self.global_this, jsc::JsError::Thrown);
}
Ok(())
}

#[cfg(not(windows))]
Expand Down Expand Up @@ -1642,7 +1656,12 @@ impl<'a> CopyFileWindows<'a> {
// SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward.
unsafe { Self::destroy(core::ptr::from_mut(self)) };
// `promise` points to a GC-owned `JSPromise` cell, not into `self`; valid after `destroy`.
let _ = promise.reject(global_this, err_instance); // TODO: properly propagate exception upwards
if promise.reject(global_this, err_instance).is_err() {
// A failed settle leaves an exception pending on the VM; report it
// so it cannot ride the tick into unrelated JS (the error label
// erases Thrown, so probe the VM instead).
global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown);
}
}

pub(crate) fn on_complete(&mut self, written_actual: usize) {
Expand Down Expand Up @@ -1729,7 +1748,13 @@ impl<'a> CopyFileWindows<'a> {
// SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward.
unsafe { Self::destroy(core::ptr::from_mut(self)) };
// `promise` points to a GC-owned `JSPromise` cell, not into `self`; valid after `destroy`.
let _ = promise.resolve(global_this, JSValue::js_number_from_uint64(written as u64)); // TODO: properly propagate exception upwards
if promise
.resolve(global_this, JSValue::js_number_from_uint64(written as u64))
.is_err()
{
// See `throw` above: keep a failed settle's exception off the tick.
global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown);
}
}

#[cold]
Expand Down
52 changes: 39 additions & 13 deletions src/runtime/webcore/blob/write_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,9 @@ mod windows_impl {
)
} {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
return;
}
Expand All @@ -940,7 +942,9 @@ mod windows_impl {
if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } {
match e {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
}
}
Expand Down Expand Up @@ -989,7 +993,9 @@ mod windows_impl {
// SAFETY: caller contract — `this` is live; `throw` consumes it.
match unsafe { Self::throw(this, err_) } {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
return;
}
Expand All @@ -998,14 +1004,16 @@ mod windows_impl {
if let Err(e) = unsafe { Self::open(this) } {
match e {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
}
}

/// `ManagedTask`-shaped trampoline for [`on_mkdirp_complete`]: takes
/// `*mut Self` and returns the event-loop `JsResult<()>` (always `Ok`;
/// the inner body already swallows `JSTerminated`).
/// a failed settle is reported inside the completion callback).
fn on_mkdirp_complete_task(this: *mut WriteFileWindows) -> bun_event_loop::JsResult<()> {
// SAFETY: `this` is the live Box-allocated `WriteFileWindows` whose
// pointer was stashed in `on_mkdirp_complete_concurrent` below;
Expand Down Expand Up @@ -1059,7 +1067,9 @@ mod windows_impl {
)
} {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
return;
}
Expand All @@ -1070,7 +1080,9 @@ mod windows_impl {
if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } {
match e {
WriteFileWindowsError::WriteFileWindowsDeinitialized => {}
WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards
// The completion callback reported any failed settle;
// JSTerminated is real termination, left pending.
WriteFileWindowsError::JSTerminated => {}
}
}
}
Expand Down Expand Up @@ -1303,22 +1315,28 @@ impl WriteFilePromise {
// SAFETY: GC-owned cell (kept alive below); scoped shared access.
let value = unsafe { (*promise).to_js() };
value.ensure_still_alive();
match count {
let settled = match count {
WriteFileResultType::Err(err) => {
// SAFETY: GC-owned cell; the error build's shared borrow ends before the
// scoped exclusive `reject` borrow.
unsafe {
let err_js = err.to_error_instance_with_async_stack(global_this, &*promise);
(*promise).reject(global_this, Ok(err_js))?;
(*promise).reject(global_this, Ok(err_js))
}
}
WriteFileResultType::Result(wrote) => {
// SAFETY: GC-owned cell; exclusive borrow scoped to the call.
unsafe {
(*promise)
.resolve(global_this, JSValue::js_number_from_uint64(wrote as u64))?;
(*promise).resolve(global_this, JSValue::js_number_from_uint64(wrote as u64))
}
}
};
if settled.is_err() {
// The settle error label erases Thrown, so probe the VM: report a
// pending non-termination exception instead of letting the task
// channel treat it as termination, and keep real termination as
// the `Err` that unwinds the tick loop.
return jsc::task::report_error_or_terminate(global_this, jsc::JsError::Thrown);
}
Ok(())
}
Expand All @@ -1343,8 +1361,16 @@ impl WriteFileWaitFromLockedValueTask {
let this = unsafe {
bun_core::heap::take(this.cast::<WriteFileWaitFromLockedValueTask>().as_ptr())
};
let _ = Self::then(this, value);
// TODO: properly propagate exception upwards
// `BackRef` is `Copy`; keep the global past the consuming call.
let global_ref = this.global_this;
if Self::then(this, value).is_err() {
// A failed settle leaves an exception pending on the VM; report it
// so it cannot ride the tick into unrelated JS (the error label
// erases Thrown, so probe the VM instead).
global_ref
.get()
.report_active_exception_as_unhandled(jsc::JsError::Thrown);
}
}

pub(crate) fn then(
Expand Down
17 changes: 14 additions & 3 deletions src/runtime/webcore/s3/multipart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ impl MultiPartUpload {
self.state.set(State::SinglefileStarted);
// we can do only 1 request
self.ref_();
let _ = execute_simple_s3_request(
let started = execute_simple_s3_request(
&self.credentials,
s3_simple_request::S3RequestOptions {
path: &self.path,
Expand All @@ -1007,10 +1007,21 @@ impl MultiPartUpload {
},
s3_simple_request::S3Callback::Upload(Self::single_send_upload_response),
self.as_ctx_ptr(),
); // TODO: properly propagate exception upwards
);
if started.is_err() {
// A failed settle leaves an exception pending on the VM; report
// it so it cannot ride the tick into unrelated JS (the error
// label erases Thrown, so probe the VM instead).
self.global_this
.report_active_exception_as_unhandled(bun_jsc::JsError::Thrown);
}
} else {
// we need to split
let _ = self.process_multi_part(part_size); // TODO: properly propagate exception upwards
if self.process_multi_part(part_size).is_err() {
// See the single-request arm above.
self.global_this
.report_active_exception_as_unhandled(bun_jsc::JsError::Thrown);
}
}
}

Expand Down
61 changes: 61 additions & 0 deletions test/js/bun/io/bun-write.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -943,4 +943,65 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) {

expect(f.name).toBe(filePath);
});

// The native write completions (copy_file, write_file, and the locked-body
// wait task) settle their promises from the event loop and must never leave
// an exception riding the tick. Pin both paths with a hostile
// `Object.prototype.then` accessor installed, mirroring the
// prototype-pollution state fuzzer processes run in.
it("write completions settle under Object.prototype.then pollution", async () => {
using dir = tempDir("bun-write-pollution", { "src.txt": "copy me" });
const fixture = `
const fs = require("fs");
const dir = ${JSON.stringify(String(dir))};
const server = Bun.serve({
port: 0,
async fetch() {
return new Response(new ReadableStream({
async pull(c) {
c.enqueue(new TextEncoder().encode("hello "));
await Bun.sleep(10);
c.enqueue(new TextEncoder().encode("world"));
c.close();
},
}));
},
});
const resp = await fetch("http://localhost:" + server.port + "/");

Object.defineProperty(Object.prototype, "then", {
configurable: true,
get() {
throw new Error("boom");
},
});
// Body still streaming: Bun.write waits on the locked body, then
// resolves its promise with the inner write's promise.
const wroteStream = await Bun.write(dir + "/a.txt", resp);
// copy_file completion. The resolved count is backend-dependent on
// Windows (uv_fs_copyfile reports none, the fallback loop the real
// count), so pin only its type; the content check below is the proof.
const wroteCopy = await Bun.write(Bun.file(dir + "/dst.txt"), Bun.file(dir + "/src.txt"));
delete Object.prototype.then;

const results = [
wroteStream,
fs.readFileSync(dir + "/a.txt", "utf8"),
typeof wroteCopy,
fs.readFileSync(dir + "/dst.txt", "utf8"),
];
server.stop(true);
console.log(JSON.stringify(results));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", fixture],
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(JSON.parse(stdout)).toEqual([11, "hello world", "number", "copy me"]);
expect(exitCode).toBe(0);
});
});
Loading
Loading