ebkalderon / tower-lsp

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

Restrict JSON-RPC request/notification params to objects and arrays #384

Open ebkalderon opened 1 year ago

ebkalderon commented 1 year ago

According to both the JSON-RPC 2.0 specification and the Language Server Protocol specification, the params field in all JSON-RPC requests and notifications must be either:

  • by-position: params MUST be an Array, containing the values in the Server expected order.
  • by-name: params MUST be an Object, with member names that match the Server expected parameter names. The absence of expected names MAY result in an error being generated. The names MUST match exactly, including case, to the method's expected parameters.

At the time of writing, tower-lsp's Request type does not reject messages with params fields that are not either object-like or array-like. This was done to accommodate the telemetry/event notification which, until recently, erroneously mandated a params type of object | number | boolean | string. Thankfully, this was corrected in the spec: https://github.com/microsoft/language-server-protocol/issues/1686

We already restrict users from emitting non-object and non-array telemetry/event payloads in client.rs, so there's no additional work necessary on that front.

https://github.com/ebkalderon/tower-lsp/blob/e96d8ca822e63218dab8bae8825a7f8a4537fea0/src/service/client.rs#L203-L211

However, we should still update the Request type and LspServiceBuilder::custom_method() to reject params values that are not serializable/deserializable to and from objects/arrays in order to fully comply.

ebkalderon commented 1 year ago

I've opened https://github.com/gluon-lang/lsp-types/pull/259 to correct this at the lsp-types level.

ebkalderon commented 1 year ago

This issue was previously touched on in PR #379. Most of the code in improve-jsonrpc-deserialization that hasn't yet been integrated into mainline covers a big portion of this work already.