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

Projects
None yet
3 participants
@phansch
Copy link
Contributor

commented Mar 25, 2019

Fixes #164

@phansch phansch force-pushed the phansch:fix_out_of_bounds branch from 87949a5 to df1bba0 Mar 25, 2019

@oli-obk
Copy link
Collaborator

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() {

This comment has been minimized.

Copy link
@oli-obk

oli-obk Mar 25, 2019

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

should do the same thing.

@killercup

This comment has been minimized.

Copy link
Collaborator

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Collaborator

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.

@killercup
Copy link
Collaborator

left a comment

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

@killercup killercup merged commit fa8a2f0 into rust-lang-nursery:master Mar 26, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@killercup

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2019

@phansch phansch deleted the phansch: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
You can’t perform that action at this time.