Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ws server]: should reply parse_error when deserialization fails #130

Closed
niklasad1 opened this issue Oct 15, 2020 · 3 comments
Closed

[ws server]: should reply parse_error when deserialization fails #130

niklasad1 opened this issue Oct 15, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@niklasad1
Copy link
Member

niklasad1 commented Oct 15, 2020

The server should reply with -32700 Parse error when the deserialization fails, see https://github.com/paritytech/jsonrpsee/blob/master/src/transport/ws/server.rs#L339-#L342

@niklasad1 niklasad1 added the bug Something isn't working label Oct 15, 2020
@tomusdrw
Copy link
Contributor

The problem with this is that you need to at least extract the id of the call for the response to make any sense (in WS the user can't match request with a response in any other way). We do it like this in jsonrpc:
https://github.com/paritytech/jsonrpc/blob/master/core/src/types/request.rs#L49

@niklasad1
Copy link
Member Author

niklasad1 commented Oct 15, 2020

Thanks, got it we do the same in jsonrpsee.

However, in the v2 we just return parse_error with id=null if the deserialization failed, see https://github.com/paritytech/jsonrpsee/blob/v2/src/ws/transport.rs#L423-#L426

After reading

If there was an error in detecting the id in the Request object (e.g. Parse error/Invalid Request), it MUST be Null.

I got a little worried that it's actually possible that we actually could detect the id when something else failed during parsing. But I think that is a bad idea because it could be parsing order dependent I guess?!

To elaborate what I mean is the following:

     # id is missing, can't be parsed
     req: {"jsonrpc":"2.0","method":"say_hello","id"}
     response:  {"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}
     # invalid params but id is valid,  deserialization could fail
     req: {"jsonrpc":"2.0","method":"say_hello", "params": [1, "id": 1212}
     response:  {"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":1212},`

@niklasad1
Copy link
Member Author

Fixed by #180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants