jhttp: Requests with invalid ID structure are incorrectly flagged as notifications #80
Assignees
Labels
bug
Something isn't working
Comments
creachadair
added a commit
that referenced
this issue
Feb 20, 2022
Add a new ParsedRequest type and return that instead of Request. This is a breaking change to the jrpc2.ParseRequests function. The Request is meant for consumption by Handler methods where validation has already been handled. The new type exposes validation errors, allowing the caller to detect specific problems with each request in a batch. This change also allows us to simplify checking of version errors, and to remove most of the special case version checks in the client and server. - Update the jhttp.Bridge to use the new ParsedRequest type. - Update test cases where affected. Add a testutil package for internal tests. - Fix handling of invalid requests in jhttp.Bridge (fixes #80).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The bridge parses requests with jrpc2.ParseRequests. This correctly detects invalid request ID structure (e.g.,
{"bogus":true}), but this error is not correctly propagated. In particular, ParseRequests does not check for validation errors, and the ID field isnilwhen the structure was invalid.This causes the bridge to treat the invalid request as a notification, as if its ID were not set at all. It is forwarded to the server, but the server does not see the problem since the ID on that request is not set.
Either a request needs to surface validation errors, or the bridge needs to use a smarter parsing structure.
The text was updated successfully, but these errors were encountered: