Skip to content

Commit

Permalink
Reject structurally invalid request IDs.
Browse files Browse the repository at this point in the history
The JSON-RPC 2.0 spec stipulates that request IDs should be strings or integral
number values. Prior to this commit, the library would accept basically any
JSON value as an ID. Now we reject objects (other than null), Booleans, and
arrays. The spec also says that fractional numbers should not be used, but we
still accept them.

Explicitly setting "id" to null is treated as equivalent to omitting the "id".
  • Loading branch information
creachadair committed Aug 2, 2021
1 parent a9921c4 commit 1eef6e8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
20 changes: 19 additions & 1 deletion base.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,20 @@ type jmessage struct {
err error // if not nil, this message is invalid and err is why
}

// isValidID reports whether v is a valid JSON encoding of a request ID.
// Precondition: v is a valid JSON value, or empty.
func isValidID(v json.RawMessage) bool {
if len(v) == 0 || isNull(v) {
return true // nil or empty is OK, as is "null"
} else if v[0] == '"' || v[0] == '-' || (v[0] >= '0' && v[0] <= '9') {
return true // strings and numbers are OK

// N.B. This definition does not reject fractional numbers, although the
// spec says numeric IDs should not have fractional parts.
}
return false // anything else is garbage
}

func (j *jmessage) fail(code code.Code, msg string) error {
j.err = Errorf(code, msg)
return j.err
Expand All @@ -312,7 +326,11 @@ func (j *jmessage) parseJSON(data []byte) error {
j.fail(code.ParseError, "invalid version key")
}
case "id":
j.ID = val
if isValidID(val) {
j.ID = val
} else {
j.fail(code.InvalidRequest, "invalid request ID")
}
case "method":
if json.Unmarshal(val, &j.M) != nil {
j.fail(code.ParseError, "invalid method name")
Expand Down
8 changes: 8 additions & 0 deletions jrpc2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ func TestOtherClient(t *testing.T) {
}
}()

const invalidIDMessage = `{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"invalid request ID"}}`
tests := []struct {
input, want string
}{
Expand Down Expand Up @@ -663,6 +664,13 @@ func TestOtherClient(t *testing.T) {
// A broken single request should report a top-level error.
{`{"bogus"][++`,
`{"jsonrpc":"2.0","id":null,"error":{"code":-32700,"message":"invalid request message"}}`},

// Various invalid ID checks.
{`{"jsonrpc":"2.0", "id":[], "method":"X"}`, invalidIDMessage}, // invalid ID: array
{`{"jsonrpc":"2.0", "id":["q"], "method":"X"}`, invalidIDMessage}, // "
{`{"jsonrpc":"2.0", "id":{}, "method":"X"}`, invalidIDMessage}, // invalid ID: object
{`{"jsonrpc":"2.0", "id":true, "method":"X"}`, invalidIDMessage}, // invalid ID: Boolean
{`{"jsonrpc":"2.0", "id":false, "method":"X"}`, invalidIDMessage}, // "
}
for _, test := range tests {
if err := cli.Send([]byte(test.input)); err != nil {
Expand Down

0 comments on commit 1eef6e8

Please sign in to comment.