dom: Always replace unpaired surrogates when handling page text #35381
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrobinson
force-pushed
the
replace-surrogates
branch
from
fa61306 to
7389f9a
Compare
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#50594) with upstreamable changes. |
mrobinson
force-pushed
the
replace-surrogates
branch
from
7389f9a to
4a761ed
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#50594). |
Background: > JavaScript strings are potentially ill-formed UTF-16 (arbitrary > Vec<u16>) and can contain unpaired surrogates. Rust’s String type is > well-formed UTF-8 and can not contain any surrogate. Surrogates are > never emitted when decoding bytes from the network, but they can sneak > in through document.write, the Element.innerHtml setter, or other DOM > APIs. In 2015, Servo launched an experiment to see if unpaired surrogates cropped up in page content. That experiment caused Servo to panic if unpaired surrogates were encountered with a request to report the page to bug servo#6564. During that time several pages were reported with unpaired surrogates, causing Servo to panic. In addition, when running the WPT tests Servo will never panic due to the `-Z replace-surrogates` option being passed by the test driver. Motivation: After this 10 year experiment, it's clear that unpaired surrogates are a real concern in page content. Several reports were filed of Servo panicking after encountering them in real world pages. A complete fix for this issue would be to somehow maintain unpaired surrogates in the DOM, but that is a much larger task than simply emitting U+FFD instead of an unpaired surrogate. Since it is clear that this kind of content exists, it is better for Servo to try its best to handle the content rather than crash as production browsers should not crash due to user content when possible. In this change, I modify Servo to always replace unpaired surrogates. It would have been ideal to only crash when debug assertions are enabled, but debug assertions are enabled by default in release mode -- so this wouldn't be effective for WPT tests. Signed-off-by: Martin Robinson <[email protected]>
mrobinson
force-pushed
the
replace-surrogates
branch
from
4a761ed to
d361314
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#50594). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background:
In 2015, Servo launched an experiment to see if unpaired surrogates
cropped up in page content. That experiment caused Servo to panic if
unpaired surrogates were encountered with a request to report the page
to bug #6564. During that time several pages were reported with unpaired
surrogates, causing Servo to panic. In addition, when running the WPT
tests Servo will never panic due to the
-Z replace-surrogatesoptionbeing passed by the test driver.
Motivation:
After this 10 year experiment, it's clear that unpaired surrogates are a
real concern in page content. Several reports were filed of Servo
panicking after encountering them in real world pages. A complete fix for
this issue would be to somehow maintain unpaired surrogates in the DOM,
but that is a much larger task than simply emitting U+FFD instead of an
unpaired surrogate.
Since it is clear that this kind of content exists, it is better for
Servo to try its best to handle the content rather than crash as
production browsers should not crash due to user content when possible.
In this change, I modify Servo to always replace unpaired surrogates.
It would have been ideal to only crash when debug assertions are
enabled, but debug assertions are enabled by default in release mode --
so this wouldn't be effective for WPT tests.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors