diff --git a/src/rust/AGENTS.md b/src/rust/AGENTS.md index e64727d49ea..1a67b0af73a 100644 --- a/src/rust/AGENTS.md +++ b/src/rust/AGENTS.md @@ -67,6 +67,7 @@ _Snapshot — the set drifts as crates come and go; `bazel query //src/rust/...` - `src/rust/jsg-compile-tests` holds snippets that must not compile (or, prefixed `ok-`, must). Add one for every new lifetime or `Send`/`Sync` rule; the `//~ ERROR` annotations must not depend on how many errors rustc reports. - **Parameter ordering**: `&Lock` / `&mut Lock` must always be the first parameter in any function that takes a lock (matching the C++ convention where `jsg::Lock&` is always first). This applies to free functions, trait methods, and associated functions (excluding `&self`/`&mut self` receivers and a JSG method's `this: jsg::Rc`, which come before `lock`). - **Method naming**: do not use `get_` prefixes on methods — e.g. `buf.backing_store()` not `buf.get_backing_store()`. Static constructors belong on the marker struct (`impl ArrayBuffer { fn new(...) }`) not on `impl Local<'_, ArrayBuffer>`. +- **`v8` module conventions**: handles in, handles out; `FromJS`/`ToJS` are the only conversions between Rust and JavaScript values. A `v8` method returns a Rust value only where V8 returns a C scalar, from a typed handle (`value`, `length`, `is_*`, `value_of`) or from a coercion of any value (`int32_value`, `uint32_value`, `integer_value`, which return `Result`), or copies memory out (`to_vec`, `to_string`), and takes raw Rust data only where V8 takes C data (constructor input, indices, times, `&str` property keys). So `Function::call` returns a `Local`; for a Rust value write `let result = f.call(lock, None, &[])?;` then `f64::from_js(lock, result)?`. Anything that can run JavaScript or needs a context returns `Result`; `Option` means only that something is absent or that V8 refused with nothing pending (the `String` constructors past `MAX_LENGTH`, except `new_from_utf8_literal`, which panics); never an infallible signature over a C++ shim that can throw. Names: `as_*` borrows for free, `to_*` costs something, `value`/`*_value` read a scalar, named as in V8. Full text in `src/rust/jsg/README.md` §Conventions of the `v8` module. - **FFI naming**: instance methods on an existing handle use a `local__` prefix (e.g. `local_array_buffer_byte_length`). Static constructors that create a new value do **not** use the `local_` prefix — name them `_` (e.g. `array_buffer_new_with_mode`, `array_buffer_maybe_new`, `backing_store_new_resizable`). - **FFI groups**: `v8.rs` `mod ffi`, `ffi.h`, and `ffi.c++` all use matching comment groups (e.g. `// Local`, `// Local`, `// Local`, `// Local`, `// Local`, `// Local`, `// BackingStore`, `// Unwrappers`, `// TracedGlobal`, `// FunctionCallbackInfo`). When adding new FFI functions, place them in the correct group in **all three files**. Do not scatter related functions across groups. - **Feature flags**: `Lock::feature_flags()` returns a capnp `compatibility_flags::Reader` for the current worker. Use `lock.feature_flags().get_node_js_compat()`. Flags are parsed once and stored in the `Realm` at construction; C++ passes canonical capnp bytes to `realm_create()`. Schema: `src/workerd/io/compatibility-date.capnp`, generated Rust bindings: `compatibility_date_capnp` crate. diff --git a/src/rust/api/inspector.rs b/src/rust/api/inspector.rs index 51a04d7b052..e6c4efdeea8 100644 --- a/src/rust/api/inspector.rs +++ b/src/rust/api/inspector.rs @@ -133,7 +133,7 @@ impl InspectorConnection { let message = message.to_js(lock)?; // If the callback throws, its exception propagates from `dispatch()`, and the // rest of the batch is dropped. - callback.call::>(lock, None, &[message])?; + callback.call(lock, None, &[message])?; } } } diff --git a/src/rust/jsg-bench/lib.rs b/src/rust/jsg-bench/lib.rs index 5f8cba8cccb..7aace3ec524 100644 --- a/src/rust/jsg-bench/lib.rs +++ b/src/rust/jsg-bench/lib.rs @@ -5,6 +5,7 @@ use std::cell::Cell; use std::hint::black_box; +use jsg::FromJS; use jsg::ToJS; use jsg::v8; use jsg_macros::jsg_constructor; @@ -266,7 +267,8 @@ impl Target { .try_into() .map_err(|_| jsg::Error::new_type_error("expected a function"))?; let n = n.to_js(lock)?; - a.call::(lock, None, &[n]) + let result = a.call(lock, None, &[n])?; + f64::from_js(lock, result) } #[jsg_method] diff --git a/src/rust/jsg-compile-tests/ok-slice-then-js.rs b/src/rust/jsg-compile-tests/ok-slice-then-js.rs index 119ad919f97..4469e119316 100644 --- a/src/rust/jsg-compile-tests/ok-slice-then-js.rs +++ b/src/rust/jsg-compile-tests/ok-slice-then-js.rs @@ -10,6 +10,6 @@ pub fn t<'s>( f: v8::Local<'s, v8::Function>, ) -> Result { let first = bytes.as_slice(lock)[0]; - f.call::>(lock, None, &[])?; + f.call(lock, None, &[])?; Ok(first) } diff --git a/src/rust/jsg-compile-tests/slice-across-js-call.rs b/src/rust/jsg-compile-tests/slice-across-js-call.rs index 0f6ddeb58d3..f5a4cb61172 100644 --- a/src/rust/jsg-compile-tests/slice-across-js-call.rs +++ b/src/rust/jsg-compile-tests/slice-across-js-call.rs @@ -11,6 +11,6 @@ pub fn t<'s>( f: v8::Local<'s, v8::Function>, ) { let slice = bytes.as_slice(lock); - let _ = f.call::>(lock, None, &[]); //~ ERROR E0502 + let _ = f.call(lock, None, &[]); //~ ERROR E0502 let _ = slice[0]; } diff --git a/src/rust/jsg-test/tests/coercion_safety.rs b/src/rust/jsg-test/tests/coercion_safety.rs index 08b0620da8b..3e0970ef818 100644 --- a/src/rust/jsg-test/tests/coercion_safety.rs +++ b/src/rust/jsg-test/tests/coercion_safety.rs @@ -790,7 +790,7 @@ impl ConvertingResource { ) -> Result<(), jsg::Error> { let callback = v8::Local::::try_from(callback) .map_err(|_| jsg::Error::new_type_error("not a function"))?; - let called = callback.call::>(lock, None, &[]); + let called = callback.call(lock, None, &[]); let conversion = self.conversion.get().unwrap(); let converted = conversion.run(lock, value); self.saw_pending diff --git a/src/rust/jsg-test/tests/function.rs b/src/rust/jsg-test/tests/function.rs index 362910c0904..d65876f2fe4 100644 --- a/src/rust/jsg-test/tests/function.rs +++ b/src/rust/jsg-test/tests/function.rs @@ -81,8 +81,8 @@ fn call_zero_args() { harness.run_in_context(|lock, ctx| { let value = ctx.eval_raw(lock, "(() => 42)").unwrap(); let func = v8::Local::::try_from(value).unwrap(); - let result = func.call::(lock, None::>, &[])?; - assert_eq!(result, 42); + let result = func.call(lock, None, &[])?; + assert_eq!(u32::from_js(lock, result)?, 42); Ok(()) }); } @@ -97,8 +97,8 @@ fn call_with_args() { let a = 10_i32.to_js(lock)?; let b = 32_i32.to_js(lock)?; - let result = func.call::(lock, None::>, &[a, b])?; - assert_eq!(result, -22); + let result = func.call(lock, None, &[a, b])?; + assert_eq!(i32::from_js(lock, result)?, -22); Ok(()) }); } @@ -112,7 +112,8 @@ fn call_returns_string() { let func = v8::Local::::try_from(value).unwrap(); let arg = String::from("hello").to_js(lock)?; - let result = func.call::(lock, None::>, &[arg])?; + let result = func.call(lock, None, &[arg])?; + let result = String::from_js(lock, result)?; assert_eq!(result, "HELLO"); Ok(()) }); @@ -130,8 +131,8 @@ fn call_with_receiver() { .unwrap(); let func = v8::Local::::try_from(func_val).unwrap(); - let result = func.call::(lock, Some(obj), &[])?; - assert_eq!(result, 100); + let result = func.call(lock, Some(obj), &[])?; + assert_eq!(u32::from_js(lock, result)?, 100); Ok(()) }); } @@ -150,7 +151,8 @@ fn call_with_object_receiver_via_into() { let func = v8::Local::::try_from(func_val).unwrap(); // Pass Local as receiver — the Into> bound handles conversion. - let result = func.call::(lock, Some(obj.into()), &[])?; + let result = func.call(lock, Some(obj.into()), &[])?; + let result = String::from_js(lock, result)?; assert_eq!(result, "hello world"); Ok(()) }); @@ -168,9 +170,7 @@ fn call_leaves_thrown_exception_pending() { let func = v8::Local::::try_from(func).unwrap(); let caught = lock.try_catch(|lock| { - let err = func - .call::>(lock, None, &[]) - .unwrap_err(); + let err = func.call(lock, None, &[]).unwrap_err(); assert!(err.is_pending()); Err::<(), _>(err) })?; @@ -187,7 +187,10 @@ fn try_catch_catches_thrown_non_error() { let func = ctx.eval_raw(lock, "(() => { throw 42; })").unwrap(); let func = v8::Local::::try_from(func).unwrap(); - let caught = lock.try_catch(|lock| func.call::(lock, None, &[]))?; + let caught = lock.try_catch(|lock| { + let result = func.call(lock, None, &[])?; + u32::from_js(lock, result) + })?; let thrown = u32::from_js(lock, caught.unwrap_err())?; assert_eq!(thrown, 42); Ok(()) @@ -214,8 +217,7 @@ fn try_catch_does_not_run_thrown_value_accessors() { let func = ctx.eval_raw(lock, "(() => { throw thrown; })").unwrap(); let func = v8::Local::::try_from(func).unwrap(); - let caught = - lock.try_catch(|lock| func.call::>(lock, None, &[]))?; + let caught = lock.try_catch(|lock| func.call(lock, None, &[]))?; assert_eq!(caught.unwrap_err(), thrown); let reads = ctx.eval::(lock, "reads").unwrap(); assert_eq!(reads, 0, "{thrown:?} ran {reads} accessors"); @@ -232,7 +234,10 @@ fn try_catch_returns_result_on_success() { let func = ctx.eval_raw(lock, "(() => 7)").unwrap(); let func = v8::Local::::try_from(func).unwrap(); - let result = lock.try_catch(|lock| func.call::(lock, None, &[]))?; + let result = lock.try_catch(|lock| { + let result = func.call(lock, None, &[])?; + u32::from_js(lock, result) + })?; assert_eq!(result.unwrap(), 7); Ok(()) }); diff --git a/src/rust/jsg-test/tests/function_new.rs b/src/rust/jsg-test/tests/function_new.rs index 12dbe666fb6..a1156324389 100644 --- a/src/rust/jsg-test/tests/function_new.rs +++ b/src/rust/jsg-test/tests/function_new.rs @@ -204,9 +204,7 @@ impl TerminatingCreator { ) -> Result<(), jsg::Error> { lock.terminate_execution(); let spin: v8::Local = spin.try_into().unwrap(); - let err = spin - .call::>(lock, None, &[]) - .unwrap_err(); + let err = spin.call(lock, None, &[]).unwrap_err(); assert!(err.is_pending()); let err = v8::Function::new(lock, (), |lock, (), _args| Ok(lock.undefined())).unwrap_err(); self.saw_pending.set(err.is_pending()); @@ -255,9 +253,7 @@ impl ThrowingCreator { throws: v8::Local<'s, v8::Value>, ) -> Result<(), jsg::Error> { let throws: v8::Local = throws.try_into().unwrap(); - let err = throws - .call::>(lock, None, &[]) - .unwrap_err(); + let err = throws.call(lock, None, &[]).unwrap_err(); assert!(err.is_pending()); let err = v8::Function::new(lock, (), |lock, (), _args| Ok(lock.undefined())).unwrap_err(); self.saw_pending.set(err.is_pending()); @@ -376,7 +372,7 @@ fn reentrant_call_sees_its_own_data() { if depth > 0 { let again: v8::Local = args.get(1).try_into().unwrap(); let next = (depth - 1).to_js(lock)?; - again.call::>(lock, None, &[next])?; + again.call(lock, None, &[next])?; } log.borrow_mut().push(format!("-{depth}")); log.borrow().join(" ").to_js(lock) diff --git a/src/rust/jsg-test/tests/isolates.rs b/src/rust/jsg-test/tests/isolates.rs index 055770cc648..dcf92d88fc3 100644 --- a/src/rust/jsg-test/tests/isolates.rs +++ b/src/rust/jsg-test/tests/isolates.rs @@ -343,7 +343,7 @@ fn function_call_with_argument_of_another_isolate_panics_in_debug_builds() { .try_into() .unwrap(); assert_isolate_check_panics("calling B's function with A's object", || { - let _ = func.call::(lock_b, None, &[object]); + let _ = func.call(lock_b, None, &[object]); }); Ok(()) }); diff --git a/src/rust/jsg-test/tests/local_cast.rs b/src/rust/jsg-test/tests/local_cast.rs index ddbc21527a9..fffb4757f09 100644 --- a/src/rust/jsg-test/tests/local_cast.rs +++ b/src/rust/jsg-test/tests/local_cast.rs @@ -5,6 +5,7 @@ //! Tests for `impl_local_cast!` conversions added for Float/BigInt typed arrays, //! and upcasts to Object (Function -> Object, Array -> Object, `TypedArray` -> Object). +use jsg::FromJS; use jsg::ToJsTypedArray; use jsg::v8; @@ -358,7 +359,8 @@ fn function_object_roundtrip() { assert_eq!(name, "namedFn"); // Roundtrip back and verify it's still callable let back: v8::Local = obj.try_into().unwrap(); - assert_eq!(back.call::(lock, None, &[])?, 42); + let result = back.call(lock, None, &[])?; + assert_eq!(u32::from_js(lock, result)?, 42); Ok(()) }); } diff --git a/src/rust/jsg-test/tests/lock_errors.rs b/src/rust/jsg-test/tests/lock_errors.rs new file mode 100644 index 00000000000..433933f743f --- /dev/null +++ b/src/rust/jsg-test/tests/lock_errors.rs @@ -0,0 +1,104 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! The errors a `Lock` creates and throws reach JavaScript as errors of their type, with their +//! message. + +use jsg_macros::jsg_method; +use jsg_macros::jsg_resource; + +#[jsg_resource] +struct Thrower; + +#[jsg_resource] +impl Thrower { + #[jsg_method] + fn throw_error(&self, lock: &mut jsg::Lock, message: String) { + lock.throw_error(&message); + } + + #[jsg_method] + fn throw_invalid_string_length(&self, lock: &mut jsg::Lock) { + lock.throw_exception(&jsg::Error::invalid_string_length()); + } + + #[jsg_method] + fn range_error<'s>( + &self, + lock: &mut jsg::Lock<'s>, + message: String, + ) -> jsg::v8::Local<'s, jsg::v8::Object> { + lock.range_error(&message) + } +} + +fn caught(lock: &mut jsg::Lock<'_>, ctx: &crate::EvalContext<'_>, call: &str, check: &str) -> bool { + ctx.eval::( + lock, + &format!( + "{{ let caught; + try {{ {call}; }} catch (e) {{ caught = e; }} + {check} }}" + ), + ) + .unwrap() +} + +#[test] +fn throw_error_throws_an_error_with_the_message() { + crate::Harness::new().run_in_context(|lock, ctx| { + ctx.set_global(lock, "thrower", jsg::Rc::new(Thrower)); + assert!(caught( + lock, + ctx, + "thrower.throwError('boom')", + "caught.constructor === Error && caught.message === 'boom'" + )); + // A message with a multi-byte character reaches JavaScript whole. + assert!(caught( + lock, + ctx, + "thrower.throwError('héllo')", + "caught.message === 'héllo'" + )); + Ok(()) + }); +} + +#[test] +fn invalid_string_length_is_v8s_range_error() { + crate::Harness::new().run_in_context(|lock, ctx| { + ctx.set_global(lock, "thrower", jsg::Rc::new(Thrower)); + assert!(caught( + lock, + ctx, + "thrower.throwInvalidStringLength()", + "caught instanceof RangeError && caught.message === 'Invalid string length'" + )); + // The same error V8 throws for a string it cannot create. + assert!(caught( + lock, + ctx, + "'x'.repeat(2 ** 30)", + "caught instanceof RangeError && caught.message === 'Invalid string length'" + )); + Ok(()) + }); +} + +#[test] +fn range_error_is_a_range_error_object() { + crate::Harness::new().run_in_context(|lock, ctx| { + ctx.set_global(lock, "thrower", jsg::Rc::new(Thrower)); + assert!( + ctx.eval::( + lock, + "const e = thrower.rangeError('out of range'); + e instanceof RangeError && e.message === 'out of range'" + ) + .unwrap() + ); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/mod.rs b/src/rust/jsg-test/tests/mod.rs index c6f9809d222..4eff79bbfa3 100644 --- a/src/rust/jsg-test/tests/mod.rs +++ b/src/rust/jsg-test/tests/mod.rs @@ -25,6 +25,7 @@ mod jsg_struct; mod local_cast; mod local_handle; mod local_value; +mod lock_errors; mod name; mod nested_type; mod non_coercible; diff --git a/src/rust/jsg-test/tests/promise.rs b/src/rust/jsg-test/tests/promise.rs index 2228d91eaab..63bcc4ebdc7 100644 --- a/src/rust/jsg-test/tests/promise.rs +++ b/src/rust/jsg-test/tests/promise.rs @@ -529,7 +529,7 @@ impl Watcher { watch.state.seen.borrow_mut().push(seen); // No borrow is held across the call, whose JavaScript calls back into the watcher. let callback = watch.callback.as_local(lock); - callback.call::>(lock, None, &[value]) + callback.call(lock, None, &[value]) }) } @@ -588,9 +588,7 @@ impl TerminatingPromises { let promise = resolver.promise(); lock.terminate_execution(); let spin = v8::Local::::try_from(spin).unwrap(); - let err = spin - .call::>(lock, None, &[]) - .unwrap_err(); + let err = spin.call(lock, None, &[]).unwrap_err(); assert!(err.is_pending()); let mut pending = Vec::new(); @@ -918,8 +916,7 @@ impl ReadThroughCache { let key_js = key.as_str().to_js(lock)?; // No RefCell borrow is held here: the fallback is JavaScript and may call `read` again. - let called = - lock.try_catch(|lock| fallback.call::>(lock, None, &[key_js])); + let called = lock.try_catch(|lock| fallback.call(lock, None, &[key_js])); match called { // Resolving with a promise follows it, and with any other value fulfills. Ok(Ok(produced)) => resolver.resolve(lock, produced)?, diff --git a/src/rust/jsg-test/tests/resource_callback.rs b/src/rust/jsg-test/tests/resource_callback.rs index b7fd5564cf8..dfa2a0b74cb 100644 --- a/src/rust/jsg-test/tests/resource_callback.rs +++ b/src/rust/jsg-test/tests/resource_callback.rs @@ -558,7 +558,7 @@ impl ReentrantResource { let callback: v8::Local = callback .try_into() .map_err(|_| jsg::Error::new_type_error("not a function"))?; - callback.call::>(lock, None, &[])?; + callback.call(lock, None, &[])?; self.depth.set(self.depth.get() - 1); Ok(()) } @@ -615,7 +615,7 @@ impl CallingResource { callback: v8::Local<'s, v8::Value>, ) -> Result<(), jsg::Error> { Self::function(callback)? - .call::>(lock, None, &[]) + .call(lock, None, &[]) .inspect_err(|err| self.saw_pending.set(err.is_pending()))?; Ok(()) } @@ -628,8 +628,8 @@ impl CallingResource { first: v8::Local<'s, v8::Value>, second: v8::Local<'s, v8::Value>, ) -> Result<(), jsg::Error> { - let first = Self::function(first)?.call::>(lock, None, &[]); - let second = Self::function(second)?.call::>(lock, None, &[]); + let first = Self::function(first)?.call(lock, None, &[]); + let second = Self::function(second)?.call(lock, None, &[]); self.saw_pending .set(second.is_err_and(|err| err.is_pending())); first.map(|_| ()) @@ -644,7 +644,7 @@ impl CallingResource { ) -> Result { let callback = Self::function(callback)?; let caught = lock - .try_catch(|lock| callback.call::>(lock, None, &[])) + .try_catch(|lock| callback.call(lock, None, &[])) .inspect_err(|err| self.saw_pending.set(err.is_pending()))?; Ok(caught.is_err()) } @@ -656,7 +656,7 @@ impl CallingResource { lock: &mut jsg::Lock<'s>, callback: v8::Local<'s, v8::Value>, ) -> Result<(), jsg::Error> { - let _ = Self::function(callback)?.call::>(lock, None, &[]); + let _ = Self::function(callback)?.call(lock, None, &[]); Err(jsg::Error::new_type_error("replaced")) } @@ -670,7 +670,7 @@ impl CallingResource { ) -> Result<(), jsg::Error> { let callback = Self::function(callback)?; let _ = lock.try_catch(|lock| { - let result = callback.call::>(lock, None, &[]); + let result = callback.call(lock, None, &[]); if let Err(err) = &result { self.saved.set(Some(err.clone())); } diff --git a/src/rust/jsg-test/tests/string.rs b/src/rust/jsg-test/tests/string.rs index f47e9ed4858..21b8cfcf69c 100644 --- a/src/rust/jsg-test/tests/string.rs +++ b/src/rust/jsg-test/tests/string.rs @@ -8,6 +8,8 @@ use jsg::v8::String as JsString; use jsg::v8::Utf8Value; use jsg::v8::Value; use jsg::v8::WriteFlags; +use jsg_macros::jsg_method; +use jsg_macros::jsg_resource; // Convenience: create a Local from a UTF-8 str literal. fn from_utf8<'a>(lock: &mut jsg::Lock<'a>, s: &str) -> Local<'a, JsString> { @@ -557,3 +559,76 @@ fn string_max_length_matches_v8() { #[cfg(target_pointer_width = "64")] assert_eq!(JsString::MAX_LENGTH, (1 << 29) - 24); } + +// ============================================================================= +// Strings longer than V8 allows +// ============================================================================= + +/// A string one byte longer than V8 accepts. Its bytes are zero, so that the allocation costs +/// no physical memory until something reads it, which refusing it by its length never does. +fn overlong_string() -> String { + let bytes = vec![0u8; usize::try_from(JsString::MAX_LENGTH).unwrap() + 1]; + // SAFETY: zero bytes are ASCII, so valid UTF-8. + unsafe { String::from_utf8_unchecked(bytes) } +} + +#[jsg_resource] +struct Overlong; + +#[jsg_resource] +impl Overlong { + #[jsg_method] + fn string(&self) -> String { + overlong_string() + } +} + +#[test] +fn string_constructors_refuse_an_overlong_string() { + crate::Harness::new().run_in_context(|lock, ctx| { + let s = overlong_string(); + assert!(JsString::new_from_str(lock, &s).is_none()); + assert!(JsString::new_from_utf8(lock, s.as_bytes()).is_none()); + assert!(JsString::new_internalized_from_str(lock, &s).is_none()); + assert!(JsString::new_from_one_byte(lock, s.as_bytes()).is_none()); + // V8 refused with nothing pending, so JavaScript still runs. + assert!(ctx.eval::(lock, "1 + 1 === 2").unwrap()); + Ok(()) + }); +} + +#[test] +fn to_js_of_an_overlong_string_is_a_range_error() { + crate::Harness::new().run_in_context(|lock, ctx| { + let s = overlong_string(); + for err in [ + s.as_str().to_js(lock).unwrap_err(), + s.to_js(lock).unwrap_err(), + ] { + assert_eq!(err.name, jsg::ExceptionType::RangeError); + assert_eq!(err.message, "Invalid string length"); + assert!(!err.is_pending(), "the error is not a pending exception"); + } + // Nothing is pending, so JavaScript still runs. + assert!(ctx.eval::(lock, "1 + 1 === 2").unwrap()); + Ok(()) + }); +} + +/// JavaScript catches the `RangeError` that V8 itself throws for such a string. +#[test] +fn method_returning_an_overlong_string_throws_a_range_error() { + crate::Harness::new().run_in_context(|lock, ctx| { + ctx.set_global(lock, "overlong", jsg::Rc::new(Overlong)); + assert!( + ctx.eval::( + lock, + "let caught; + try { overlong.string(); } catch (e) { caught = e; } + caught instanceof RangeError && caught.message === 'Invalid string length'" + ) + .unwrap() + ); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/to_js_errors.rs b/src/rust/jsg-test/tests/to_js_errors.rs index 0291a1552e6..13cbbaf6893 100644 --- a/src/rust/jsg-test/tests/to_js_errors.rs +++ b/src/rust/jsg-test/tests/to_js_errors.rs @@ -23,7 +23,7 @@ impl<'s> ToJS<'s> for CallsWhenConverted { fn to_js(self, lock: &mut jsg::Lock<'s>) -> jsg::Result> { let callback = v8::Local::::try_from(self.callback.as_local(lock)) .map_err(|_| jsg::Error::new_type_error("expected a function"))?; - callback.call::>(lock, None::>, &[]) + callback.call(lock, None, &[]) } } diff --git a/src/rust/jsg-test/tests/wildcard_property.rs b/src/rust/jsg-test/tests/wildcard_property.rs index 7813a7f07b2..1490a8d9029 100644 --- a/src/rust/jsg-test/tests/wildcard_property.rs +++ b/src/rust/jsg-test/tests/wildcard_property.rs @@ -89,7 +89,7 @@ impl Caller { } let callback = self.callback.borrow().as_ref().map(|f| f.as_local(lock)); if let Some(callback) = callback { - let _ = callback.call::>(lock, None, &[]); + let _ = callback.call(lock, None, &[]); } Err(jsg::Error::new_type_error("replaced")) } diff --git a/src/rust/jsg/README.md b/src/rust/jsg/README.md index ae7665cc2f1..1a50f309a7c 100644 --- a/src/rust/jsg/README.md +++ b/src/rust/jsg/README.md @@ -428,7 +428,8 @@ yet. Other V8 classes that Rust JSG wraps, such as `v8::BackingStore`, are also A `Local` is one non-null pointer, laid out like `v8::Local`. It does not record its isolate; methods that need the isolate take the lock. A handle that may be empty is an `Option>`, laid out like `v8::MaybeLocal`: for example, the `String` -constructors return `None` when V8 cannot create the string. +constructors that take runtime input return `None` for a string longer than +`String::MAX_LENGTH`, which V8 refuses with no exception pending. A cast to a supertype, such as `Local` to `Local` or `Local`, is a `From` and costs nothing, whichever supertype it targets. A cast to a subtype is a @@ -555,18 +556,53 @@ or a JavaScript exception is pending, `Function::new` creates nothing and return propagating. As with the functions C++ JSG wraps, the function's `name` is empty, its `length` is 0, and it can be called with `new`. +### Conventions of the `v8` module + +The `v8` module takes handles and returns handles. Converting between a Rust value and a +JavaScript value is the job of `FromJS` and `ToJS`, not of a `v8` method. + +- A method returns a Rust value only where V8 itself returns a C scalar, or copies memory out + (`to_vec`, `to_string`). The scalar comes from a typed handle (`value`, `length`, `value_of`, + `int64_value`, `is_*`, `byte_length`), or from one of V8's own coercions of any value + (`int32_value`, `uint32_value`, `integer_value`). `Function::call` and `Object::get` return a + `Local`; convert it where a Rust value is wanted: + + ```rust + let result = func.call(lock, None, &[x, y])?; + let sum = f64::from_js(lock, result)?; + ``` + +- A method takes raw Rust data only where V8 takes C data: a constructor's input + (`String::new_from_str`, `Number::new`, `Integer::new`), an index, a time, and a `&str` + property key as a convenience (`obj.get(lock, "name")`). +- A method that can run JavaScript, or that needs a context, returns a `Result`, as the + coercions do; its error is usually pending (see + [Errors from JavaScript](#errors-from-javascript)). It returns an `Option` only for "absent", + as `int64_value` does for a `BigInt` that does not fit, or for "V8 refused, with nothing + pending", as the `String` constructors do for input longer than `String::MAX_LENGTH`. The + exception is `String::new_from_utf8_literal`, which panics for an overlong literal, as V8's + `NewFromUtf8Literal` fails a `static_assert`. A method never has an infallible signature over + a C++ shim that can throw: the shim is declared `Result` in the bridge, or Rust decides before + calling it, as `String::new_from_utf8` checks the length. +- Naming: `as_*` is free and borrows (`as_slice`, `as_local`); `to_*` costs a copy or an + allocation (`to_vec`, `to_string`, `to_global`); `value` and `*_value` read a scalar under + the name V8 gives it (`Local::value`, `int64_value`, `int32_value`). + ## Errors from JavaScript When a function that `Local::call` calls throws, the exception stays pending in V8, as with C++ JSG's `JsExceptionThrown`, and `call` returns an error for which `Error::is_pending()` is true. The same goes for these other operations that run JavaScript: converting a value to a string or a number (its `toString`, `valueOf` or `Symbol.toPrimitive`, or a Proxy trap), `Object::get` and `Object::has` (a getter or Proxy trap), `Object::set` (a setter, also one on the prototype chain, or a Proxy trap) and `Array::set` (a setter, also one on the prototype chain), `Array::iterate` and so converting to a `Vec` (an element's getter), and `ser::serialize` (a getter or Proxy trap, or a `DataCloneError`). Building the array when converting a `Vec` to JavaScript runs no JavaScript: like C++ JSG, it defines the elements as data properties (`Array::with_elements`), so no setter on `Array.prototype` runs. Nor does converting a `jsg::Dict` to JavaScript, which defines its entries as data properties. A `#[jsg_struct]` follows C++ JSG's `fast_jsg_struct` flag: with it, the fields are defined as data properties (`Object::with_data_properties`), an absent optional field as `undefined`, and no JavaScript runs; without it, they are set one by one with `Object::set`, an absent optional field is omitted, so a setter on `Object.prototype` runs, and what it throws is passed on. `Error`'s own `FromJS`, which reads `name` and `message`, passes such an error on too. `Lock::throw_exception` throws nothing for that error, so a callback that returns it propagates the original exception to its JavaScript caller. V8 keeps reporting the exception where it was thrown, and DevTools' "pause on uncaught exceptions" still sees whether JavaScript further up catches it. A termination is handled the same way. -Converting a value to JavaScript can fail too, as C++ JSG's `wrap()` can throw, so `ToJS::to_js` returns a `Result`: creating a `Date` or a resource's wrapper fails, leaving the exception pending, when V8 throws or execution is terminating, and a container passes on the error from converting an element. These conversions fail only with pending errors. A `#[jsg_method]`, property getter or wildcard property throws an error from converting its return value as it throws one the method returns, with `Lock::throw_exception`: for a pending error the original exception, or the termination, keeps propagating, and any other error is thrown as a new JavaScript error of its type. The method has already run by then; only its return value is lost. +Converting a value to JavaScript can fail too, as C++ JSG's `wrap()` can throw, so `ToJS::to_js` returns a `Result`: creating a `Date` or a resource's wrapper fails, leaving the exception pending, when V8 throws or execution is terminating, and a container passes on the error from converting an element, whichever kind it is. A `&str` or `String` longer than `v8::String::MAX_LENGTH` fails with a `RangeError` ("Invalid string length", as V8 reports it), which is not pending; every other conversion of a single value in this crate fails only with a pending error. A `#[jsg_method]`, property getter or wildcard property throws an error from converting its return value as it throws one the method returns, with `Lock::throw_exception`: for a pending error the original exception, or the termination, keeps propagating, and any other error is thrown as a new JavaScript error of its type. The method has already run by then; only its return value is lost. Code that gets a pending error must not call into JavaScript, and should return the error. Converting it to another error and throwing that replaces the exception, as when C++ code throws while a `JsExceptionThrown` unwinds; nothing ever replaces a termination, and none of these run while execution is terminating: they fail with a pending error without preparing V8 to run JavaScript, which would clear the termination, even for a value that would convert without running JavaScript. A pending error is only meaningful while its exception is pending: one kept past that, such as a clone stored after `try_catch` caught the exception, is thrown as an internal error when V8 has no exception pending. To handle an exception instead, run the code in `Lock::try_catch`, which catches as C++ `jsg::Lock::tryCatch()` does: the exception, or an error the closure returns, as a value; a termination keeps propagating. ```rust -match lock.try_catch(|lock| callback.call::(lock, None, &[]))? { +match lock.try_catch(|lock| { + let result = callback.call(lock, None, &[])?; + f64::from_js(lock, result) +})? { Ok(result) => { /* ... */ } Err(exception) => { /* handle what the callback threw */ } } diff --git a/src/rust/jsg/fast_api.rs b/src/rust/jsg/fast_api.rs index 8099586c409..f29678ed1b6 100644 --- a/src/rust/jsg/fast_api.rs +++ b/src/rust/jsg/fast_api.rs @@ -306,7 +306,11 @@ unsafe fn throw(options: *mut ffi::FastApiCallbackOptions, err: &Err if err.is_internal { ffi::fast_api_throw_internal_error(options, &err.message); } else { - ffi::fast_api_throw_exception(options, err.name, &err.message); + ffi::fast_api_throw_exception( + options, + err.name, + crate::error_message_for_v8(err.message.as_bytes()), + ); } } T::default() diff --git a/src/rust/jsg/ffi.c++ b/src/rust/jsg/ffi.c++ index 3a81e5cd7a3..c65228ad751 100644 --- a/src/rust/jsg/ffi.c++ +++ b/src/rust/jsg/ffi.c++ @@ -60,33 +60,32 @@ namespace workerd::rust::jsg { // BackingStore — the size_t handle is the address of a heap-allocated // std::shared_ptr allocated with `new`. -// Creates a V8 string from UTF-8. NewFromUtf8 fails only when the string is longer than -// v8::String::kMaxLength, and then leaves no JavaScript exception pending, so an overlong string -// throws a KJ exception instead. The length is checked before the call, which keeps a length -// above INT_MAX from reaching NewFromUtf8's int parameter, where a negative value means -// "NUL-terminated". kMaxLength counts UTF-16 code units and the check counts UTF-8 bytes, of -// which there are never fewer, so it may reject a long multi-byte string that V8 could hold, but -// never lets through one that V8 would reject. -static v8::Local newUtf8String( +// The longest UTF-8 input that NewFromUtf8 accepts. kMaxLength counts UTF-16 code units, and a +// string never has more of those than it has UTF-8 bytes, so a UTF-8 input of at most this many +// bytes always fits. +static constexpr size_t MAX_UTF8_STRING_SIZE = static_cast(v8::String::kMaxLength); + +// Creates a V8 string from UTF-8. Returns an empty handle, with no JavaScript exception pending, +// if the input is longer than MAX_UTF8_STRING_SIZE, as NewFromUtf8 itself does. The length is +// checked here so that a length above INT_MAX never reaches NewFromUtf8's int parameter, where a +// negative value means "NUL-terminated". Never throws, and never fails for a shorter input, so +// each caller decides what an overlong string means. +static v8::MaybeLocal newUtf8String( v8::Isolate* isolate, kj::ArrayPtr utf8, v8::NewStringType type) { - KJ_REQUIRE(utf8.size() <= static_cast(v8::String::kMaxLength), - "string exceeds V8's string length limit", utf8.size()); - v8::Local result; - if (!v8::String::NewFromUtf8(isolate, utf8.begin(), type, static_cast(utf8.size())) - .ToLocal(&result)) { - KJ_FAIL_REQUIRE("V8 failed to create a string", utf8.size()); - } - return result; + if (utf8.size() > MAX_UTF8_STRING_SIZE) return {}; + return v8::String::NewFromUtf8(isolate, utf8.begin(), type, static_cast(utf8.size())); } // Creates an interned V8 string from a Rust identifier name (rust::String or rust::Str). An -// overlong name is checked here too, so that the error names it. +// overlong name throws. The property shims, which take a key from arbitrary Rust text, are +// declared fallible in v8.rs for that reason. The names a resource template interns come from +// Rust source and are far shorter. template static v8::Local makeInternedStr(v8::Isolate* isolate, const Name& name) { - KJ_REQUIRE(name.size() <= static_cast(v8::String::kMaxLength), - "Rust identifier name exceeds V8 string length limit", name); + KJ_REQUIRE(name.size() <= MAX_UTF8_STRING_SIZE, "name exceeds V8's string length limit", name); return newUtf8String( - isolate, kj::arrayPtr(name.data(), name.size()), v8::NewStringType::kInternalized); + isolate, kj::arrayPtr(name.data(), name.size()), v8::NewStringType::kInternalized) + .ToLocalChecked(); } // Wrappable implementation - calls into Rust via CXX bridge @@ -123,12 +122,6 @@ Local local_new_number(Isolate* isolate, double value) { return to_ffi(kj::mv(val)); } -Local local_new_string(Isolate* isolate, ::rust::Str value) { - auto val = - newUtf8String(isolate, kj::arrayPtr(value.data(), value.size()), v8::NewStringType::kNormal); - return to_ffi(kj::mv(val)); -} - Local local_new_boolean(Isolate* isolate, bool value) { v8::Local val = v8::Boolean::New(isolate, value); return to_ffi(kj::mv(val)); @@ -1212,12 +1205,14 @@ TraitObjectPtr fast_api_unwrap_receiver(FastApiCallbackOptions* options, Local r return {}; } -void fast_api_throw_exception( - FastApiCallbackOptions* options, ExceptionType exceptionType, ::rust::Str message) { +void fast_api_throw_exception(FastApiCallbackOptions* options, + ExceptionType exceptionType, + ::rust::Slice message) { + // `isolate_throw_exception` would throw nothing, so create nothing. + if (options->isolate->IsExecutionTerminating()) return; v8::HandleScope scope(options->isolate); - isolate_throw_exception(options->isolate, - exception_create_from_bytes(options->isolate, exceptionType, - reinterpret_cast(message.data()), static_cast(message.size()))); + isolate_throw_exception( + options->isolate, exception_create_from_bytes(options->isolate, exceptionType, message)); } void fast_api_throw_internal_error(FastApiCallbackOptions* options, ::rust::Str internalMessage) { @@ -1364,8 +1359,10 @@ Global create_resource_template(Isolate* isolate, const ResourceDescriptor& desc instance->SetInternalFieldCount(workerd::jsg::Wrappable::INTERNAL_FIELD_COUNT); + // A resource's name comes from Rust source, so it is far shorter than V8's limit. auto classname = newUtf8String(isolate, - kj::arrayPtr(descriptor.name.data(), descriptor.name.size()), v8::NewStringType::kNormal); + kj::arrayPtr(descriptor.name.data(), descriptor.name.size()), v8::NewStringType::kNormal) + .ToLocalChecked(); if (workerd::jsg::getShouldSetToStringTag(isolate)) { prototype->Set(v8::Symbol::GetToStringTag(isolate), classname, v8::PropertyAttribute::DontEnum); @@ -1629,16 +1626,22 @@ Local exceptionCreateFromMessage(ExceptionType exception_type, v8::Local message) { // kNormal (not kInternalized): error messages are arbitrary, potentially // non-UTF-8 byte strings (e.g. filesystem paths) and should not pollute the // internalized string table (which is never GC'd). NewFromUtf8 replaces // invalid sequences with U+FFFD, matching the legacy jsg::Lock::error() / // js.str() behavior. - auto message = newUtf8String(isolate, - kj::arrayPtr(reinterpret_cast(data), static_cast(length)), - v8::NewStringType::kNormal); - return exceptionCreateFromMessage(exception_type, message); + // + // A message longer than V8's limit is cut to fit rather than refused, so that creating an + // error never fails. The Rust callers cut it at a character boundary first (see + // `error_message_for_v8` in lib.rs); the cut here keeps the shim total for any caller, and + // after it NewFromUtf8 cannot refuse the string. + auto utf8 = kj::arrayPtr(reinterpret_cast(message.data()), message.size()); + auto v8Message = newUtf8String( + isolate, utf8.first(kj::min(utf8.size(), MAX_UTF8_STRING_SIZE)), v8::NewStringType::kNormal) + .ToLocalChecked(); + return exceptionCreateFromMessage(exception_type, v8Message); } namespace { @@ -1684,16 +1687,6 @@ void isolate_throw_exception(Isolate* isolate, Local exception) { isolate->ThrowException(local_from_ffi(kj::mv(exception))); } -void isolate_throw_error(Isolate* isolate, ::rust::Str description) { - // kNormal (not kInternalized): error messages are arbitrary caller-supplied - // strings and should not pollute the internalized string table (which is never - // GC'd), matching C++ jsg::Lock::v8Error() / v8Str() behavior. - if (isolate->IsExecutionTerminating()) return; - auto message = newUtf8String( - isolate, kj::arrayPtr(description.data(), description.size()), v8::NewStringType::kNormal); - isolate->ThrowError(message); -} - void isolate_throw_internal_error(Isolate* isolate, ::rust::Str internalMessage) { if (isolate->IsExecutionTerminating()) { // Logs the message, but creates no error: creating one while execution is terminating fails diff --git a/src/rust/jsg/ffi.h b/src/rust/jsg/ffi.h index 037eb52411f..416df45ebc0 100644 --- a/src/rust/jsg/ffi.h +++ b/src/rust/jsg/ffi.h @@ -125,7 +125,6 @@ void global_clear_weak(uintptr_t* global); // Local Global local_to_global(Isolate* isolate, Local value); Local local_new_number(Isolate* isolate, double value); -Local local_new_string(Isolate* isolate, ::rust::Str value); Local local_new_boolean(Isolate* isolate, bool value); Local local_new_object(Isolate* isolate); Local local_new_null(Isolate* isolate); @@ -449,8 +448,9 @@ Local pqci_get_this(PropertyQueryCallbackInfo* info); // FastApiCallbackOptions // Fast callbacks run without a HandleScope; none of these leak a handle. TraitObjectPtr fast_api_unwrap_receiver(FastApiCallbackOptions* options, Local receiver); -void fast_api_throw_exception( - FastApiCallbackOptions* options, ExceptionType exceptionType, ::rust::Str message); +void fast_api_throw_exception(FastApiCallbackOptions* options, + ExceptionType exceptionType, + ::rust::Slice message); void fast_api_throw_internal_error(FastApiCallbackOptions* options, ::rust::Str internalMessage); void fast_api_terminate_execution(FastApiCallbackOptions* options); @@ -521,8 +521,9 @@ MaybeLocal value_deserialize(Isolate* isolate, ::rust::Slice data // Builds an exception whose message is raw bytes interpreted as UTF-8. Invalid // UTF-8 sequences are replaced with U+FFFD by v8::String::NewFromUtf8 rather // than rejected, so this accepts arbitrary byte input (e.g. filesystem paths). +// A message longer than V8's string limit is cut to fit, so this never fails. Local exception_create_from_bytes( - Isolate* isolate, ExceptionType exception_type, const uint8_t* data, int32_t length); + Isolate* isolate, ExceptionType exception_type, ::rust::Slice message); // The exception `isolate_throw_internal_error` throws, without throwing it: logs // `internalMessage` with a new reference ID and returns a generic error carrying only the ID. Local exception_create_internal(Isolate* isolate, ::rust::Str internalMessage); @@ -541,7 +542,6 @@ void isolate_try_catch( // The isolate_throw_*() functions throw nothing while execution is terminating, so that they // never replace the termination with a catchable exception. void isolate_throw_exception(Isolate* isolate, Local exception); -void isolate_throw_error(Isolate* isolate, ::rust::Str message); void isolate_throw_internal_error(Isolate* isolate, ::rust::Str internalMessage); void isolate_terminate_execution(Isolate* isolate); bool isolate_is_execution_terminating(Isolate* isolate); diff --git a/src/rust/jsg/lib-test.rs b/src/rust/jsg/lib-test.rs index 19e571269ba..4b7674d507a 100644 --- a/src/rust/jsg/lib-test.rs +++ b/src/rust/jsg/lib-test.rs @@ -108,3 +108,69 @@ fn expected_prefix_without_tunneling_tag_is_redacted() { assert_eq!(err.message, "expected someCondition; some message"); assert!(err.is_internal); } + +// ============================================================================= +// error_message_for_v8 / utf8_prefix +// ============================================================================= + +#[test] +fn utf8_prefix_keeps_bytes_that_fit() { + assert_eq!(utf8_prefix(b"abc", 3), b"abc"); + assert_eq!(utf8_prefix(b"abc", 4), b"abc"); + assert_eq!(utf8_prefix(b"", 0), b""); +} + +#[test] +fn utf8_prefix_cuts_ascii_at_the_limit() { + assert_eq!(utf8_prefix(b"abcdef", 4), b"abcd"); + assert_eq!(utf8_prefix(b"abcdef", 1), b"a"); + assert_eq!(utf8_prefix(b"abcdef", 0), b""); +} + +#[test] +fn utf8_prefix_does_not_cut_inside_a_sequence() { + // "aé" is 61 C3 A9: a limit of 2 falls inside the two-byte sequence. + let bytes = "aé".as_bytes(); + assert_eq!(utf8_prefix(bytes, 2), b"a"); + assert_eq!(utf8_prefix(bytes, 1), b"a"); + assert_eq!(utf8_prefix(bytes, 3), bytes); + + // A four-byte sequence followed by an ASCII byte. + let bytes = "😀z".as_bytes(); + for limit in 1..4 { + assert_eq!(utf8_prefix(bytes, limit), b"", "limit {limit}"); + } + assert_eq!(utf8_prefix(bytes, 4), "😀".as_bytes()); + assert_eq!(utf8_prefix(bytes, 5), bytes); + + // The cut lands between two multi-byte characters. + let bytes = "éé".as_bytes(); + assert_eq!(utf8_prefix(bytes, 2), "é".as_bytes()); + assert_eq!(utf8_prefix(bytes, 3), "é".as_bytes()); +} + +#[test] +fn utf8_prefix_of_invalid_bytes_moves_back_at_most_three() { + // Continuation bytes with no lead byte are not valid UTF-8. The cut moves back over at most + // three of them, and never runs off the start. + assert_eq!(utf8_prefix(&[0x80, 0x80, 0x80], 2), b""); + assert_eq!(utf8_prefix(&[0x61, 0x80, 0x80, 0x80], 2), b""); + assert_eq!(utf8_prefix(&[0x80; 8], 6), &[0x80; 3]); + assert_eq!(utf8_prefix(&[0x80; 8], 3), b""); +} + +#[test] +fn error_message_for_v8_keeps_a_short_message() { + assert_eq!(error_message_for_v8(b"boom"), b"boom"); + assert_eq!(error_message_for_v8(b""), b""); +} + +#[test] +fn error_message_for_v8_cuts_to_v8s_limit() { + let max = usize::try_from(v8::String::MAX_LENGTH).unwrap(); + // Zeroed, so that the allocation costs no physical memory until it is read, and the cut + // reads one byte of it. + let message = vec![0u8; max + 1]; + assert_eq!(error_message_for_v8(&message).len(), max); + assert_eq!(error_message_for_v8(&message[..max]).len(), max); +} diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index 3897896ed89..a634ab10da9 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -294,6 +294,13 @@ impl Error { } } + /// The error for a string longer than [`v8::String::MAX_LENGTH`]: a `RangeError` with the + /// message V8 gives such a string, "Invalid string length". It is not pending. + #[must_use] + pub fn invalid_string_length() -> Self { + Self::new_range_error("Invalid string length") + } + /// Returns whether this error stands for an exception or termination that is pending in V8, /// as after a call into JavaScript that threw. /// @@ -314,27 +321,11 @@ impl Error { /// /// This always builds the exception from `self.message` verbatim: it does /// not check `is_internal` / redact anything. Prefer `Lock::throw_exception()`, - /// which does perform that check before falling back to this method. - #[expect( - unsafe_code, - reason = "creates the V8 exception object through the FFI" - )] + /// which does perform that check before falling back to this method. A message longer + /// than V8's string limit is cut to fit; see [`Lock::error`]. pub fn to_local<'s>(&self, lock: &mut Lock<'s>) -> v8::Local<'s, v8::Value> { - let isolate = lock.isolate(); - let message = self.message.as_bytes(); - // Strings longer than i32::MAX bytes are truncated to satisfy V8's - // int-typed length parameter; V8 rejects strings that large anyway. - let len = i32::try_from(message.len()).unwrap_or(i32::MAX); - // SAFETY: isolate is valid and locked (guaranteed by caller); message - // pointer and len describe a valid byte slice. - unsafe { - v8::Local::from_ffi(v8::ffi::exception_create_from_bytes( - isolate.as_ffi(), - self.name, - message.as_ptr(), - len, - )) - } + lock.create_error_from_bytes(self.name, self.message.as_bytes()) + .into() } } @@ -574,6 +565,11 @@ impl<'s> Lock<'s> { /// /// Mirrors C++ `jsg::Lock::error()`. The returned object can have additional /// properties attached before being thrown or returned. + /// + /// A message longer than [`v8::String::MAX_LENGTH`] bytes is cut to fit, at a character + /// boundary, rather than refused. A message that long is a bug or embeds user data, and + /// the cut keeps its start. So the length of the message never makes this, + /// [`Lock::throw_exception`] or the other error constructors fail. pub fn error(&mut self, message: &str) -> v8::Local<'s, v8::Object> { self.create_error(ExceptionType::Error, message) } @@ -629,27 +625,32 @@ impl<'s> Lock<'s> { exception_type: ExceptionType, message: &[u8], ) -> v8::Local<'s, v8::Object> { - // Strings longer than i32::MAX bytes are truncated to satisfy V8's - // int-typed length parameter; V8 rejects strings that large anyway. - let len = i32::try_from(message.len()).unwrap_or(i32::MAX); - // SAFETY: Lock guarantees the isolate is valid and locked; message.as_ptr() - // and len describe a valid byte slice. V8's error constructors always return an - // object. + let message = error_message_for_v8(message); + // SAFETY: Lock guarantees the isolate is valid and locked. V8's error constructors + // always return an object. let error: v8::Local<'s, v8::Object> = unsafe { v8::Local::from_ffi(v8::ffi::exception_create_from_bytes( self.isolate().as_ffi(), exception_type, - message.as_ptr(), - len, + message, )) }; debug_assert!(error.is_object()); error } + /// Throws a JavaScript `Error` with the given message, as [`Lock::error`] creates it. + /// + /// Throws nothing while execution is terminating; see [`Lock::throw_exception`]. pub fn throw_error(&mut self, message: &str) { - // SAFETY: Lock guarantees the isolate is valid and locked - unsafe { v8::ffi::isolate_throw_error(self.isolate().as_ffi(), message) } + if self.is_execution_terminating() { + // `isolate_throw_exception` would throw nothing, so create nothing. + return; + } + let error = self.error(message); + // SAFETY: Lock guarantees the isolate is valid and locked; `error` is a handle of its + // scope. + unsafe { v8::ffi::isolate_throw_exception(self.isolate().as_ffi(), error.into_ffi()) } } /// Runs `fut` as I/O and later calls `callback` with its output under a new lock. Both run @@ -752,7 +753,10 @@ impl<'s> Lock<'s> { /// # Example /// /// ```ignore - /// match lock.try_catch(|lock| callback.call::(lock, None, &[]))? { + /// match lock.try_catch(|lock| { + /// let result = callback.call(lock, None, &[])?; + /// f64::from_js(lock, result) + /// })? { /// Ok(result) => { /* ... */ } /// Err(exception) => { /* handle what the callback threw */ } /// } @@ -1269,6 +1273,27 @@ fn panic_message(payload: &(dyn std::any::Any + Send)) -> &str { } } +/// The part of an error message that V8 can hold as a string: all of `message` if it is at most +/// [`v8::String::MAX_LENGTH`] bytes, otherwise a prefix of that many bytes or fewer, cut before +/// a UTF-8 sequence rather than inside one. See [`Lock::error`]. +pub(crate) fn error_message_for_v8(message: &[u8]) -> &[u8] { + utf8_prefix(message, v8::String::MAX_LENGTH as usize) +} + +/// The longest prefix of `bytes` of at most `max_len` bytes that does not end inside a UTF-8 +/// sequence. A continuation byte is one of the form `10xxxxxx`, and a sequence has at most +/// three of them, so the cut moves back at most three bytes; `bytes` need not be valid UTF-8. +fn utf8_prefix(bytes: &[u8], max_len: usize) -> &[u8] { + if bytes.len() <= max_len { + return bytes; + } + let mut end = max_len; + while end > max_len.saturating_sub(3) && bytes[end] & 0b1100_0000 == 0b1000_0000 { + end -= 1; + } + &bytes[..end] +} + #[cfg(test)] #[path = "lib-test.rs"] mod tests; diff --git a/src/rust/jsg/v8-test.rs b/src/rust/jsg/v8-test.rs new file mode 100644 index 00000000000..edeb65335a8 --- /dev/null +++ b/src/rust/jsg/v8-test.rs @@ -0,0 +1,25 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +use super::*; + +#[test] +fn v8_length_accepts_up_to_max_length() { + assert_eq!(String::v8_length(0), Some(0)); + assert_eq!(String::v8_length(1), Some(1)); + let max = usize::try_from(String::MAX_LENGTH).unwrap(); + assert_eq!(String::v8_length(max), Some(String::MAX_LENGTH)); +} + +#[test] +fn v8_length_refuses_longer_input() { + let max = usize::try_from(String::MAX_LENGTH).unwrap(); + assert_eq!(String::v8_length(max + 1), None); + // Longer than V8's `int` length parameter, which must never see it. + assert_eq!( + String::v8_length(usize::try_from(i32::MAX).unwrap() + 1), + None + ); + assert_eq!(String::v8_length(usize::MAX), None); +} diff --git a/src/rust/jsg/v8.rs b/src/rust/jsg/v8.rs index 93c3c115976..0968c8e10ce 100644 --- a/src/rust/jsg/v8.rs +++ b/src/rust/jsg/v8.rs @@ -50,7 +50,6 @@ use std::sync::Arc; pub use self::ffi::PromiseState; use crate::Error; -use crate::FromJS; use crate::GarbageCollected; use crate::Lock; use crate::Resource; @@ -184,7 +183,6 @@ pub mod ffi { // Local pub unsafe fn local_to_global(isolate: *mut Isolate, value: Local) -> Global; pub unsafe fn local_new_number(isolate: *mut Isolate, value: f64) -> Local; - pub unsafe fn local_new_string(isolate: *mut Isolate, value: &str) -> Local; pub unsafe fn local_new_boolean(isolate: *mut Isolate, value: bool) -> Local; pub unsafe fn local_new_object(isolate: *mut Isolate) -> Local; pub unsafe fn local_new_null(isolate: *mut Isolate) -> Local; @@ -654,7 +652,7 @@ pub mod ffi { pub unsafe fn fast_api_throw_exception( options: *mut FastApiCallbackOptions, exception_type: ExceptionType, - message: &str, + message: &[u8], ); pub unsafe fn fast_api_throw_internal_error( options: *mut FastApiCallbackOptions, @@ -676,11 +674,11 @@ pub mod ffi { pub unsafe fn value_deserialize(isolate: *mut Isolate, data: &[u8]) -> Result; // Errors + // Infallible: a message longer than V8's string limit is cut to fit (see ffi.h). pub unsafe fn exception_create_from_bytes( isolate: *mut Isolate, exception_type: ExceptionType, - data: *const u8, - length: i32, + message: &[u8], ) -> Local; pub unsafe fn exception_create_internal( isolate: *mut Isolate, @@ -697,7 +695,6 @@ pub mod ffi { // Isolate pub unsafe fn isolate_throw_exception(isolate: *mut Isolate, exception: Local); - pub unsafe fn isolate_throw_error(isolate: *mut Isolate, message: &str); pub unsafe fn isolate_throw_internal_error(isolate: *mut Isolate, internal_message: &str); pub unsafe fn isolate_terminate_execution(isolate: *mut Isolate); pub unsafe fn isolate_is_execution_terminating(isolate: *mut Isolate) -> bool; @@ -960,14 +957,27 @@ pub struct Date; impl String { /// Maximum length of a V8 string in UTF-16 code units. /// - /// Matches `v8::String::kMaxLength`. Attempting to create a string longer than - /// this will cause V8 to return `None`. + /// Matches `v8::String::kMaxLength`. The constructors return `None` for input longer than + /// this, and [`ToJS`](crate::ToJS) for `&str` and `String` fails with a `RangeError`. pub const MAX_LENGTH: i32 = if cfg!(target_pointer_width = "32") { (1 << 28) - 16 } else { (1 << 29) - 24 }; + /// The length V8 takes for input of `len` units (bytes or UTF-16 code units), or `None` if + /// the input is longer than [`Self::MAX_LENGTH`], which V8 refuses with no exception pending. + /// + /// For UTF-8 input the units are bytes, of which a string never has fewer than UTF-16 code + /// units, so the input fits whenever the check passes. Checking here keeps a length above + /// `i32::MAX` from reaching V8's `int` parameter, where a negative value means + /// "NUL-terminated". + fn v8_length(len: usize) -> Option { + i32::try_from(len) + .ok() + .filter(|&len| len <= Self::MAX_LENGTH) + } + /// Returns the empty string singleton. /// /// Corresponds to `v8::String::Empty()`. @@ -979,6 +989,8 @@ impl String { /// Creates a new string from a `&str`. /// + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] bytes. + /// /// Corresponds to `v8::String::NewFromUtf8`. pub fn new_from_str<'s>(lock: &mut crate::Lock<'s>, data: &str) -> Option> { Self::new_from_utf8(lock, data.as_bytes()) @@ -1020,16 +1032,13 @@ impl String { /// Creates a new string from UTF-8 data. /// - /// Returns `None` if V8 cannot allocate the string. - /// - /// Strings longer than `i32::MAX` bytes are silently truncated to `i32::MAX` bytes - /// to satisfy V8's `int`-typed length parameter; in practice V8 will reject strings - /// that large anyway due to heap limits. + /// Returns `None`, with no exception pending, if `data` is longer than + /// [`Self::MAX_LENGTH`] bytes. Invalid sequences become U+FFFD. /// /// Corresponds to `v8::String::NewFromUtf8`. pub fn new_from_utf8<'s>(lock: &mut crate::Lock<'s>, data: &[u8]) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked and a HandleScope is active; // data.as_ptr() and len describe a valid byte slice. let handle = @@ -1043,7 +1052,7 @@ impl String { /// Equal strings will be pointer-equal after internalization, which speeds up /// property-key lookups at the cost of a hash-table probe on creation. /// - /// Strings longer than `i32::MAX` bytes are silently truncated to `i32::MAX` bytes. + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] bytes. /// /// Corresponds to `v8::String::NewFromUtf8` with `kInternalized`. pub fn new_internalized_from_utf8<'s>( @@ -1051,7 +1060,7 @@ impl String { data: &[u8], ) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked and a HandleScope is active; // data.as_ptr() and len describe a valid byte slice. let handle = @@ -1063,9 +1072,7 @@ impl String { /// Creates a new string from Latin-1 (one-byte) data. /// /// Each byte is mapped to the Unicode code point with the same value. - /// Returns `None` if V8 cannot allocate the string. - /// - /// Strings longer than `i32::MAX` bytes are silently truncated to `i32::MAX` bytes. + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] bytes. /// /// Corresponds to `v8::String::NewFromOneByte`. pub fn new_from_one_byte<'s>( @@ -1073,7 +1080,7 @@ impl String { data: &[u8], ) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked; data.as_ptr() and len are valid. let handle = unsafe { ffi::local_string_new_from_one_byte(isolate.as_ffi(), data.as_ptr(), len, false) @@ -1085,7 +1092,7 @@ impl String { /// Creates an internalized string from Latin-1 (one-byte) data. /// /// Equal strings will be pointer-equal after internalization. - /// Strings longer than `i32::MAX` bytes are silently truncated to `i32::MAX` bytes. + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] bytes. /// /// Corresponds to `v8::String::NewFromOneByte` with `kInternalized`. pub fn new_internalized_from_one_byte<'s>( @@ -1093,7 +1100,7 @@ impl String { data: &[u8], ) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked; data.as_ptr() and len are valid. let handle = unsafe { ffi::local_string_new_from_one_byte(isolate.as_ffi(), data.as_ptr(), len, true) @@ -1104,9 +1111,7 @@ impl String { /// Creates a new string from UTF-16 data. /// - /// Returns `None` if V8 cannot allocate the string. - /// - /// Strings longer than `i32::MAX` code units are silently truncated to `i32::MAX` code units. + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] code units. /// /// Corresponds to `v8::String::NewFromTwoByte`. pub fn new_from_two_byte<'s>( @@ -1114,7 +1119,7 @@ impl String { data: &[u16], ) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked; data.as_ptr() and len are valid. let handle = unsafe { ffi::local_string_new_from_two_byte(isolate.as_ffi(), data.as_ptr(), len, false) @@ -1126,7 +1131,7 @@ impl String { /// Creates an internalized string from UTF-16 data. /// /// Equal strings will be pointer-equal after internalization. - /// Strings longer than `i32::MAX` code units are silently truncated to `i32::MAX` code units. + /// Returns `None` if `data` is longer than [`Self::MAX_LENGTH`] code units. /// /// Corresponds to `v8::String::NewFromTwoByte` with `kInternalized`. pub fn new_internalized_from_two_byte<'s>( @@ -1134,7 +1139,7 @@ impl String { data: &[u16], ) -> Option> { let isolate = lock.isolate(); - let len = i32::try_from(data.len()).unwrap_or(i32::MAX); + let len = Self::v8_length(data.len())?; // SAFETY: Lock guarantees the isolate is locked; data.as_ptr() and len are valid. let handle = unsafe { ffi::local_string_new_from_two_byte(isolate.as_ffi(), data.as_ptr(), len, true) @@ -1928,37 +1933,37 @@ where } impl<'s> Local<'s, Function> { - /// Calls this function and converts the result via [`FromJS`]. + /// Calls this function and returns what it returns, as a handle. /// /// `receiver` is the `this` value; pass `None` for `undefined`. /// - /// `args` is a slice of `Local` — use [`ToJS::to_js`](crate::ToJS::to_js) to - /// convert Rust values, or `.into()` for other `Local` handles. + /// `args` is a slice of `Local`: use [`ToJS::to_js`](crate::ToJS::to_js) to convert + /// Rust values, or `.into()` for other `Local` handles. To get a Rust value back, convert + /// the result with [`FromJS::from_js`](crate::FromJS::from_js), as the example does. /// /// # Errors /// /// If the function throws, or execution is terminating (then without calling it), the - /// exception stays pending and this - /// returns an error for which [`Error::is_pending`] is true. [`Lock::throw_exception`] throws - /// nothing for it, so - /// returning it from a callback propagates the exception to the callback's JavaScript caller, - /// as C++ JSG does. Don't call into JavaScript after such an error; to handle the exception - /// instead, make the call in [`Lock::try_catch`]. Also fails if the result does not convert - /// to `R`. + /// exception stays pending and this returns an error for which [`Error::is_pending`] is + /// true. [`Lock::throw_exception`] throws nothing for it, so returning it from a callback + /// propagates the exception to the callback's JavaScript caller, as C++ JSG does. Don't call + /// into JavaScript after such an error; to handle the exception instead, make the call in + /// [`Lock::try_catch`]. /// /// # Example /// /// ```ignore /// let x = x.to_js(lock)?; /// let y = y.to_js(lock)?; - /// let result: f64 = func.call(lock, None, &[x, y])?; + /// let result = func.call(lock, None, &[x, y])?; + /// let sum = f64::from_js(lock, result)?; /// ``` - pub fn call>( + pub fn call( self, lock: &mut Lock<'s>, receiver: Option>, args: &[Local<'s, Value>], - ) -> Result { + ) -> Result, Error> { let recv = receiver.unwrap_or_else(|| lock.undefined()); self.debug_assert_isolate(lock); recv.debug_assert_isolate(lock); @@ -1966,7 +1971,7 @@ impl<'s> Local<'s, Function> { arg.debug_assert_isolate(lock); } - let result = call_into_js(lock, |lock| { + call_into_js(lock, |lock| { // SAFETY: lock guarantees the isolate is locked and a HandleScope is active. // self is a valid Local; recv and args are valid Local handles. The // result belongs to the lock's scope. @@ -1978,8 +1983,7 @@ impl<'s> Local<'s, Function> { Local::raw_slice(args), )) }) - })?; - R::from_js(lock, result) + }) } } @@ -4955,3 +4959,7 @@ impl WrappableRc { self.resolve::() } } + +#[cfg(test)] +#[path = "v8-test.rs"] +mod tests; diff --git a/src/rust/jsg/wrappable.rs b/src/rust/jsg/wrappable.rs index 48f08c0ce6f..0b87303532a 100644 --- a/src/rust/jsg/wrappable.rs +++ b/src/rust/jsg/wrappable.rs @@ -391,10 +391,12 @@ impl Traced for std::rc::Rc { /// /// # Errors /// -/// The conversions in this crate fail only with an error for which [`Error::is_pending`] is -/// true. Creating a `Date` or a resource's wrapper fails, leaving the exception pending, when V8 -/// throws or execution is terminating, and a container passes on the first error from converting -/// an element. A conversion implemented elsewhere may return any `Error`. +/// A `&str` or `String` longer than [`v8::String::MAX_LENGTH`] bytes fails with +/// [`Error::invalid_string_length`], a `RangeError` that is not pending. Every other conversion +/// of a single value in this crate fails only with an error for which [`Error::is_pending`] is +/// true: creating a `Date` or a resource's wrapper fails, leaving the exception pending, when +/// V8 throws or execution is terminating. A container passes on the first error from converting +/// an element, whichever kind it is. A conversion implemented elsewhere may return any `Error`. /// /// A `#[jsg_method]`, property getter or wildcard property throws an error from converting its /// return value with [`Lock::throw_exception`], as it throws an `Err` the method returns. The @@ -535,11 +537,13 @@ impl<'s> ToJS<'s> for std::string::String { } impl<'s> ToJS<'s> for &str { + /// Fails with [`Error::invalid_string_length`] if the string is longer than + /// [`v8::String::MAX_LENGTH`] bytes. #[inline] fn to_js(self, lock: &mut Lock<'s>) -> Result, Error> { - let isolate = lock.isolate(); - // SAFETY: `lock` proves the isolate is locked and a HandleScope for `'s` is open. - Ok(unsafe { v8::Local::from_ffi(v8::ffi::local_new_string(isolate.as_ffi(), self)) }) + v8::String::new_from_str(lock, self) + .map(Into::into) + .ok_or_else(Error::invalid_string_length) } }