From 9aa09d73b5d82406b0d922a30f30b65ffcf9eb96 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 16 Apr 2025 02:52:48 -0400 Subject: [PATCH] Fix crash when setting custom property on Location (#36494) The JS engine uses types like `Handle>` in various places and our automated bindings are not able to handle the Maybe type. We have hand-written bindings that use outparams to indicate a PropertyDescriptor value is actually the Nothing type, but that data was getting lost when we passed the property descriptor to SetPropertyIgnoringNamedGetter, which assumed that the property descriptor was always valid. Depends on https://github.com/servo/mozjs/pull/579. Testing: Manual testing on testcase from https://github.com/servo/servo/issues/34709, and new crashtest added. Fixes: #34709 Signed-off-by: Josh Matthews --- Cargo.lock | 6 +++--- components/script_bindings/proxyhandler.rs | 7 ++++++- ...ation-prototype-setting-same-origin-domain.sub.html.ini | 3 --- .../location-prototype-setting-same-origin.html.ini | 3 --- .../opening-the-input-stream/no-new-global.window.js.ini | 3 --- tests/wpt/mozilla/meta/MANIFEST.json | 7 +++++++ tests/wpt/mozilla/tests/mozilla/location-set-crash.html | 5 +++++ 7 files changed, 21 insertions(+), 13 deletions(-) delete mode 100644 tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin-domain.sub.html.ini delete mode 100644 tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin.html.ini create mode 100644 tests/wpt/mozilla/tests/mozilla/location-set-crash.html diff --git a/Cargo.lock b/Cargo.lock index e9af4b24fdd..fefafec532a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4673,7 +4673,7 @@ dependencies = [ [[package]] name = "mozjs" version = "0.14.1" -source = "git+https://github.com/servo/mozjs#e4d4f9ac06162fe2647078dc4be8c270b7219807" +source = "git+https://github.com/servo/mozjs#d1525dfaee22cc1ea9ee16c552cdeedaa9f20741" dependencies = [ "bindgen 0.71.1", "cc", @@ -4684,8 +4684,8 @@ dependencies = [ [[package]] name = "mozjs_sys" -version = "0.128.9-0" -source = "git+https://github.com/servo/mozjs#e4d4f9ac06162fe2647078dc4be8c270b7219807" +version = "0.128.9-1" +source = "git+https://github.com/servo/mozjs#d1525dfaee22cc1ea9ee16c552cdeedaa9f20741" dependencies = [ "bindgen 0.71.1", "cc", diff --git a/components/script_bindings/proxyhandler.rs b/components/script_bindings/proxyhandler.rs index 314d1369048..29da3e91a92 100644 --- a/components/script_bindings/proxyhandler.rs +++ b/components/script_bindings/proxyhandler.rs @@ -565,13 +565,18 @@ pub(crate) unsafe extern "C" fn maybe_cross_origin_set_rawcx( return false; } + let own_desc_handle = own_desc.handle().into(); js::jsapi::SetPropertyIgnoringNamedGetter( *cx, proxy, id, v, receiver, - own_desc.handle().into(), + if is_none { + ptr::null() + } else { + &own_desc_handle + }, result, ) } diff --git a/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin-domain.sub.html.ini b/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin-domain.sub.html.ini deleted file mode 100644 index e94f347f6ee..00000000000 --- a/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin-domain.sub.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[location-prototype-setting-same-origin-domain.sub.html] - [Same-origin-domain: setting the prototype to an empty object via __proto__ should throw a TypeError] - expected: FAIL diff --git a/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin.html.ini b/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin.html.ini deleted file mode 100644 index 7bf488f91f7..00000000000 --- a/tests/wpt/meta/html/browsers/history/the-location-interface/location-prototype-setting-same-origin.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[location-prototype-setting-same-origin.html] - [Same-origin: setting the prototype to an empty object via __proto__ should throw a TypeError] - expected: FAIL diff --git a/tests/wpt/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini b/tests/wpt/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini index e71fd79e31e..38cbf536604 100644 --- a/tests/wpt/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini +++ b/tests/wpt/meta/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/no-new-global.window.js.ini @@ -1,6 +1,3 @@ [no-new-global.window.html] [BarProp maintains its prototype and properties through open()] expected: FAIL - - [Location maintains its prototype and properties through open()] - expected: FAIL diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index ce4e2b8200c..874da107470 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -23,6 +23,13 @@ {} ] ], + "location-set-crash.html": [ + "0b1695df79b0437fb644bfcb3ef09bc0eb906f1e", + [ + null, + {} + ] + ], "test-wait-crash.html": [ "2419da6af0c278a17b9ff974d4418f9e386ef3e0", [ diff --git a/tests/wpt/mozilla/tests/mozilla/location-set-crash.html b/tests/wpt/mozilla/tests/mozilla/location-set-crash.html new file mode 100644 index 00000000000..0b1695df79b --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/location-set-crash.html @@ -0,0 +1,5 @@ + + +