search

ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Optimize message parsing; fix #314 #316

Closed silvanshade closed 2 years ago

silvanshade commented 2 years ago

I've refactored the decoder implementation and believe the new version should address the comments from https://github.com/ebkalderon/tower-lsp/issues/314.

silvanshade commented 2 years ago

One thing to note about this, in the recovers_from_parse_error test, there is a section of headers at the beginning like this:

foobar{}Content-Length: foobar\r\n\r\n

This could be handled two different ways.

The first way is the current behavior, which treats foobar in foobarContent-Length as garbage bytes, and first strips that and advances and then tries to parse Content-Length: foobar (with the result being an error about invalid content length).

The second way would be to view foobarContent-Length: foobar as a valid header (which httparse parses correctly) and to handle the whole foobar{}Content-Length: foobar\r\n\r\n header section as missing the actual Content-Length header.

I think that technically the second approach is more accurate, but I've opted to keep the first approach because otherwise the recovers_from_parse_error breaks as it is currently written (although it's easy to change).

I don't really have a strong opinion on which approach to use but I thought it would be good to point out that there is a kind of implicit choice that we are making here regarding error handling and recovery when parsing headers.