Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-09-10 15:08:21 +01:00
Code Issues Projects Releases Packages Wiki Activity

script: Make get_property_jsval a safe function (#39137)

Browse source 
Accept the safe `JSContext` wraper to this function so that it can be
safe. Some callers also become safe as well.

Testing: This does not change behavior and is thus covered by existing
tests.
Fixes: #39129.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-09-04 16:11:15 -07:00 • committed by GitHub
parent 589a750cac
commit e5fbb31452
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 59 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

43
components/script/dom/bindings/conversions.rs
View file

@ -37,11 +37,12 @@ use std::ffi;
pub(crate) use js::conversions::{ pub(crate) use js::conversions::{
ConversionBehavior, ConversionResult, FromJSValConvertible, ToJSValConvertible, ConversionBehavior, ConversionResult, FromJSValConvertible, ToJSValConvertible,
}; };
use js::jsapi::{JS_IsExceptionPending, JSContext, JSObject}; use js::jsapi::{JS_IsExceptionPending, JSContext as RawJSContext, JSObject};
use js::jsval::UndefinedValue; use js::jsval::UndefinedValue;
use js::rust::wrappers::{JS_GetProperty, JS_HasProperty}; use js::rust::wrappers::{JS_GetProperty, JS_HasProperty};
use js::rust::{HandleObject, MutableHandleValue}; use js::rust::{HandleObject, MutableHandleValue};
pub(crate) use script_bindings::conversions::{is_dom_proxy, *}; pub(crate) use script_bindings::conversions::{is_dom_proxy, *};
use script_bindings::script_runtime::JSContext;
use crate::dom::bindings::error::{Error, Fallible}; use crate::dom::bindings::error::{Error, Fallible};
use crate::dom::bindings::reflector::DomObject; use crate::dom::bindings::reflector::DomObject;
@ -63,7 +64,7 @@ where
/// Get a `DomRoot<T>` for a DOM object accessible from a `HandleObject`. /// Get a `DomRoot<T>` for a DOM object accessible from a `HandleObject`.
pub(crate) fn root_from_handleobject<T>( pub(crate) fn root_from_handleobject<T>(
obj: HandleObject, obj: HandleObject,
cx: *mut JSContext, cx: *mut RawJSContext,
) -> Result<DomRoot<T>, ()> ) -> Result<DomRoot<T>, ()>
where where
T: DomObject + IDLInterface, T: DomObject + IDLInterface,
@ -72,36 +73,38 @@ where
} }
/// Get a property from a JS object. /// Get a property from a JS object.
pub(crate) unsafe fn get_property_jsval( pub(crate) fn get_property_jsval(
cx: *mut JSContext, cx: JSContext,
object: HandleObject, object: HandleObject,
name: &str, name: &str,
mut rval: MutableHandleValue, mut rval: MutableHandleValue,
) -> Fallible<()> { ) -> Fallible<()> {
rval.set(UndefinedValue()); rval.set(UndefinedValue());
let cname = match ffi::CString::new(name) {
Ok(cname) => cname, let Ok(cname) = ffi::CString::new(name) else {
Err(_) => return Ok(()), return Ok(());
}; };
let mut found = false; let mut found = false;
unsafe { unsafe {
if JS_HasProperty(cx, object, cname.as_ptr(), &mut found) && found { if !JS_HasProperty(*cx, object, cname.as_ptr(), &mut found) || !found {
JS_GetProperty(cx, object, cname.as_ptr(), rval); if JS_IsExceptionPending(*cx) {
if JS_IsExceptionPending(cx) {
return Err(Error::JSFailed); return Err(Error::JSFailed);
} }
Ok(()) return Ok(());
} else if JS_IsExceptionPending(cx) {
Err(Error::JSFailed)
} else {
Ok(())
} }
JS_GetProperty(*cx, object, cname.as_ptr(), rval);
if JS_IsExceptionPending(*cx) {
return Err(Error::JSFailed);
}
Ok(())
} }
} }
/// Get a property from a JS object, and convert it to a Rust value. /// Get a property from a JS object, and convert it to a Rust value.
pub(crate) unsafe fn get_property<T>( pub(crate) fn get_property<T>(
cx: *mut JSContext, cx: JSContext,
object: HandleObject, object: HandleObject,
name: &str, name: &str,
option: T::Config, option: T::Config,
@ -110,14 +113,14 @@ where
T: FromJSValConvertible, T: FromJSValConvertible,
{ {
debug!("Getting property {}.", name); debug!("Getting property {}.", name);
rooted!(in(cx) let mut result = UndefinedValue()); rooted!(in(*cx) let mut result = UndefinedValue());
unsafe { get_property_jsval(cx, object, name, result.handle_mut())? }; get_property_jsval(cx, object, name, result.handle_mut())?;
if result.is_undefined() { if result.is_undefined() {
debug!("No property {}.", name); debug!("No property {}.", name);
return Ok(None); return Ok(None);
} }
debug!("Converting property {}.", name); debug!("Converting property {}.", name);
let value = unsafe { T::from_jsval(cx, result.handle(), option) }; let value = unsafe { T::from_jsval(*cx, result.handle(), option) };
match value { match value {
Ok(ConversionResult::Success(value)) => Ok(Some(value)), Ok(ConversionResult::Success(value)) => Ok(Some(value)),
Ok(ConversionResult::Failure(_)) => Ok(None), Ok(ConversionResult::Failure(_)) => Ok(None),

22
components/script/dom/paintworkletglobalscope.rs
View file

@ -513,20 +513,17 @@ impl PaintWorkletGlobalScopeMethods<crate::DomTypeHolder> for PaintWorkletGlobal
// Step 4-6. // Step 4-6.
let mut property_names: Vec<String> = let mut property_names: Vec<String> =
unsafe { get_property(*cx, paint_obj.handle(), "inputProperties", ()) }? get_property(cx, paint_obj.handle(), "inputProperties", ())?.unwrap_or_default();
.unwrap_or_default();
let properties = property_names.drain(..).map(Atom::from).collect(); let properties = property_names.drain(..).map(Atom::from).collect();
// Step 7-9. // Step 7-9.
let input_arguments: Vec<String> = let input_arguments: Vec<String> =
unsafe { get_property(*cx, paint_obj.handle(), "inputArguments", ()) }? get_property(cx, paint_obj.handle(), "inputArguments", ())?.unwrap_or_default();
.unwrap_or_default();
// TODO: Steps 10-11. // TODO: Steps 10-11.
// Steps 12-13. // Steps 12-13.
let alpha: bool = let alpha: bool = get_property(cx, paint_obj.handle(), "alpha", ())?.unwrap_or(true);
unsafe { get_property(*cx, paint_obj.handle(), "alpha", ()) }?.unwrap_or(true);
// Step 14 // Step 14
if unsafe { !IsConstructor(paint_obj.get()) } { if unsafe { !IsConstructor(paint_obj.get()) } {
@ -535,9 +532,7 @@ impl PaintWorkletGlobalScopeMethods<crate::DomTypeHolder> for PaintWorkletGlobal
// Steps 15-16 // Steps 15-16
rooted!(in(*cx) let mut prototype = UndefinedValue()); rooted!(in(*cx) let mut prototype = UndefinedValue());
unsafe { get_property_jsval(cx, paint_obj.handle(), "prototype", prototype.handle_mut())?;
get_property_jsval(*cx, paint_obj.handle(), "prototype", prototype.handle_mut())?;
}
if !prototype.is_object() { if !prototype.is_object() {
return Err(Error::Type(String::from("Prototype is not an object."))); return Err(Error::Type(String::from("Prototype is not an object.")));
} }
@ -545,14 +540,7 @@ impl PaintWorkletGlobalScopeMethods<crate::DomTypeHolder> for PaintWorkletGlobal
// Steps 17-18 // Steps 17-18
rooted!(in(*cx) let mut paint_function = UndefinedValue()); rooted!(in(*cx) let mut paint_function = UndefinedValue());
unsafe { get_property_jsval(cx, prototype.handle(), "paint", paint_function.handle_mut())?;
get_property_jsval(
*cx,
prototype.handle(),
"paint",
paint_function.handle_mut(),
)?;
}
if !paint_function.is_object() || unsafe { !IsCallable(paint_function.to_object()) } { if !paint_function.is_object() || unsafe { !IsCallable(paint_function.to_object()) } {
return Err(Error::Type(String::from("Paint function is not callable."))); return Err(Error::Type(String::from("Paint function is not callable.")));
} }

69
components/script/webdriver_handlers.rs
View file

@ -457,7 +457,13 @@ unsafe fn jsval_to_webdriver_inner(
Err(WebDriverJSError::JSError) Err(WebDriverJSError::JSError)
} }
} else { } else {
clone_an_object(cx, global_scope, val, seen, object.handle()) clone_an_object(
SafeJSContext::from_ptr(cx),
global_scope,
val,
seen,
object.handle(),
)
} }
} else { } else {
Err(WebDriverJSError::UnknownType) Err(WebDriverJSError::UnknownType)
@ -467,7 +473,7 @@ unsafe fn jsval_to_webdriver_inner(
#[allow(unsafe_code)] #[allow(unsafe_code)]
/// <https://w3c.github.io/webdriver/#dfn-clone-an-object> /// <https://w3c.github.io/webdriver/#dfn-clone-an-object>
unsafe fn clone_an_object( unsafe fn clone_an_object(
cx: *mut JSContext, cx: SafeJSContext,
global_scope: &GlobalScope, global_scope: &GlobalScope,
val: HandleValue, val: HandleValue,
seen: &mut HashSet<HashableJSVal>, seen: &mut HashSet<HashableJSVal>,
@ -482,49 +488,38 @@ unsafe fn clone_an_object(
seen.insert(hashable.clone()); seen.insert(hashable.clone());
let return_val = if unsafe { let return_val = if unsafe {
is_array_like::<crate::DomTypeHolder>(cx, val) || is_arguments_object(cx, val) is_array_like::<crate::DomTypeHolder>(*cx, val) || is_arguments_object(*cx, val)
} { } {
let mut result: Vec<JSValue> = Vec::new(); let mut result: Vec<JSValue> = Vec::new();
let get_property_result = unsafe { let get_property_result =
get_property::<u32>(cx, object_handle, "length", ConversionBehavior::Default) get_property::<u32>(cx, object_handle, "length", ConversionBehavior::Default);
};
let length = match get_property_result { let length = match get_property_result {
Ok(length) => match length { Ok(length) => match length {
Some(length) => length, Some(length) => length,
_ => return Err(WebDriverJSError::UnknownType), _ => return Err(WebDriverJSError::UnknownType),
}, },
Err(error) => { Err(error) => {
throw_dom_exception( throw_dom_exception(cx, global_scope, error, CanGc::note());
unsafe { SafeJSContext::from_ptr(cx) },
global_scope,
error,
CanGc::note(),
);
return Err(WebDriverJSError::JSError); return Err(WebDriverJSError::JSError);
}, },
}; };
// Step 4. For each enumerable property in value, run the following substeps: // Step 4. For each enumerable property in value, run the following substeps:
for i in 0..length { for i in 0..length {
rooted!(in(cx) let mut item = UndefinedValue()); rooted!(in(*cx) let mut item = UndefinedValue());
let get_property_result = let get_property_result =
unsafe { get_property_jsval(cx, object_handle, &i.to_string(), item.handle_mut()) }; get_property_jsval(cx, object_handle, &i.to_string(), item.handle_mut());
match get_property_result { match get_property_result {
Ok(_) => { Ok(_) => {
let conversion_result = let conversion_result =
unsafe { jsval_to_webdriver_inner(cx, global_scope, item.handle(), seen) }; unsafe { jsval_to_webdriver_inner(*cx, global_scope, item.handle(), seen) };
match conversion_result { match conversion_result {
Ok(converted_item) => result.push(converted_item), Ok(converted_item) => result.push(converted_item),
err @ Err(_) => return err, err @ Err(_) => return err,
} }
}, },
Err(error) => { Err(error) => {
throw_dom_exception( throw_dom_exception(cx, global_scope, error, CanGc::note());
unsafe { SafeJSContext::from_ptr(cx) },
global_scope,
error,
CanGc::note(),
);
return Err(WebDriverJSError::JSError); return Err(WebDriverJSError::JSError);
}, },
} }
@ -533,10 +528,10 @@ unsafe fn clone_an_object(
} else { } else {
let mut result = HashMap::new(); let mut result = HashMap::new();
let mut ids = unsafe { IdVector::new(cx) }; let mut ids = unsafe { IdVector::new(*cx) };
let succeeded = unsafe { let succeeded = unsafe {
GetPropertyKeys( GetPropertyKeys(
cx, *cx,
object_handle.into(), object_handle.into(),
jsapi::JSITER_OWNONLY, jsapi::JSITER_OWNONLY,
ids.handle_mut(), ids.handle_mut(),
@ -546,13 +541,13 @@ unsafe fn clone_an_object(
return Err(WebDriverJSError::JSError); return Err(WebDriverJSError::JSError);
} }
for id in ids.iter() { for id in ids.iter() {
rooted!(in(cx) let id = *id); rooted!(in(*cx) let id = *id);
rooted!(in(cx) let mut desc = PropertyDescriptor::default()); rooted!(in(*cx) let mut desc = PropertyDescriptor::default());
let mut is_none = false; let mut is_none = false;
let succeeded = unsafe { let succeeded = unsafe {
JS_GetOwnPropertyDescriptorById( JS_GetOwnPropertyDescriptorById(
cx, *cx,
object_handle.into(), object_handle.into(),
id.handle().into(), id.handle().into(),
desc.handle_mut().into(), desc.handle_mut().into(),
@ -563,10 +558,10 @@ unsafe fn clone_an_object(
return Err(WebDriverJSError::JSError); return Err(WebDriverJSError::JSError);
} }
rooted!(in(cx) let mut property = UndefinedValue()); rooted!(in(*cx) let mut property = UndefinedValue());
let succeeded = unsafe { let succeeded = unsafe {
JS_GetPropertyById( JS_GetPropertyById(
cx, *cx,
object_handle.into(), object_handle.into(),
id.handle().into(), id.handle().into(),
property.handle_mut().into(), property.handle_mut().into(),
@ -577,13 +572,13 @@ unsafe fn clone_an_object(
} }
if !property.is_undefined() { if !property.is_undefined() {
let name = unsafe { jsid_to_string(cx, id.handle()) }; let name = unsafe { jsid_to_string(*cx, id.handle()) };
let Some(name) = name else { let Some(name) = name else {
return Err(WebDriverJSError::JSError); return Err(WebDriverJSError::JSError);
}; };
if let Ok(value) = if let Ok(value) =
unsafe { jsval_to_webdriver_inner(cx, global_scope, property.handle(), seen) } unsafe { jsval_to_webdriver_inner(*cx, global_scope, property.handle(), seen) }
{ {
result.insert(name.into(), value); result.insert(name.into(), value);
} else { } else {
@ -1705,14 +1700,12 @@ pub(crate) fn handle_get_property(
let cx = document.window().get_cx(); let cx = document.window().get_cx();
rooted!(in(*cx) let mut property = UndefinedValue()); rooted!(in(*cx) let mut property = UndefinedValue());
match unsafe { match get_property_jsval(
get_property_jsval( cx,
*cx, element.reflector().get_jsobject(),
element.reflector().get_jsobject(), &name,
&name, property.handle_mut(),
property.handle_mut(), ) {
)
} {
Ok(_) => { Ok(_) => {
match jsval_to_webdriver( match jsval_to_webdriver(
cx, cx,