Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbidden extension in RegExp pattern grammar #4368

Closed
mathiasbynens opened this issue Dec 3, 2017 · 0 comments
Closed

Forbidden extension in RegExp pattern grammar #4368

mathiasbynens opened this issue Dec 3, 2017 · 0 comments
Assignees
Milestone

Comments

@mathiasbynens
Copy link
Contributor

@mathiasbynens mathiasbynens commented Dec 3, 2017

See https://tc39.github.io/ecma262/#sec-forbidden-extensions (and https://bugs.ecmascript.org/show_bug.cgi?id=3157 for more background):

The RegExp pattern grammars in 21.2.1 and B.1.4 must not be extended to recognize any of the source characters A-Z or a-z as IdentityEscape[+U] when the [U] grammar parameter is present.

Chakra incorrectly extends the grammar in this way, which is especially problematic for \p and \P:

$ eshost -e '/\p{Script_Extensions=Greek}/u.test("π")'
#### Chakra
false

#### JavaScriptCore
true

#### V8
true

This presents a compatibility risk. One might reasonably assume that if the above RegExp compiles without throwing an exception, the engine supports Unicode property escapes, and then rely on the results that it produces. With the current behavior, that would result in incorrect results.

Any of the following should throw an exception:

/\p/u;
/\P/u;
/\a/u;
/\A/u;
/\e/u;
/\E/u;
/\y/u;
/\Y/u;
/\z/u;
/\Z/u;

And this should throw an exception until Unicode property escapes are implemented:

/\p{Script_Extensions=Greek}/;
/\P{Script_Extensions=Greek}/;
@digitalinfinity digitalinfinity added this to the 1.8 milestone Dec 4, 2017
@bterlson bterlson added the Severity: 1 label Jan 4, 2018
chakrabot pushed a commit that referenced this issue Feb 2, 2018
…disallow invalid escapes.

Merge pull request #4637 from dilijev:re-escape-forbidden

Fixes #4368
chakrabot pushed a commit that referenced this issue Feb 2, 2018
…xplicitly disallow invalid escapes.

Merge pull request #4637 from dilijev:re-escape-forbidden

Fixes #4368
@chakrabot chakrabot closed this in 8933017 Feb 2, 2018
chakrabot pushed a commit that referenced this issue Feb 2, 2018
…ode RegExp, explicitly disallow invalid escapes.

Merge pull request #4637 from dilijev:re-escape-forbidden

Fixes #4368
dilijev added a commit to dilijev/ChakraCore that referenced this issue Feb 6, 2018
See chakra-core#4637: Fix chakra-core#4368: In unicode-mode RegExp, explicitly disallow invalid escapes.

Fixes OS#15775563
dilijev added a commit to dilijev/ChakraCore that referenced this issue Feb 6, 2018
See chakra-core#4637: Fix chakra-core#4368: In unicode-mode RegExp, explicitly disallow invalid escapes.

Fixes OS#15775563
chakrabot pushed a commit that referenced this issue Feb 6, 2018
…y allow RegExp identity escapes.

Merge pull request #4644 from dilijev:fix_unicode_forbidden_escapes

See #4637: Fix #4368: In unicode-mode RegExp, explicitly disallow invalid escapes.

Fixes OS#15775563
chakrabot pushed a commit that referenced this issue Feb 6, 2018
… Explicitly allow RegExp identity escapes.

Merge pull request #4644 from dilijev:fix_unicode_forbidden_escapes

See #4637: Fix #4368: In unicode-mode RegExp, explicitly disallow invalid escapes.

Fixes OS#15775563
chakrabot pushed a commit that referenced this issue Feb 6, 2018
…vious change. Explicitly allow RegExp identity escapes.

Merge pull request #4644 from dilijev:fix_unicode_forbidden_escapes

See #4637: Fix #4368: In unicode-mode RegExp, explicitly disallow invalid escapes.

Fixes OS#15775563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants