ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
1k stars 55 forks source link

Fix client response deserialization by fixing serde overlap #269

Closed ebkalderon closed 3 years ago

ebkalderon commented 3 years ago

Fixed

It seems that client request support has been broken since 0.13.3 due to an overlap in deserialization of incoming client responses, causing them to be incorrectly identified by serde as server RequestKind::Invalid instead of ClientMethod::Response.

This occurred due to the variant ordering of the Incoming enum, where the Incoming::Request branch would always be attempted before Incoming::Response. This meant that the overly permissive catch-all "invalid server request" case would swallow up the incoming client response and the Incoming::Response branch would never be attempted.

By simply swapping the order of the enum variants, we can ensure that the incoming deserialization order is fixed as follows:

  1. Incoming client response
  2. Incoming server request (valid)
  3. Incoming server request (invalid, fall-through case)

To prevent such a bug from occurring again, this commit extends and hardens the jsonrpc.rs unit test suite to check that both Incoming and Outgoing parse JSON-RPC requests and responses correctly and verify that the correct enum variants are chosen as a result.

Fixes #267. Closes #244, which previously attempted to fix this issue but broke JSON-RPC status code correctness along the way.