Skip to content

Handle PCRE errors gracefully#75

Merged
danielbachhuber merged 2 commits intomasterfrom
65-check-for-pcre-error-on-return
Apr 20, 2018
Merged

Handle PCRE errors gracefully#75
danielbachhuber merged 2 commits intomasterfrom
65-check-for-pcre-error-on-return

Conversation

@schlessera
Copy link
Copy Markdown
Member

We leave the content untouched (instead of replacing with null) and issue a warning.

This is far from ideal, as it can potentially spam the screen full of warnings. However, a better mechanism will require a total refactor of the SearchReplacer class.

Fixes #65

We leave the content untouched (instead of replacing with `null`) and issue a warning.

This is far from ideal, as it can potentially spam the screen full of warnings. However, a better mechanism will require a total refactor of the `SearchReplacer` class.

Fixes #65
@schlessera schlessera added the command:search-replace Related to 'search-replace' command label Apr 19, 2018
@schlessera schlessera added this to the 1.3.0 milestone Apr 19, 2018
@schlessera schlessera requested a review from a team April 19, 2018 11:15
@schlessera
Copy link
Copy Markdown
Member Author

I fail to find a working test to provoke a PCRE error (without just brute-forcing a stack overflow).

If anyone has a good idea or some sample code of how to provoke a runtime PCRE check, please let me know so I can add a regression test here.

@danielbachhuber
Copy link
Copy Markdown
Member

@schlessera Looks like tests need to be updated for trunk here too?

@schlessera
Copy link
Copy Markdown
Member Author

@danielbachhuber Yes => #78

@danielbachhuber danielbachhuber merged commit b45eeb4 into master Apr 20, 2018
@danielbachhuber danielbachhuber deleted the 65-check-for-pcre-error-on-return branch April 20, 2018 18:15
danielbachhuber added a commit that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:search-replace Related to 'search-replace' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants