Closed Bug 1287524 Opened 4 years ago Closed 4 years ago

Missing property check before applying optimized RegExp.prototype[Symbol.replace] code path

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox48 --- unaffected
firefox49 --- affected
firefox-esr45 --- unaffected
firefox50 --- fixed

People

(Reporter: anba, Assigned: arai)

References

()

Details

Attachments

(1 file)

A custom accessor for the "unicode" property can invalidate the fast-path assumptions in RegExp.prototype[Symbol.replace].


Test case:
---
Object.defineProperty(RegExp.prototype, "unicode", {
  get() {
    RegExp.prototype.exec = () => null;
  }
});

rx = RegExp("a", "g");
s = "abba";
r = rx[Symbol.replace](s, "c");

print(r);
---

Expected: Prints "abba"
Actual: Prints "cbbc"
Added the check for RegExp.prototype.unicode to RegExpPrototypeOptimizable, at the same time as global and sticky.
So that we don't use optimized path when unicode accessor is modified.

That won't affect performance, and in most case unicode accessor won't be modified :)
Assignee: nobody → arai.unmht
Attachment #8772227 - Flags: review?(hv1989)
bug 887016 was landed to firefox48, but backed out from firefox48 by bug 1265307, so this bug affects from firefox49.
Comment on attachment 8772227 [details] [diff] [review]
Check RegExp.prototype.unicode in RegExpPrototypeOptimizable.

Review of attachment 8772227 [details] [diff] [review]:
-----------------------------------------------------------------

Don't we need to do the same for ignoreCase and multiline. And if not why not? Else can you open a follow-up bug?
Good find!
Attachment #8772227 - Flags: review?(hv1989) → review+
Thank you for reviewing :)

(In reply to Hannes Verschore [:h4writer] from comment #3)
> Don't we need to do the same for ignoreCase and multiline. And if not why
> not? Else can you open a follow-up bug?

I was testing that and directly accessing slot in bug 1263340 for performance reason, but I couldn't get any performance improvement so far.
anyway, adding them to RegExpPrototypeOptimizable won't harm so much, and it will prevent similar future issue.
will file it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f099a449488b2b277dc5488d9f8a37272c6cd792
Bug 1287524 - Check RegExp.prototype.unicode in RegExpPrototypeOptimizable. r=h4writer
https://hg.mozilla.org/mozilla-central/rev/f099a449488b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.