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

Fix out of bounds access in parse_snippet #165

Merged
merged 2 commits into from Mar 26, 2019

Conversation

@phansch
Copy link
Member

@phansch phansch commented Mar 25, 2019

Fixes #164

Copy link
Collaborator

@oli-obk oli-obk left a comment

While I think rustc should provide sensible diagnostic spans, we should probably not panic on broken input. So, lgtm

src/lib.rs Outdated

// If we get a DiagnosticSpanLine where highlight_end > text.len(), we prevent an 'out of
// bounds' access by using text.len() - 1 instead.
let last_tail_index = if (last.highlight_end - 1) > last.text.len() {
Copy link
Collaborator

@oli-obk oli-obk Mar 25, 2019

let last_tail_index = last.highlight_end.min(last.text.len()) - 1;

should do the same thing.

@killercup
Copy link
Member

@killercup killercup commented Mar 25, 2019

Do we actually want to validate and fix this? We can also do last.text.get(indent..last_tail_index)? and just return None.

(I don't mind either way)

@phansch
Copy link
Member Author

@phansch phansch commented Mar 25, 2019

This particular diagnostic from rustc doesn't provide a suggestion anyway and I'm not sure if there are other diagnostics that provide a suggestion and would cause this crash :/

@oli-obk
Copy link
Collaborator

@oli-obk oli-obk commented Mar 25, 2019

I think truncating is fine. We can create a diagnostics json validator later which does a bunch of sanity checks. We can then use that to improve the output generated by rustc.

Copy link
Member

@killercup killercup left a comment

Cool. I'll publish a new version in a minute!

@killercup killercup merged commit fa8a2f0 into rust-lang:master Mar 26, 2019
2 checks passed
2 checks passed
@travis-ci[bot]
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@killercup
Copy link
Member

@killercup killercup commented Mar 26, 2019

@phansch phansch deleted the fix_out_of_bounds branch Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants