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

Prevent crash when user provides bad input. #1803

Closed

Conversation

hbina
Copy link
Contributor

@hbina hbina commented Feb 12, 2021

Running "echo '' | rg --crlf x?" should no longer crash.
I probably introduced performance regressions haha

Related issue: #1765

aww shucks, sublice pattern is unstable. If you really want this, I could reimplement it using more backward compat ways.

Signed-off-by: Hanif Bin Ariffin hanif.ariffin.4326@gmail.com

Running "echo '' | rg --crlf x?" should no longer crash.
I probably introduced performance regressions haha

Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
@BurntSushi
Copy link
Owner

Thanks! But I believe it is intentional that is_suffix is defined this way. I ended up going in a different direction. (See linked commit.)

BurntSushi added a commit that referenced this pull request May 30, 2021
This fixes a bug where it was assumed that 'is_suffix' when CRLF
handling was enabled mean that '\r\n' was present. But that's not the
case, and it is intentional that 'is_suffix' only looks for '\n'. (Which
is why #1803 wasn't taken, which tries to fix this by changing
'is_suffix'.)

Fixes #1765, Closes #1803
@BurntSushi BurntSushi added the rollup A PR that has been merged with many others in a rollup. label May 30, 2021
BurntSushi added a commit that referenced this pull request May 31, 2021
This fixes a bug where it was assumed that 'is_suffix' when CRLF
handling was enabled mean that '\r\n' was present. But that's not the
case, and it is intentional that 'is_suffix' only looks for '\n'. (Which
is why #1803 wasn't taken, which tries to fix this by changing
'is_suffix'.)

Fixes #1765, Closes #1803
BurntSushi added a commit that referenced this pull request Jun 1, 2021
This fixes a bug where it was assumed that 'is_suffix' when CRLF
handling was enabled mean that '\r\n' was present. But that's not the
case, and it is intentional that 'is_suffix' only looks for '\n'. (Which
is why #1803 wasn't taken, which tries to fix this by changing
'is_suffix'.)

Fixes #1765, Closes #1803
@BurntSushi BurntSushi closed this in 12dd455 Jun 1, 2021
@hbina hbina deleted the fix/dont-crash-with-bad-input-in-crlf branch June 1, 2021 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR that has been merged with many others in a rollup.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants