Skip to content

Commit

Permalink
fix(ballot-interpreter): correct JS argument marshaling
Browse files Browse the repository at this point in the history
If `options` was given as `{ scoreWriteIns: undefined }`, the interpreter would fail to coerce the value as a `bool` but would ignore the error until some point in the future when interacting with the JavaScript context. This is because the `get::<JsBoolean>` call would put the context into a throwing state, but would not then bubble that `Throw` up to the caller. As a result, subsequent calls to the context would also yield `Throw` for operations that should have succeeded. In this case the error would be "failed to downcast any to boolean".

To fix this we need to not trigger throws when we don't intend to bubble them up. In particular, instead of `get::<JsBoolean>` we now `get_value()?.downcast::<JsBoolean>` so that we can bubble possible errors from the getter but then handle a mismatched type in Rust appopriately.
  • Loading branch information
eventualbuddha committed Oct 24, 2024
1 parent 5800d86 commit 72bd015
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
7 changes: 3 additions & 4 deletions libs/ballot-interpreter/src/hmpb-rust/js/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ pub fn get_image_data_or_path_from_arg(
let path = path.value(&mut *cx);
Ok(ImageSource::Path(PathBuf::from(path)))
} else if let Ok(image_data) = argument.downcast::<JsObject, _>(cx) {
ImageData::from_js_object(cx, image_data).map_or_else(
|| cx.throw_type_error("unable to read argument as ImageData"),
|image| Ok(ImageSource::ImageData(image)),
)
Ok(ImageSource::ImageData(ImageData::from_js_object(
cx, image_data,
)?))
} else {
cx.throw_type_error("expected image data or path")
}
Expand Down
17 changes: 8 additions & 9 deletions libs/ballot-interpreter/src/hmpb-rust/js/image_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,18 @@ impl ImageData {
}

/// Converts a JavaScript `ImageData` object to a Rust `ImageData`.
pub fn from_js_object(cx: &mut FunctionContext, js_object: Handle<JsObject>) -> Option<Self> {
let width = js_object.get::<JsNumber, _, _>(cx, "width").ok()?.value(cx) as u32;
let height = js_object
.get::<JsNumber, _, _>(cx, "height")
.ok()?
.value(cx) as u32;
pub fn from_js_object(
cx: &mut FunctionContext,
js_object: Handle<JsObject>,
) -> NeonResult<Self> {
let width = js_object.get::<JsNumber, _, _>(cx, "width")?.value(cx) as u32;
let height = js_object.get::<JsNumber, _, _>(cx, "height")?.value(cx) as u32;
let data = js_object
.get::<JsBuffer, _, _>(cx, "data")
.ok()?
.get::<JsBuffer, _, _>(cx, "data")?
.borrow()
.as_slice(cx)
.to_vec();
Some(Self::new(width, height, data))
Ok(Self::new(width, height, data))
}

pub fn to_js_object<'a>(&self, cx: &mut FunctionContext<'a>) -> JsResult<'a, JsObject> {
Expand Down
11 changes: 10 additions & 1 deletion libs/ballot-interpreter/src/hmpb-rust/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,22 @@ pub fn interpret(mut cx: FunctionContext) -> JsResult<JsObject> {
let side_b_image_or_path = get_image_data_or_path_from_arg(&mut cx, 2)?;
let debug_side_a_base = get_path_from_arg_opt(&mut cx, 3);
let debug_side_b_base = get_path_from_arg_opt(&mut cx, 4);

// Equivalent to:
// let options = typeof arguments[5] === 'object' ? arguments[5] : {};
let options = match cx.argument_opt(5) {
Some(arg) => arg.downcast::<JsObject, _>(&mut cx).or_throw(&mut cx)?,
None => cx.empty_object(),
};

// Equivalent to:
// let score_write_ins =
// typeof options.scoreWriteIns === 'boolean'
// ? options.scoreWriteIns
// : false;
let score_write_ins = options
.get::<JsBoolean, _, _>(&mut cx, "scoreWriteIns")
.get_value(&mut cx, "scoreWriteIns")?
.downcast::<JsBoolean, _>(&mut cx)
.ok()
.map_or(false, |b| b.value(&mut cx));

Expand Down

0 comments on commit 72bd015

Please sign in to comment.