Skip to content

Commit

Permalink
fix: FastString - v8_string should error when cannot allocate (#986)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Dec 9, 2024
1 parent eefb5c0 commit 0ff818e
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 53 deletions.
24 changes: 16 additions & 8 deletions core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub fn to_v8_error<'a>(
pub(crate) fn call_site_evals_key<'a>(
scope: &mut v8::HandleScope<'a>,
) -> v8::Local<'a, v8::Private> {
let name = v8_static_strings::CALL_SITE_EVALS.v8_string(scope);
let name = v8_static_strings::CALL_SITE_EVALS.v8_string(scope).unwrap();
v8::Private::for_api(scope, Some(name))
}

Expand Down Expand Up @@ -432,7 +432,7 @@ fn get_property<'a>(
object: v8::Local<v8::Object>,
key: FastStaticString,
) -> Option<v8::Local<'a, v8::Value>> {
let key = key.v8_string(scope);
let key = key.v8_string(scope).unwrap();
object.get(scope, key.into())
}

Expand Down Expand Up @@ -1015,7 +1015,7 @@ v8_static_strings::v8_static_strings! {
pub(crate) fn original_call_site_key<'a>(
scope: &mut v8::HandleScope<'a>,
) -> v8::Local<'a, v8::Private> {
let name = ORIGINAL.v8_string(scope);
let name = ORIGINAL.v8_string(scope).unwrap();
v8::Private::for_api(scope, Some(name))
}

Expand All @@ -1040,7 +1040,9 @@ fn original_call_site<'a>(
.get_private(scope, orig_key)
.and_then(|v| v8::Local::<v8::Object>::try_from(v).ok())
else {
let message = ERROR_RECEIVER_IS_NOT_VALID_CALLSITE_OBJECT.v8_string(scope);
let message = ERROR_RECEIVER_IS_NOT_VALID_CALLSITE_OBJECT
.v8_string(scope)
.unwrap();
let exception = v8::Exception::type_error(scope, message);
scope.throw_exception(exception);
return None;
Expand All @@ -1058,7 +1060,7 @@ macro_rules! make_callsite_fn {
let Some(orig) = original_call_site(scope, args.this()) else {
return;
};
let key = $field.v8_string(scope).into();
let key = $field.v8_string(scope).unwrap().into();
let orig_ret = orig
.cast::<v8::Object>()
.get(scope, key)
Expand Down Expand Up @@ -1111,7 +1113,10 @@ pub mod callsite_fns {
// strip off `file://`
let string = ret_val.to_rust_string_lossy(scope);
if let Some(file_name) = maybe_to_path_str(&string) {
let v8_str = crate::FastString::from(file_name).v8_string(scope).into();
let v8_str = crate::FastString::from(file_name)
.v8_string(scope)
.unwrap()
.into();
rv.set(v8_str);
} else {
rv.set(ret_val.into());
Expand Down Expand Up @@ -1161,7 +1166,10 @@ pub mod callsite_fns {
let orig_file_name = serde_v8::to_utf8(orig_file_name, scope);
if let Some(file_name) = maybe_to_path_str(&orig_file_name) {
let to_string = orig_to_string.replace(&orig_file_name, &file_name);
let v8_str = crate::FastString::from(to_string).v8_string(scope).into();
let v8_str = crate::FastString::from(to_string)
.v8_string(scope)
.unwrap()
.into();
rv.set(v8_str);
} else {
rv.set(orig_to_string_v8.into());
Expand Down Expand Up @@ -1193,7 +1201,7 @@ pub(crate) fn make_callsite_prototype<'s>(

macro_rules! set_attr {
($scope:ident, $template:ident, $fn:ident, $field:ident) => {
let key = $field.v8_string($scope).into();
let key = $field.v8_string($scope).unwrap().into();
$template.set_with_attr(
key,
v8::FunctionBuilder::<v8::FunctionTemplate>::new(callsite_fns::$fn)
Expand Down
31 changes: 23 additions & 8 deletions core/fast_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use serde::Deserializer;
use serde::Serializer;
use std::borrow::Borrow;
use std::convert::Infallible;
use std::ffi::OsStr;
use std::fmt::Debug;
use std::fmt::Display;
Expand Down Expand Up @@ -50,7 +49,7 @@ impl FastStaticString {
pub fn v8_string<'s>(
&self,
scope: &mut v8::HandleScope<'s>,
) -> v8::Local<'s, v8::String> {
) -> Result<v8::Local<'s, v8::String>, FastStringV8AllocationError> {
FastString::from(*self).v8_string(scope)
}

Expand Down Expand Up @@ -122,6 +121,20 @@ impl Display for FastStaticString {
}
}

#[derive(Debug)]
pub struct FastStringV8AllocationError;

impl std::error::Error for FastStringV8AllocationError {}

impl std::fmt::Display for FastStringV8AllocationError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
f,
"failed to allocate string; buffer exceeds maximum length"
)
}
}

/// Module names and code can be sourced from strings or bytes that are either owned or borrowed. This enumeration allows us
/// to perform a minimal amount of cloning and format-shifting of the underlying data.
///
Expand Down Expand Up @@ -287,17 +300,19 @@ impl FastString {
pub fn v8_string<'a>(
&self,
scope: &mut v8::HandleScope<'a>,
) -> v8::Local<'a, v8::String> {
) -> Result<v8::Local<'a, v8::String>, FastStringV8AllocationError> {
match self.inner {
FastStringInner::StaticAscii(s) => {
v8::String::new_external_onebyte_static(scope, s.as_bytes()).unwrap()
v8::String::new_external_onebyte_static(scope, s.as_bytes())
.ok_or(FastStringV8AllocationError)
}
FastStringInner::StaticConst(s) => {
v8::String::new_from_onebyte_const(scope, s.s).unwrap()
v8::String::new_from_onebyte_const(scope, s.s)
.ok_or(FastStringV8AllocationError)
}
_ => {
v8::String::new_from_utf8(scope, self.as_bytes(), NewStringType::Normal)
.unwrap()
.ok_or(FastStringV8AllocationError)
}
}
}
Expand Down Expand Up @@ -443,14 +458,14 @@ impl From<FastString> for Arc<str> {
}

impl<'s> ToV8<'s> for FastString {
type Error = Infallible;
type Error = FastStringV8AllocationError;

#[inline]
fn to_v8(
self,
scope: &mut v8::HandleScope<'s>,
) -> Result<v8::Local<'s, v8::Value>, Self::Error> {
Ok(self.v8_string(scope).into())
Ok(self.v8_string(scope)?.into())
}
}

