fix(jsonrpc): handle omitted params and non-object request bodies

[Zhuoxi2000]

Description

JSON-RPC 2.0 lets a request omit params (section 4). JSONRPCUtils.parseRequestBody passed the missing member down as null, and every method except GetExtendedAgentCard hit a NullPointerException in parseRequestBody(JsonElement, ...). A body that is valid JSON but not an object (for example []) threw IllegalStateException from getAsJsonObject(). A2AServerRoutes maps neither exception, so both ended up in catch (Throwable): the server logged an ERROR stack trace and the client got -32603 Internal error.

Changes:

• parseRequestBody(String, String) now rejects a non-object body with a JsonMappingException. A2AServerRoutes already maps that to -32600 Invalid Request, the code JSON-RPC 2.0 section 5.1 defines for "not a valid Request object".
• parseMethodRequest parses an omitted params like an empty params object ({}). ListTasks (all fields optional) now succeeds without params. Methods with required fields go through the same validation they already apply to "params": {}; for example SendMessage without params now returns -32602 Invalid params ("Parameter 'message' may not be null"). The GetExtendedAgentCard branch is unchanged and still treats missing params as absent.
• JsonUtil.writeJsonRpcId (jsonrpc-common) now actually writes "id": null. JSON-RPC 2.0 section 5 requires id in every response, set to null when the request id could not be determined. The method called nullValue(), but error responses are written with a JsonWriter that does not serialize nulls, so the member was silently dropped and these error responses had no id at all. Null serialization is now enabled just for that value. This affects every error response without a known id (parse errors, Invalid Request), not only the cases in this PR.

The response-parsing paths (parseResponseEvent / parseResponseBody) are not touched.

Notes on the review points from #1196:

• GetExtendedAgentCard: its branch still reads the raw params member, so absent parameters are handled exactly as before; testParseGetExtendedAgentCard_AbsentParamsUnchanged covers both an omitted and an explicit-null params.
• Only an omitted params is normalized. The check is on the member being absent. An explicit "params": null arrives as JsonNull and goes through the existing path unchanged; testParseExplicitNullParams_IsNotTreatedAsOmitted pins this.
• Response ids: invalid-params responses keep the request id, and non-object bodies now get "id": null (see the JsonUtil change above). Both are asserted at the route level.
• Client side: JSONRPCUtils.parseResponseBody already rejected an error response with no usable id, and it still does. Only the message differs: it is now "Invalid 'id' type: JsonNull" instead of "Request 'id' cannot be null". Letting the client surface the error object of a null-id response would be a separate change.
• Batch requests: [] is an invalid request per the spec. A non-empty array is a valid batch in JSON-RPC 2.0, but this server does not implement batches, so it now gets -32600 instead of -32603. Supporting batches would be a separate change.

Tests, covering the cases requested in #1196:

• JSONRPCUtilsTest
  • testParseOmittedParams_SameAsEmptyParams (parameterized over the 10 methods that parse params): an omitted params gives the same result as "params": {}.
  • testParseOmittedParams_ListTasksWithoutParams: ListTasks without params parses, with non-null params.
  • testParseOmittedParams_RequiredFieldsMissing_ThrowsInvalidParams (SendMessage, SendStreamingMessage): an omitted params hits the existing required-field validation and throws InvalidParamsJsonMappingException with the request id.
  • testParseNonObjectBody_ThrowsJsonMappingException (parameterized: [], a one-element batch array, a string, a number, true, null): a plain JsonMappingException is thrown.
  • testParseExplicitNullParams_IsNotTreatedAsOmitted: ListTasks with "params": null is still rejected.
  • testParseGetExtendedAgentCard_AbsentParamsUnchanged (omitted and explicit-null params).
• A2AServerRoutesTest
  • testOmittedParams_ReturnsInvalidParamsError: SendMessage without params gets -32602 with "id": 1.
  • testOmittedParams_ListTasksIsDispatched: ListTasks without params reaches JSONRPCHandler.onListTasks, and the response carries no error and "id": 1.
  • testNonObjectBody_ReturnsInvalidRequestError (same six bodies): each one gets -32600 with "id": null.

The other methods with required fields (for example GetTask without an id) parse "params": {} without an error today. With this change an omitted params behaves exactly the same, which testParseOmittedParams_SameAsEmptyParams checks. Tightening that validation is out of scope here.

Test evidence (JDK 17):

mvn -B -pl spec-grpc,reference/jsonrpc -am install -DskipTests
mvn -B -pl spec-grpc,reference/jsonrpc test -Dtest='JSONRPCUtilsTest,A2AServerRoutesTest' -Dsurefire.failIfNoSpecifiedTests=false

• With this change, JSONRPCUtilsTest 49/49 and A2AServerRoutesTest 27/27 pass. mvn -pl jsonrpc-common,spec-grpc,transport/jsonrpc,reference/jsonrpc,client/transport/jsonrpc -am test also passes, including the full reference JSON-RPC server suite.

• With JSONRPCUtils.java and JsonUtil.java reverted to main, the new tests fail:

  • JSONRPCUtilsTest: 8 failures and 11 errors, made up of:
    • NullPointerException: Cannot invoke "com.google.gson.JsonElement.toString()" because "jsonRpc" is null for the omitted-params cases;
    • IllegalStateException: Not a JSON Object: ... for each non-object body.
  • A2AServerRoutesTest: 8 failures, made up of:
    • expected: <-32600> but was: <-32603> for each of the six non-object bodies;
    • expected: <-32602> but was: <-32603> for SendMessage;
    • the ListTasks dispatch check.

• With only JsonUtil.java reverted, the six non-object route cases fail because the response has no id member.

• Follow the CONTRIBUTING Guide.

• Make your Pull Request title in the https://www.conventionalcommits.org/ specification.

• Ensure the tests pass

• Appropriate READMEs were updated (if necessary): not needed

This fixes #1196

[kabir]

Thanks @Zhuoxi2000!

Could you also add a test in JsonUtilTest for writeJsonRpcId? The method temporarily enables writing null values so that "id": null appears in the response, then restores the writer’s previous setting.

The route tests already verify that "id": null appears. A direct test would also verify that the writer’s setting is restored afterward, so writing the ID doesn’t change how other null fields are serialized. Please cover both starting settings: null serialization enabled and disabled.

This is an optional improvement, not a blocker.

[Zhuoxi2000]

Thanks @kabir, added in 87a442e: JsonUtilTest.testWriteJsonRpcIdWritesNullIdAndRestoresSerializeNulls, parameterized over both starting settings. It checks that "id": null is written, that getSerializeNulls() is back to its starting value right after the call, and that a later null member follows that setting.

Sanity check: with writeJsonRpcId from main, the false case fails because there is no id member. With a variant that enables null serialization but never restores it, the false case fails on the restore assertion.

[kabir]

Thank you @Zhuoxi2000