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

Fix non ES5 compliant regexp #2021

Merged
merged 1 commit into from
Dec 24, 2015
Merged

Fix non ES5 compliant regexp #2021

merged 1 commit into from
Dec 24, 2015

Conversation

zetaben
Copy link
Contributor

@zetaben zetaben commented Dec 22, 2015

ES5 appears to require that { be escaped when not used as part of a quantifier. While this works fine in browsers it appears to choke less lenient runtimes (e.g. Duktape).

A similar case is discussed in Duktape (svaarala/duktape#69 and svaarala/duktape#74).

Please find attached a simple Duktape-powered C file that tries to parse the given argument file. test.c.zip

Compile and run using:

$ gcc -std=c99 -o test test.c duktape-1.3.1/src/duktape.c -I duktape-1.3.1/src/  -lm && ./test mocha/mocha.js

When choking it produces a:
eval failed: SyntaxError: invalid regexp quantifier (unknown char) (line 5755)
when the parsing is fine it will produce (because I did not setup a complete environment):
eval failed: ReferenceError: identifier 'window' undefined

Review on Reviewable

ES5 appears to require that { be escaped when not used as part of a quantifier. While this works fine in browsers it appears to choke less lenient runtimes (e.g. Duktape)
@jbnicolai
Copy link

Looks good to me, thanks for the effort!


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

jbnicolai pushed a commit that referenced this pull request Dec 24, 2015
Fix non ES5 compliant regexp
@jbnicolai jbnicolai merged commit 5d1fdc2 into mochajs:master Dec 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants