fix: Stop lonely hyphens from causing panic #411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The method `starts_with` as implemented for the `OsStrExt2` trait on `OsStr` assumed that the needle given is shorter than the haystack. When this is not the case, the method panics due to an attempted out-of-bounds access on the byte representation of `self`. Problematic if, say, an end-user gives us `"-"` and the library tries to see if that starts with `"--"`. Fortunately, slices already implement a `starts_with` method, and we can delegate to it. This *does* create a semantics change: if both `self` and the needle have length 0, this implementation will return `true`, but the old implementation would return `false`. Based on the test suite still passing, acknowledging the vacuous truth doesn't seem to cause any problems. Fixes #410
|
Thanks for the pull request, and welcome! The team is excited to review your changes, and you should hear from @kbknapp (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
📌 Commit 85b1146 has been approved by |
homu
added a commit
that referenced
this pull request
Feb 4, 2016
fix: Stop lonely hyphens from causing panic The method `starts_with` as implemented for the `OsStrExt2` trait on `OsStr` assumed that the needle given is shorter than the haystack. When this is not the case, the method panics due to an attempted out-of-bounds access on the byte representation of `self`. Problematic if, say, an end-user gives us `"-"` and the library tries to see if that starts with `"--"`. Fortunately, slices already implement a `starts_with` method, and we can delegate to it. This *does* create a semantics change: if both `self` and the needle have length 0, this implementation will return `true`, but the old implementation would return `false`. Based on the test suite still passing, acknowledging the vacuous truth doesn't seem to cause any problems. Fixes #410
homu
added a commit
that referenced
this pull request
Feb 4, 2016
fix: Stop lonely hyphens from causing panic The method `starts_with` as implemented for the `OsStrExt2` trait on `OsStr` assumed that the needle given is shorter than the haystack. When this is not the case, the method panics due to an attempted out-of-bounds access on the byte representation of `self`. Problematic if, say, an end-user gives us `"-"` and the library tries to see if that starts with `"--"`. Fortunately, slices already implement a `starts_with` method, and we can delegate to it. This *does* create a semantics change: if both `self` and the needle have length 0, this implementation will return `true`, but the old implementation would return `false`. Based on the test suite still passing, acknowledging the vacuous truth doesn't seem to cause any problems. Fixes #410
|
☀️ Test successful - status |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The method
starts_withas implemented for theOsStrExt2trait onOsStrassumed that the needle given is shorter than the haystack. Whenthis is not the case, the method panics due to an attempted
out-of-bounds access on the byte representation of
self. Problematicif, say, an end-user gives us
"-"and the library tries to see if thatstarts with
"--".Fortunately, slices already implement a
starts_withmethod, and we candelegate to it.
This does create a semantics change: if both
selfand the needlehave length 0, this implementation will return
true, but the oldimplementation would return
false. Based on the test suite stillpassing, acknowledging the vacuous truth doesn't seem to cause any
problems.
Fixes #410