-
Notifications
You must be signed in to change notification settings - Fork 686
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. |
zherczeg
left a comment
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
zherczeg
left a comment
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
zherczeg
left a comment
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
akosthekiss
left a comment
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. |
rerobika
left a comment
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
rerobika
left a comment
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. |
akosthekiss
left a comment
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 Nwhosedefault 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