Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Fix crash if there is no newline at the end of the last line #83

Merged
merged 1 commit into from
Apr 16, 2023

Conversation

tcx4c70
Copy link
Contributor

@tcx4c70 tcx4c70 commented Apr 16, 2023

The Range may extend the actual range of the text according to the doc of LSP:

... the end position is exclusive. If you want to specify a range
that contains a line including the line ending character(s) then use
an end position denoting the start of the next line.

If there is no newline at the end of the last line (which is common for files created by Vistual Studio) and client set the end of Range to the start of the next line of the last line, then csharp-ls will crash because argument is out of the range. On the other hand, we SHOULDN'T trust the value client sent to us.

The `Range` may extend the actual range of the text according to the doc
of LSP:
> ... the end position is exclusive. If you want to specify a range
> that contains a line including the line ending character(s) then use
> an end position denoting the start of the next line.

If there is no newline at the end of the last line (which is common for
files created by Vistual Studio) and client set the end of Range to the
start of the next line of the last line, then csharp-ls will crash
because argument is out of the range. On the other hand, we SHOULDN'T
trust the value client sent to us.

Signed-off-by: Adam Tao <tcx4c70@gmail.com>
@razzmatazz
Copy link
Owner

this definitely makes sense.. will check and merge

thanks!

@razzmatazz razzmatazz merged commit f81bf89 into razzmatazz:master Apr 16, 2023
@tcx4c70 tcx4c70 deleted the fix/validate-range branch April 22, 2023 11:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants