Closed Bug 834989 Opened 12 years ago Closed 11 years ago

Use of RegExp in self-hosted code can be observed through RegExp statics


(Core :: JavaScript Engine, defect)

Not set





(Reporter: mozillabugs, Assigned: mozillabugs)


(Blocks 1 open bug)



(3 files, 1 obsolete file)

The use of RegExp methods or of regular expressions in String methods can be observed by client code through "RegExp statics", properties of the RegExp constructor. These properties are documented at

The attached test case calls self-hosted functions that use the RegExp methods test and exec, and the String method match, each separately with regular expressions that include or don't include capturing parentheses. The test output shows that all methods change at least some of the properties of the RegExp constructor; more properties if capturing parentheses are used.

The use of regular expressions within self-hosted code should not be observable by client code at all.
Blocks: es-intl
No longer blocks: 769872
Blocks: 854320
This patch adds intrinsics regexp_exec_no_statics and regexp_test_no_statics that self-hosted code can use instead of RegExp.prototype.exec and RegExp.prototype.test and that don't modify the RegExp statics.

The patch does not add such intrinsics for the String methods that accept RegExp arguments:
- String.prototype.match can be trivially replaced with regexp_exec_no_statics.
- String.prototype.replace has so many different usage patterns that inserting a str_replace_no_statics would require serious testing, and the only call in existing self-hosted code was easy to rewrite based on regexp_exec_no_statics.
- String.prototype.split has no caller in current self-hosted code that passes in a RegExp object.
- has no caller in current self-hosted code at all.
Assignee: general → mozillabugs
Attachment #769977 - Flags: review?(jwalden+bmo)
This updated test case uses the new intrinsics instead of RegExp.prototype.exec and RegExp.prototype.test. It passes for the four individual tests testing the new intrinsics; still fails for the two individual tests testing String.prototype.match.

Note that the real test for the new intrinsics will be the ECMA-402 conformance tests testing the behavior relying on the intrinsics - see bug 854320.
Comment on attachment 769977 [details] [diff] [review]
Use alternatives to RegExp.p.exec and RegExp.p.test that don't modify RegExp statics

Review of attachment 769977 [details] [diff] [review]:

::: js/src/builtin/RegExp.h
@@ +23,5 @@
>  namespace js {
>  RegExpRunStatus
>  ExecuteRegExp(JSContext *cx, HandleObject regexp, HandleString string,
> +              MatchConduit &matches, bool setStatics);

We try to avoid having unadorned bools like this, when it's not obvious at the call site what the meanings are.  (setVisible(true/false) is obvious; ExecuteRegExp(..., true/false) is not.)  Please add something like (better name suggestions welcome):

  enum AffectsRegExpStatics { UpdateRegExpStatics, DontUpdateRegExpStatics };

and use that as the argument type here.
Attachment #769977 - Flags: review?(jwalden+bmo) → review+
Updated per comment 3. Carrying r+jwalden.
Attachment #769977 - Attachment is obsolete: true
Attachment #773353 - Flags: review+
Attachment #773353 - Flags: checkin?(jwalden+bmo)
Comment on attachment 773353 [details] [diff] [review]
Use alternatives to RegExp.p.exec and RegExp.p.test that don't modify RegExp statics
Attachment #773353 - Flags: checkin?(jwalden+bmo) → checkin+
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.