-
Notifications
You must be signed in to change notification settings - Fork 682
Add RegExp recursion depth limit #2543
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
Add RegExp recursion depth limit #2543
Conversation
IMHO this limit should be applied to |
79d9642
to
1c5ca4a
Compare
e6f4c2a
to
0e8ff57
Compare
@zherczeg @LaszloLango I've applied the first version of the patch and updated the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise the patch is ok.
0e8ff57
to
25de724
Compare
25de724
to
da1149a
Compare
@LaszloLango @zherczeg @akosthekiss I've updated the PR. |
d271448
to
5c82a89
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only final touches.
559750e
to
f8235f2
Compare
4b1a08c
to
59659c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
9583c51
to
fb04ecb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some quick style change requests.
fb04ecb
to
355963f
Compare
@akosthekiss @rerobika Thank you! I've fixed the typos, style issues and updated the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more minor thing.
b62104e
to
f3a3d35
Compare
f3a3d35
to
89f26c2
Compare
89f26c2
to
098afb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (informal)
@akosthekiss Do you have any more comments, remarks about this PR? |
The regexp engine does not have any recursion depth check, thus it can cause problems with various regexps. Added a new build option `--regexp-recursion-limit N` whose default value is 0, which is for unlimited recursion depth. Also added a build-option-test. Fixes jerryscript-project#2448 Fixes jerryscript-project#2190 JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu
098afb5
to
ebf605f
Compare
@akosthekiss I updated the PR with the requested changes, also updated the commit message, as you mentioned, this PR also fixes the #2190. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The regexp engine does not have any recursion depth check, thus it can cause problems with various regexps. Added a new build option
--regexp-recursion-limit N
whosedefault value is 0, which is for unlimited recursion depth. Also added a build-option-test.
Fixes #2448
Fixes #2190
JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu