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 #577

Merged
merged 1 commit into from Dec 23, 2015

Conversation

Projects
None yet
2 participants
@zetaben
Contributor

zetaben commented Dec 23, 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 chai/chai.js

When choking it produces a:
eval failed: SyntaxError: invalid regexp quantifier (unknown char) (line 4401)
when the parsing is fine it will produce :
result is: undefined

Fix non ES5 compliant regexp
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).
@keithamus

This comment has been minimized.

Show comment
Hide comment
@keithamus

keithamus Dec 23, 2015

Member

Good catch @zetaben. Normally I'd ask for some tests but I'm not exactly how you could test this one without adding something like Duktape to our test suite!

Member

keithamus commented Dec 23, 2015

Good catch @zetaben. Normally I'd ask for some tests but I'm not exactly how you could test this one without adding something like Duktape to our test suite!

keithamus added a commit that referenced this pull request Dec 23, 2015

Merge pull request #577 from zetaben/patch-1
Fix non ES5 compliant regexp

@keithamus keithamus merged commit 067074d into chaijs:master Dec 23, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zetaben

This comment has been minimized.

Show comment
Hide comment
@zetaben

zetaben Dec 24, 2015

Contributor

Thanks @keithamus !

As for tests, I was also not sure of what could be done, maybe a script that depend on execjs (https://github.com/rails/execjs) or duktape.rb (https://github.com/judofyr/duktape.rb) could be launched by your travis ? It would enable to check correct parsing under a few JS runtime without directly depending on them in the project.

Contributor

zetaben commented Dec 24, 2015

Thanks @keithamus !

As for tests, I was also not sure of what could be done, maybe a script that depend on execjs (https://github.com/rails/execjs) or duktape.rb (https://github.com/judofyr/duktape.rb) could be launched by your travis ? It would enable to check correct parsing under a few JS runtime without directly depending on them in the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment