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

Handle regexp parse errors gracefully #3266

Merged
merged 1 commit into from Jun 4, 2021
Merged

Handle regexp parse errors gracefully #3266

merged 1 commit into from Jun 4, 2021

Conversation

emjin
Copy link
Contributor

@emjin emjin commented Jun 4, 2021

Create an exception for regexp parse errors with good error messages and handle it python side

Test plan:

In semgrep/semgrep/tests/e2e, run semgrep --config rules/regex-invalid.yaml --optimizations

You should see

running 1 rules...
Invalid regexp in rule: '(abc': missing ) at position 4

PR checklist:

  • changelog is up to date

Create an exception for regexp parse errors with good error messages and handle it python side

Test plan:

In semgrep/semgrep/tests/e2e, run semgrep --config rules/regex-invalid.yaml --optimizations

You should see

running 1 rules...
Invalid regexp in rule: '(abc': missing ) at position 4
@emjin emjin requested a review from aryx June 4, 2021 02:19
@emjin
Copy link
Contributor Author

emjin commented Jun 4, 2021

Failures seem to be due to semgrep/semgrep-rules@1e44447; merging #3268 will fix

Copy link
Collaborator

@aryx aryx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@@ -28,6 +28,9 @@ exception InvalidLanguageException of string * string
exception InvalidPatternException of string * string * string * string

(*e: exception [[Parse_rules.InvalidPatternException]] *)

exception InvalidRegexpException of string * string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I don't think I introduced those exceptions, but there's no need to have them suffixed by Exception.
We know it's an exception :)
(It's fine in this PR because it's good to be consistent, but we should fix those later).

@aryx aryx merged commit e8c13b0 into develop Jun 4, 2021
@aryx aryx deleted the EJ-pcre-error branch June 4, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants