fix(goto): stop slicing strings off char boundaries, which killed the server - #237
Merged
Merged
Conversation
… server
Go-to-definition on a file containing non-ASCII text could panic, and a panic
here takes the whole process down — the project index goes with it and the
editor has no language server until it is restarted.
thread 'main' panicked at src/goto.rs:1334:26:
start byte index 8 is not a char boundary; it is inside '状'
Two sites slice by offsets that are not guaranteed to sit on a boundary.
`validate_goto_target` slices the target line by LSP columns, guarded only by
a byte-length check on `end_col`. `start_col` was never checked at all, so an
inverted range panicked too, and on a line with multi-byte text either column
can land mid-character.
`goto_declaration_by_name` slices the file read fresh from disk using offsets
that came from an earlier solc build, so they can be stale as well as
mid-character.
Both functions are already best-effort and have give-up branches — `continue`
for an unparseable `src`, and "can't read target, assume valid" for an
unreadable file. An unusable range now takes those branches instead of
panicking, via `str::get` rather than indexing.
This is about not crashing. The underlying mismatch — LSP columns are
character offsets while these slices are by byte — is asyncswap#234 and is not fixed
here; `validate_goto_target` on a non-ASCII line now declines to judge rather
than answering wrongly, which is the same thing it already does when it cannot
read the file.
Three tests, all of which panic on main:
- a column landing inside a multi-byte character
- an inverted range, which the byte bound never rejected
- a stale build offset landing mid-character in the on-disk file
Plus an ASCII case asserting the matching behavior is unchanged.
Closes asyncswap#236.
rifuki
force-pushed
the
fix/goto-panics-on-non-ascii
branch
from
August 25, 2026 18:22
c877672 to
adcd134
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #236.
A panic in these two spots doesn't fail the request — it exits the process, so the project index is lost and the editor has no language server until it's restarted. That's why I'd put this ahead of my other open PRs.
validate_goto_targetsliced the target line by LSP columns with only a byte-length check onend_col.start_colwas never checked, so an inverted range panicked too, and on a line with multi-byte text either column can land mid-character.goto_declaration_by_namesliced the file read fresh from disk using offsets from an earlier solc build, which can be stale as well as mid-character.Both functions are already best-effort and have give-up branches —
continuefor an unparseablesrc, "can't read target, assume valid" for an unreadable file. Switching from indexing tostr::getjust routes an unusable range into those.Worth being clear about what this does and doesn't do: it stops the crash. The underlying mismatch — LSP columns are character offsets while these slices are by byte — is #234 and isn't fixed here. On a non-ASCII line
validate_goto_targetnow declines to judge instead of answering wrongly, which is what it already does when it can't read the file at all.Three tests, each of which panics on
main:I ran them against
mainfirst to confirm the exact panic (start byte index 8 is not a char boundary; it is inside '状') rather than assuming. There's also an ASCII case asserting the matching behavior is unchanged.cargo test663 passed, clippy and fmt clean.Independent of my other open PRs — this one only touches
goto.rs.