Handle /v flag in slow path of RegExp.prototype.matchAll
Categories
(Core :: JavaScript Engine, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox148 | --- | fixed |
People
(Reporter: iain, Assigned: iain)
Details
Attachments
(1 file)
While reviewing this patch I noticed that we were missing a line here. This code corresponds to this spec text; note that step 11 in the spec checks for both u and v in the flags, but our existing implementation only checks for u. Existing tests don't catch this, because they all end up in the fast path where we reuse the existing regexp instead of creating a new one. However, we can trigger the bug by subclassing RegExp:
class MyRegExp extends RegExp {}
let r = new MyRegExp("(?:)", "gv");
for (var x of '𠮷'.matchAll(r)) {}
We invoke matchAll with a zero-length regexp. This ends up in the slow path because of the subclass. We don't copy the v flag. When we iterate over the result, this code sets fullUnicode to false: we check for the v flag, but it was never set. Therefore, when we reach this code, we increment lastIndex by 1 instead of advancing the string index. This leaves us in the middle of a surrogate pair. When we request another match, the code generated by OptionallyStepBackToLeadSurrogate moves us back one byte, and we end up in an infinite loop.
| Assignee | ||
Comment 1•1 day ago
|
||
The testcase spins infinitely without this fix.
Drive-by: removed the REGEXP_LEGACY_FEATURES_ENABLED_FLAG code here, which I accidentally let through while reviewing the legacy features patch. It's not necessary here: if we're taking the slow path, then we only look at the global/unicode flags (the spec represents global and unicode as separate boolean fields). The fast path reuses the flags from the existing regexp in certain cases to lazily copy the input regexp (see here), but that's irrelevant to the slow path.
Updated•1 day ago
|
Comment 3•23 minutes ago
|
||
| bugherder | ||
Description
•