Expand Down
8 changes: 4 additions & 4 deletions core/modules/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,11 @@ impl ModuleMap {
exports: Vec<(FastStaticString, v8::Local<v8::Value>)>,
) -> Result<ModuleId, ModuleError> {
let name = name.into_module_name();
let name_str = name.v8_string(scope);
let name_str = name.v8_string(scope).unwrap();

let export_names = exports
.iter()
.map(|(name, _)| name.v8_string(scope))
.map(|(name, _)| name.v8_string(scope).unwrap())
.collect::<Vec<_>>();
let module = v8::Module::create_synthetic_module(
scope,
Expand Down Expand Up @@ -605,8 +605,8 @@ impl ModuleMap {
}
}

let name_str = name.v8_string(scope);
let source_str = source.v8_string(scope);
let name_str = name.v8_string(scope).unwrap();
let source_str = source.v8_string(scope).unwrap();
let host_defined_options = self
.loader
.borrow()
Expand Down
4 changes: 2 additions & 2 deletions core/ops_builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,8 @@ fn op_import_sync<'s>(

let scope = &mut v8::TryCatch::new(scope);

let default = v8_static_strings::DEFAULT.v8_string(scope);
let es_module = v8_static_strings::ESMODULE.v8_string(scope);
let default = v8_static_strings::DEFAULT.v8_string(scope).unwrap();
let es_module = v8_static_strings::ESMODULE.v8_string(scope).unwrap();
// If the module has a default export and no __esModule export, wrap it.
if namespace.has_own_property(scope, default.into()) == Some(true)
&& namespace.has_own_property(scope, es_module.into()) == Some(false)
Expand Down
Loading

0 comments on commit 0ff818e

Please sign in to comment.