diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 396f55991f9f..b53659f04513 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -3321,6 +3322,50 @@ extern "C" void napi_internal_check_gc(napi_env env) env->checkGC(); } +// Node.js does not gate napi_create_string_* behind NAPI_PREAMBLE, so a VM +// exception may already be pending. SuspendExceptionScope stashes it for the +// allocation and restores it on return. +template +static JSC::EncodedJSValue napiCreateStringWithSuspendedException(napi_env env, const CharT* ptr, size_t length, Maker make) +{ + auto& vm = env->vm(); + JSC::SuspendExceptionScope suspend(vm); + if (!length) { + return JSValue::encode(jsEmptyString(vm)); + } + auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); + WTF::String str = make(std::span(ptr, length)); + if (scope.exception() || str.isNull()) [[unlikely]] { + scope.clearExceptionExceptTermination(); + return {}; + } + return JSValue::encode(jsString(vm, WTF::move(str))); +} + +extern "C" JSC::EncodedJSValue napi_internal_create_string_latin1(napi_env env, const Latin1Character* ptr, size_t length) +{ + return napiCreateStringWithSuspendedException(env, ptr, length, [](std::span bytes) { + return WTF::String(bytes); + }); +} + +extern "C" JSC::EncodedJSValue napi_internal_create_string_utf8(napi_env env, const char* ptr, size_t length) +{ + return napiCreateStringWithSuspendedException(env, ptr, length, [](std::span bytes) { + if (simdutf::validate_ascii(bytes.data(), bytes.size())) { + return WTF::String(std::span(reinterpret_cast(bytes.data()), bytes.size())); + } + return WTF::String::fromUTF8ReplacingInvalidSequences(std::span(reinterpret_cast(bytes.data()), bytes.size())); + }); +} + +extern "C" JSC::EncodedJSValue napi_internal_create_string_utf16(napi_env env, const char16_t* ptr, size_t length) +{ + return napiCreateStringWithSuspendedException(env, ptr, length, [](std::span units) { + return WTF::String(units); + }); +} + extern "C" bool NapiEnv__hasPendingException(napi_env env) { if (env->hasPendingException()) { diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index f4eea2cc8539..15d7510b945d 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -9,7 +9,6 @@ use bun_collections::linear_fifo::DynamicBuffer; use bun_event_loop::ConcurrentTask::AutoDeinit; use bun_event_loop::{TaskTag, Taskable, task_tag}; use bun_io::KeepAlive; -use bun_jsc::StringJsc; use bun_jsc::event_loop::{ConcurrentTaskItem as ConcurrentTask, EventLoop}; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{ @@ -74,6 +73,11 @@ unsafe extern "C" { fn NapiEnv__deref(env: *mut NapiEnv); fn NapiEnv__ref(env: *mut NapiEnv); fn napi_set_last_error(env: napi_env, status: NapiStatus) -> napi_status; + fn napi_internal_create_string_latin1(env: *mut NapiEnv, ptr: *const u8, len: usize) + -> JSValue; + fn napi_internal_create_string_utf8(env: *mut NapiEnv, ptr: *const u8, len: usize) -> JSValue; + fn napi_internal_create_string_utf16(env: *mut NapiEnv, ptr: *const u16, len: usize) + -> JSValue; } impl NapiEnv { @@ -652,22 +656,13 @@ pub(super) extern "C" fn napi_create_string_latin1( bstr::BStr::new(slice) ); - if slice.is_empty() { - let js = match bun_core::String::empty().to_js(env.to_js()) { - Ok(v) => v, - Err(_) => return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure), - }; - result.set(env, js); - return env.ok(); - } - - let (mut string, bytes) = bun_core::String::create_uninitialized_latin1(slice.len()); - bytes.copy_from_slice(slice); - - let js = match string.transfer_to_js(env.to_js()) { - Ok(v) => v, - Err(_) => return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure), + // SAFETY: env is non-null (checked above); slice ptr/len from a live &[u8]. + let js = unsafe { + napi_internal_create_string_latin1(env.as_mut_ptr(), slice.as_ptr(), slice.len()) }; + if js == JSValue::ZERO { + return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure); + } result.set(env, js); env.ok() } @@ -704,12 +699,13 @@ pub(super) extern "C" fn napi_create_string_utf8( bun_output::scoped_log!(napi, "napi_create_string_utf8: {}", bstr::BStr::new(slice)); - let global_object = env.to_js(); - let string = match jsc::bun_string_jsc::create_utf8_for_js(global_object, slice) { - Ok(v) => v, - Err(_) => return NapiEnv::set_last_error(Some(env), NapiStatus::pending_exception), - }; - result.set(env, string); + // SAFETY: env is non-null (checked above); slice ptr/len from a live &[u8]. + let js = + unsafe { napi_internal_create_string_utf8(env.as_mut_ptr(), slice.as_ptr(), slice.len()) }; + if js == JSValue::ZERO { + return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure); + } + result.set(env, js); env.ok() } @@ -753,22 +749,12 @@ pub(super) extern "C" fn napi_create_string_utf16( ); } - if slice.is_empty() { - let js = match bun_core::String::empty().to_js(env.to_js()) { - Ok(v) => v, - Err(_) => return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure), - }; - result.set(env, js); - return env.ok(); + // SAFETY: env is non-null (checked above); slice ptr/len from a live &[u16]. + let js = + unsafe { napi_internal_create_string_utf16(env.as_mut_ptr(), slice.as_ptr(), slice.len()) }; + if js == JSValue::ZERO { + return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure); } - - let (mut string, chars) = bun_core::String::create_uninitialized_utf16(slice.len()); - chars.copy_from_slice(slice); - - let js = match string.transfer_to_js(env.to_js()) { - Ok(v) => v, - Err(_) => return NapiEnv::set_last_error(Some(env), NapiStatus::generic_failure), - }; result.set(env, js); env.ok() } diff --git a/test/napi/napi-app/standalone_tests.cpp b/test/napi/napi-app/standalone_tests.cpp index f445020a02e2..f95ce4bd3d79 100644 --- a/test/napi/napi-app/standalone_tests.cpp +++ b/test/napi/napi-app/standalone_tests.cpp @@ -2892,6 +2892,80 @@ static napi_value test_pending_exception_gate(const Napi::CallbackInfo &info) { return ok(env); } +// napi_create_string_* must succeed (napi_ok) and preserve a pending +// exception that already lives on the VM. Two ways to put one there without +// relying on Bun internals: +// (a) napi_throw(E) then napi_call_function(), which returns status 10 and +// promotes E into the VM, and +// (b) napi_create_bigint_words() with a length past the engine cap, which +// throws a RangeError directly into the VM and returns status 10. +// In debug/asan builds the Rust string creators previously routed through a +// validation scope that asserts "no VM exception" on success and aborted. +static napi_value test_create_string_with_vm_exception( + const Napi::CallbackInfo &info) { + napi_env env = info.Env(); + + napi_value global, isNaN; + NODE_API_CALL(env, napi_get_global(env, &global)); + NODE_API_CALL(env, napi_get_named_property(env, global, "isNaN", &isNaN)); + + const char *labels[] = {"call_function", "bigint_words"}; + for (int route = 0; route < 2; route++) { + // Arm a VM-level exception. + if (route == 0) { + napi_value msg, err; + NODE_API_CALL(env, napi_create_string_utf8(env, "E1", 2, &msg)); + NODE_API_CALL(env, napi_create_error(env, nullptr, msg, &err)); + NODE_API_CALL(env, napi_throw(env, err)); + napi_value r; + napi_status st = napi_call_function(env, global, isNaN, 0, nullptr, &r); + printf("%s napi_call_function: status=%d\n", labels[route], (int)st); + } else { + static const uint64_t word = 1; + napi_value big; + napi_status st = + napi_create_bigint_words(env, 0, (size_t)INT_MAX, &word, &big); + printf("%s napi_create_bigint_words: status=%d\n", labels[route], + (int)st); + } + + // String creators must succeed with the exception still pending. + napi_value s; + static const char16_t wide[] = {'x', 0}; + napi_status st = napi_create_string_utf8(env, "x", 1, &s); + printf("%s napi_create_string_utf8: status=%d\n", labels[route], (int)st); + st = napi_create_string_latin1(env, "x", 1, &s); + printf("%s napi_create_string_latin1: status=%d\n", labels[route], + (int)st); + st = napi_create_string_utf16(env, wide, 1, &s); + printf("%s napi_create_string_utf16: status=%d\n", labels[route], (int)st); + // Non-ASCII path (utf8 decoder takes a different branch). + st = napi_create_string_utf8(env, "\xc3\xa9", 2, &s); + printf("%s napi_create_string_utf8_nonascii: status=%d\n", labels[route], + (int)st); + + bool pending = false; + napi_is_exception_pending(env, &pending); + printf("%s pending_after_create=%s\n", labels[route], + pending ? "true" : "false"); + + napi_value exc; + napi_get_and_clear_last_exception(env, &exc); + if (route == 0) { + // Only the napi_throw'd Error has a portable message across engines. + napi_value exc_msg; + if (napi_coerce_to_string(env, exc, &exc_msg) == napi_ok) { + char buf[64]; + size_t n; + napi_get_value_string_utf8(env, exc_msg, buf, sizeof(buf), &n); + printf("%s exception=%s\n", labels[route], buf); + } + } + } + + return ok(env); +} + // Regression test: PROPERTY_NAME_FROM_UTF8 must copy string data. // Previously it used StringImpl::createWithoutCopying for ASCII strings, // which could leave dangling pointers in JSC's atom string table. @@ -3562,6 +3636,7 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_external_buffer_with_pending_exception); REGISTER_FUNCTION(env, exports, test_pending_exception_gate); + REGISTER_FUNCTION(env, exports, test_create_string_with_vm_exception); REGISTER_FUNCTION(env, exports, test_napi_get_named_property_copied_string); REGISTER_FUNCTION(env, exports, test_issue_25933); REGISTER_FUNCTION(env, exports, test_napi_make_callback_status); diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 0c2d07071ee0..de0ba0e2a667 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -433,6 +433,29 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { expect(result).toContain("side_effect arr[7]=undefined"); expect(result).toContain("side_effect script_ran=false"); }); + + it("napi_create_string_* succeeds while a VM exception is pending", async () => { + // Two routes put an exception into the VM (napi_call_function promotes a + // napi_throw'd value; napi_create_bigint_words over the engine cap + // throws RangeError). The string creators must return napi_ok and leave + // the exception pending, matching Node. In debug/asan builds Bun's + // validation scope previously aborted here. + const result = await checkSameOutput("test_create_string_with_vm_exception", []); + expect(result).toContain("call_function napi_call_function: status=10"); + expect(result).toContain("bigint_words napi_create_bigint_words: status=10"); + for (const route of ["call_function", "bigint_words"]) { + for (const fn of [ + "napi_create_string_utf8", + "napi_create_string_latin1", + "napi_create_string_utf16", + "napi_create_string_utf8_nonascii", + ]) { + expect(result).toContain(`${route} ${fn}: status=0`); + } + expect(result).toContain(`${route} pending_after_create=true`); + } + expect(result).toContain("call_function exception=Error: E1"); + }); }); describe("napi_async_work", () => {