Fix error handling in nsContentUtils::IsPatternMatching
Categories
(Core :: JavaScript Engine, task, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox78 | --- | fixed |
People
(Reporter: iain, Assigned: iain)
References
Details
Attachments
(3 files, 1 obsolete file)
nsContentUtils::IsPatternMatching calls into SpiderMonkey several times to validate and execute a regexp. It assumes that JS::NewUCRegExpObject can only fail if the pattern is invalid. However, in the case of OOM or stack overflow, this is false.
It's awkward to handle stack overflow outside of SM. The best fix is to add a new API for pattern matching that can provide this information directly.
|
||
Updated•4 years ago
|
|
Assignee | |
Comment 1•4 years ago
|
||
To make sure that <input> elements with pattern attributes update their validation state (:invalid) properly, nsContentUtils::IsPatternMatching needs to be able to distinguish between parsing errors caused by an invalid pattern, vs parsing errors caused by OOM/overrecursion.
This patch also fixes up the places inside the new regexp engine where we can throw over-recursed to make sure that we set the right flag on the context.
|
|
Assignee | |
Comment 2•4 years ago
|
||
nsContentUtils::IsPatternMatching calls into SpiderMonkey several times to validate and execute a regexp. It assumes that JS::NewUCRegExpObject can only fail if the pattern is invalid. However, in the case of OOM or stack overflow, this is false.
In the previous patch, we added a new API for pattern matching. This patch uses the new function to clean up the error handling in IsPatternMatching.
Depends on D74499
|
|
Assignee | |
Comment 3•4 years ago
|
||
While trying to fix the error-handling, I tested one solution that should have caused a failure in a web platform test, but didn't, because the test didn't exist yet. This patch adds that test.
Depends on D74501
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
Pushed by iireland@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cb0d2a799a63 Add JS::CheckRegExpSyntax r=evilpie https://hg.mozilla.org/integration/autoland/rev/87f9a3035dd8 Use JS::CheckRegExpSyntax in nsContentUtils::IsPatternMatching r=emilio https://hg.mozilla.org/integration/autoland/rev/4cbaef432cb8 Add web platform test r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23510 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 7•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/cb0d2a799a63
https://hg.mozilla.org/mozilla-central/rev/87f9a3035dd8
https://hg.mozilla.org/mozilla-central/rev/4cbaef432cb8
|
||
Comment 8•4 years ago
|
||
Backed out together with bug 1634135 for crashes when e.g. a url gets pasted into Slack (Bug 1637243):
https://hg.mozilla.org/mozilla-central/rev/22a95484cc62214e768dd06711a542b589cb2116
Upstream PR was closed without merging
|
|
||
Comment 10•4 years ago
|
||
Backout has been merged to autoland.
|
|
Assignee | |
Comment 11•4 years ago
|
||
This code is unrelated to the backout. Relanding it.
|
||
Comment 12•4 years ago
|
||
Pushed by iireland@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4b98c08423c9 Add JS::CheckRegExpSyntax r=evilpie https://hg.mozilla.org/integration/autoland/rev/7ac33283a786 Use JS::CheckRegExpSyntax in nsContentUtils::IsPatternMatching r=emilio https://hg.mozilla.org/integration/autoland/rev/5be0a4315674 Add web platform test r=emilio
|
||
Comment 13•4 years ago
|
||
Backed out for SM bustages.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=301940639&repo=autoland&lineNumber=44294
Backout: https://hg.mozilla.org/integration/autoland/rev/dec39daedd043c2d82d9e8ebd593a62b38947571
|
|
Assignee | |
Comment 14•4 years ago
|
||
Bluh. For some reason the update to the binast tests doesn't work properly without the rest of the changes in bug 1634135.
This is weird, but I don't think it's worth delving into binast-specific issues, given that these tests pass with the full stack of changes. These three patches will just have to wait until bug 1634135 is ready again.
Upstream PR was closed without merging
|
|
||
Comment 16•4 years ago
|
||
Pushed by iireland@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5dbad591adfa Add JS::CheckRegExpSyntax r=evilpie https://hg.mozilla.org/integration/autoland/rev/da8a7cb04f92 Use JS::CheckRegExpSyntax in nsContentUtils::IsPatternMatching r=emilio https://hg.mozilla.org/integration/autoland/rev/a8cd17fca204 Add web platform test r=emilio
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
| Comment hidden (obsolete) |
| Comment hidden (obsolete) |
|
||
Comment 20•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5dbad591adfa
https://hg.mozilla.org/mozilla-central/rev/da8a7cb04f92
https://hg.mozilla.org/mozilla-central/rev/a8cd17fca204
Upstream PR merged by moz-wptsync-bot
Description
•