Preserve the Request ID in Invalid Request Error Responses
## Motivation and Context Resolves #398. `JsonRpcHandler` returned `"id": null` for JSON-RPC envelope errors (wrong or missing `jsonrpc` version, non-string `method`) even when the incoming payload carried a concrete, valid request id. From the client's perspective the original request stayed pending forever, since no response with the matching id ever arrived: ``` {"jsonrpc":"1.0","id":3,"method":"ping","params":{}} => {"jsonrpc":"2.0","id":null,"error":{"code":-32600,...}} ``` Per JSON-RPC 2.0 (Response object, `id`), the response id MUST be the same as the request's id; null is reserved for requests whose id could not be detected (e.g. Parse error). The envelope validation in `process_request` runs after JSON parsing with the request hash in hand, so the id is detectable - it was simply discarded by passing the `:unknown_id` sentinel unconditionally. The fix passes the request's id to `error_response` whenever the request carries one. The existing guards keep every other case intact: `error_response` already nils out ids that fail validation (so a type-invalid or pattern-violating id still yields `"id": null`, preserving the XSS-prevention policy on echoed ids), and the `:unknown_id` sentinel is kept for requests without an id so an Invalid Request error still produces a null-id response, matching the spec's own example. Parse errors and non-object requests are unchanged (null is correct there), and batch entries pick up the fix automatically since each entry goes through `process_request`. For reference, the TypeScript and Python SDKs currently respond with `"id": null` in these cases as well, because both validate the whole message against a schema (zod / pydantic) before extracting the id. This change is driven by the JSON-RPC 2.0 requirement rather than parity; the Ruby transport layer already echoes the request id where recoverable (`invalid_request_response(request_id:)` in `StreamableHTTPTransport`), and this closes the remaining gap in `JsonRpcHandler`. ## How Has This Been Tested? - Reproduced the three payloads from #398 before and after the fix; they now return `"id":3`, `"id":4`, and `"id":8` respectively - New and updated unit tests covering: id preserved for wrong version, missing `jsonrpc`, numeric `method`, and `rpc.`-prefixed `method`; id preserved for an invalid entry inside a batch; null id kept for requests without an id, with an explicit null id, and with an id that fails validation ## Breaking Changes None in the protocol sense - this aligns the error responses with JSON-RPC 2.0. Clients that matched `"id": null` on these envelope errors will now see the request's own id, which is what allows them to correlate the error with the pending request.
This commit is contained in:
parent
9193745291
commit
18761cf367
2 changed files with 82 additions and 4 deletions
|
|
@ -73,13 +73,18 @@ module JsonRpcHandler
|
|||
|
||||
error = if !valid_version?(request[:jsonrpc])
|
||||
"JSON-RPC version must be 2.0"
|
||||
elsif !valid_id?(request[:id], id_validation_pattern)
|
||||
elsif !valid_id?(id, id_validation_pattern)
|
||||
"Request ID must match validation pattern, or be an integer or null"
|
||||
elsif !valid_method_name?(request[:method])
|
||||
'Method name must be a string and not start with "rpc."'
|
||||
end
|
||||
|
||||
return error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: {
|
||||
# Per JSON-RPC 2.0 (Response object, `id`), the error response must carry
|
||||
# the same id as the request when the id could be detected; null is only
|
||||
# for requests whose id could not be determined. `error_response` nils out
|
||||
# ids that fail validation, and the `:unknown_id` sentinel keeps a response
|
||||
# (with a null id) being emitted when the request carried no id at all.
|
||||
return error_response(id: id.nil? ? :unknown_id : id, id_validation_pattern: id_validation_pattern, error: {
|
||||
code: ErrorCode::INVALID_REQUEST,
|
||||
message: "Invalid Request",
|
||||
data: error,
|
||||
|
|
|
|||
|
|
@ -43,6 +43,18 @@ describe JsonRpcHandler do
|
|||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_equal 1, @response[:id]
|
||||
end
|
||||
|
||||
it "returns an error preserving the request id when jsonrpc is missing" do
|
||||
handle id: 4, method: "add", params: { a: 1, b: 2 }
|
||||
|
||||
assert_rpc_error expected_error: {
|
||||
code: -32600,
|
||||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_equal 4, @response[:id]
|
||||
end
|
||||
|
||||
# method
|
||||
|
|
@ -58,6 +70,7 @@ describe JsonRpcHandler do
|
|||
message: "Invalid Request",
|
||||
data: 'Method name must be a string and not start with "rpc."',
|
||||
}
|
||||
assert_equal 1, @response[:id]
|
||||
end
|
||||
|
||||
it "returns an error when method begins with 'rpc.'" do
|
||||
|
|
@ -68,6 +81,7 @@ describe JsonRpcHandler do
|
|||
message: "Invalid Request",
|
||||
data: 'Method name must be a string and not start with "rpc."',
|
||||
}
|
||||
assert_equal 1, @response[:id]
|
||||
end
|
||||
|
||||
# params
|
||||
|
|
@ -315,6 +329,50 @@ describe JsonRpcHandler do
|
|||
assert_nil @response[:id]
|
||||
end
|
||||
|
||||
it "returns the same request id on an Invalid Request error when the id is detectable" do
|
||||
handle jsonrpc: "1.0", id: 3, method: "ping", params: {}
|
||||
|
||||
assert_rpc_error expected_error: {
|
||||
code: -32600,
|
||||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_equal 3, @response[:id]
|
||||
end
|
||||
|
||||
it "returns nil for id on an Invalid Request error when the request has no id" do
|
||||
handle jsonrpc: "1.0", method: "ping", params: {}
|
||||
|
||||
assert_rpc_error expected_error: {
|
||||
code: -32600,
|
||||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_nil @response[:id]
|
||||
end
|
||||
|
||||
it "returns nil for id on an Invalid Request error when the request id is explicitly null" do
|
||||
handle jsonrpc: "1.0", id: nil, method: "ping", params: {}
|
||||
|
||||
assert_rpc_error expected_error: {
|
||||
code: -32600,
|
||||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_nil @response[:id]
|
||||
end
|
||||
|
||||
it "returns nil for id on an Invalid Request error when the id fails validation" do
|
||||
handle jsonrpc: "1.0", id: "<script>alert('xss')</script>", method: "ping", params: {}
|
||||
|
||||
assert_rpc_error expected_error: {
|
||||
code: -32600,
|
||||
message: "Invalid Request",
|
||||
data: "JSON-RPC version must be 2.0",
|
||||
}
|
||||
assert_nil @response[:id]
|
||||
end
|
||||
|
||||
# 5.1 Error object
|
||||
#
|
||||
# When a rpc call encounters an error, the Response Object MUST contain the error member with a value that is a
|
||||
|
|
@ -610,6 +668,20 @@ describe JsonRpcHandler do
|
|||
|
||||
assert_nil @response
|
||||
end
|
||||
|
||||
it "preserves the request id of an invalid entry within a batch" do
|
||||
register("add") { |params| params[:a] + params[:b] }
|
||||
|
||||
handle [
|
||||
{ jsonrpc: "2.0", id: 100, method: "add", params: { a: 1, b: 2 } },
|
||||
{ jsonrpc: "1.0", id: 200, method: "add", params: { a: 3, b: 4 } },
|
||||
]
|
||||
|
||||
assert @response.is_a?(Array)
|
||||
assert_equal [100, 200], @response.map { |result| result[:id] }
|
||||
assert_equal 3, @response.first[:result]
|
||||
assert_equal(-32600, @response.last.dig(:error, :code))
|
||||
end
|
||||
end
|
||||
|
||||
# 7 Examples
|
||||
|
|
@ -756,10 +828,11 @@ describe JsonRpcHandler do
|
|||
assert_nil @response
|
||||
end
|
||||
|
||||
it "returns an error with the id set to nil when the request is invalid" do
|
||||
it "returns an error preserving the request id when the request is invalid" do
|
||||
handle_json({ jsonrpc: "0.0", id: 1, method: "add", params: { a: 1, b: 2 } }.to_json)
|
||||
|
||||
assert_nil @response[:id]
|
||||
assert_equal 1, @response[:id]
|
||||
assert_equal(-32600, @response.dig(:error, :code))
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue