script: Fix assertion failure when stringifying cross-origin location object#43844
script: Fix assertion failure when stringifying cross-origin location object#43844thebabalola wants to merge 3 commits intoservo:mainfrom
Conversation
jdm
left a comment
There was a problem hiding this comment.
This looks good! Can you write a test that uses https://github.com/web-platform-tests/wpt/blob/0f8fc11ddab5f0462806006b46e3b230f6e8a8d0/cors/preflight-failure.htm#L19 to create the cross-origin iframe? We can use a crash test for that, and it can go in tests/wpt/tests/html/browsers/history/the-location-interface
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#58948) with upstreamable changes. |
appreciate the review.... would be waiting for CI to finish : ) |
|
Hmm, we will need to add the test path to servo/tests/wpt/tests/lint.ignore Line 132 in 78ffbe8 ./mach update-manifest to fix the rest.
|
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58948). |
|
You will need to rebase to the latest changes in main and fix the conflict. |
… object Refactor console_argument_from_handle_value to use an inner function that returns Result<ConsoleArgument, ()>. When console_object_from_handle_value returns None for an object (e.g. a cross-origin location), the inner function returns Err instead of falling through to stringify_handle_value which would trigger the same assertion. The outer function reports any pending JS exception and returns a fallback value. Fixes servo#43530 Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Add a WPT crash test that loads a cross-origin iframe and calls console.log on its location object, verifying that the browser does not crash. Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
Signed-off-by: thebabalola <t.babalolajoseph@gmail.com>
a3700b2 to
425c8ce
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58948). |
|
with CI checks passing.... this should be ready to be merged... |
This fixes the assertion failure (
!JS_IsExceptionPending(*cx)) that happens whenconsole.log()is called on a cross-origin location object (e.g.console.log(frame.contentWindow.location)).The problem was that
maybe_stringify_dom_objectcallsToStringon cross-origin objects, which throws a JS exception via theDissimilarOriginLocationstringifier. That exception was never cleared, so subsequent JS API calls would hit the assertion.The fix refactors
console_argument_from_handle_valueusing an inner/outer function pattern based on @jdm's suggestion:Result<ConsoleArgument, ()>and returnsErr(())whenconsole_object_from_handle_valuereturnsNonefor an object, instead of falling through tostringify_handle_valuewhich could trigger the same crashErr, reports any pending JS exception viareport_pending_exception, and returns a fallbackConsoleArgumentFixes #43530
./mach build -ddoes not report any errors./mach fmtproduces no changes