diff --git a/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java b/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java index 39bdfed65..ff33b4cd9 100644 --- a/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java +++ b/jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java @@ -174,7 +174,13 @@ public static String toJsonStreamingEvent(StreamingEventKind data) throws JsonPr public static void writeJsonRpcId(JsonWriter out, @Nullable Object id) throws java.io.IOException { out.name("id"); if (id == null) { + // JSON-RPC 2.0 section 5: "id" is required in a response and must be null when the + // request id could not be determined. A JsonWriter that does not serialize nulls would + // otherwise drop the member. + boolean serializeNulls = out.getSerializeNulls(); + out.setSerializeNulls(true); out.nullValue(); + out.setSerializeNulls(serializeNulls); } else if (id instanceof Number n) { if (id instanceof Long || id instanceof Integer || id instanceof Short || id instanceof Byte) { out.value(n.longValue()); diff --git a/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtilTest.java b/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtilTest.java index aff3774c7..b07afa814 100644 --- a/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtilTest.java +++ b/jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtilTest.java @@ -3,11 +3,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.StringWriter; import java.util.Map; import com.google.gson.JsonObject; import com.google.gson.JsonParser; +import com.google.gson.stream.JsonWriter; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class JsonUtilTest { @@ -58,4 +62,27 @@ public void testReadMetadataStringConsistentWithJsonObjectOverload() throws Exce assertEquals(fromJsonObject, fromString); } + + // writeJsonRpcId tests + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testWriteJsonRpcIdWritesNullIdAndRestoresSerializeNulls(boolean serializeNulls) throws Exception { + StringWriter result = new StringWriter(); + JsonWriter out = new JsonWriter(result); + out.setSerializeNulls(serializeNulls); + + out.beginObject(); + JsonUtil.writeJsonRpcId(out, null); + assertEquals(serializeNulls, out.getSerializeNulls()); + out.name("other").nullValue(); + out.endObject(); + out.close(); + + // "id": null is always written; other null members follow the writer's own setting + JsonObject json = JsonParser.parseString(result.toString()).getAsJsonObject(); + assertTrue(json.has("id")); + assertTrue(json.get("id").isJsonNull()); + assertEquals(serializeNulls, json.has("other")); + } } diff --git a/reference/jsonrpc/src/test/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutesTest.java b/reference/jsonrpc/src/test/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutesTest.java index 9302da3d2..6e89f5f0d 100644 --- a/reference/jsonrpc/src/test/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutesTest.java +++ b/reference/jsonrpc/src/test/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutesTest.java @@ -16,8 +16,10 @@ import static io.vertx.core.http.HttpHeaders.CONTENT_TYPE; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -29,6 +31,9 @@ import java.util.concurrent.Executor; import java.util.concurrent.Flow; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + import jakarta.enterprise.inject.Instance; import org.a2aproject.sdk.jsonrpc.common.wrappers.CancelTaskRequest; @@ -43,6 +48,9 @@ import org.a2aproject.sdk.jsonrpc.common.wrappers.GetTaskResponse; import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTaskPushNotificationConfigsRequest; import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTaskPushNotificationConfigsResponse; +import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksRequest; +import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksResponse; +import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksResult; import org.a2aproject.sdk.jsonrpc.common.wrappers.SendMessageRequest; import org.a2aproject.sdk.jsonrpc.common.wrappers.SendMessageResponse; import org.a2aproject.sdk.jsonrpc.common.wrappers.SendStreamingMessageRequest; @@ -69,6 +77,8 @@ import io.vertx.ext.web.RoutingContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; /** @@ -728,6 +738,79 @@ public void testMethodNotFound_ContentTypeIsApplicationJson() { verify(mockHttpResponse).putHeader(CONTENT_TYPE, APPLICATION_JSON); } + @Test + public void testOmittedParams_ReturnsInvalidParamsError() { + // Arrange - "params" is omitted, which JSON-RPC 2.0 allows + String jsonRpcRequest = """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "SendMessage" + }"""; + when(mockRequestBody.asString()).thenReturn(jsonRpcRequest); + + // Act + routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext); + + // Assert - missing message is reported as Invalid params, not as an internal error, + // and the response keeps the request id + JsonObject response = captureResponse(); + assertEquals(-32602, response.getAsJsonObject("error").get("code").getAsInt()); + assertEquals(1, response.get("id").getAsInt()); + } + + @Test + public void testOmittedParams_ListTasksIsDispatched() { + // Arrange - "params" is omitted and every ListTasks field is optional + String jsonRpcRequest = """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "ListTasks" + }"""; + when(mockRequestBody.asString()).thenReturn(jsonRpcRequest); + when(mockJsonRpcHandler.onListTasks(any(ListTasksRequest.class), any(ServerCallContext.class))) + .thenReturn(new ListTasksResponse(1, new ListTasksResult(Collections.emptyList()))); + + // Act + routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext); + + // Assert - the request reaches the handler and the response carries no error + verify(mockJsonRpcHandler).onListTasks(any(ListTasksRequest.class), any(ServerCallContext.class)); + JsonObject response = captureResponse(); + assertFalse(response.has("error")); + assertEquals(1, response.get("id").getAsInt()); + } + + @ParameterizedTest + @ValueSource(strings = { + "[]", + "[{\"jsonrpc\": \"2.0\", \"id\": 1, \"method\": \"GetTask\", \"params\": {\"id\": \"task-1\"}}]", + "\"SendMessage\"", + "1", + "true", + "null" + }) + public void testNonObjectBody_ReturnsInvalidRequestError(String jsonRpcRequest) { + // Arrange - valid JSON, but not a JSON-RPC request object + when(mockRequestBody.asString()).thenReturn(jsonRpcRequest); + + // Act + routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext); + + // Assert - Invalid Request, and "id": null because no request id can be read + JsonObject response = captureResponse(); + assertEquals(-32600, response.getAsJsonObject("error").get("code").getAsInt()); + assertTrue(response.has("id")); + assertTrue(response.get("id").isJsonNull()); + } + + private JsonObject captureResponse() { + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(String.class); + verify(mockHttpResponse).end(bodyCaptor.capture()); + return JsonParser.parseString(bodyCaptor.getValue()).getAsJsonObject(); + } + @Test public void testGetAgentCardReturnsNullWhenNoPublicCard() throws Exception { when(mockJsonRpcHandler.getAgentCard()).thenReturn(null); diff --git a/spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java b/spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java index 7cecee5ed..271be2aff 100644 --- a/spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java +++ b/spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java @@ -181,6 +181,9 @@ public class JSONRPCUtils { public static A2ARequest parseRequestBody(String body, @Nullable String tenant) throws JsonMappingException, JsonProcessingException { JsonElement jelement = JsonParser.parseString(body); + if (!jelement.isJsonObject()) { + throw new JsonMappingException(null, "Invalid JSON-RPC request: the request must be a JSON object."); + } JsonObject jsonRpc = jelement.getAsJsonObject(); if (!jsonRpc.has("method")) { throw new IdJsonMappingException( @@ -204,53 +207,55 @@ private static void setTenantIfAbsent(Supplier existingTenantGetter, Con } } - private static A2ARequest parseMethodRequest(String version, Object id, String method, JsonElement paramsNode, @Nullable String tenant) throws InvalidParamsError, MethodNotFoundJsonMappingException, JsonProcessingException { + private static A2ARequest parseMethodRequest(String version, Object id, String method, @Nullable JsonElement paramsNode, @Nullable String tenant) throws InvalidParamsError, MethodNotFoundJsonMappingException, JsonProcessingException { + // JSON-RPC 2.0 allows "params" to be omitted: parse it like an empty params object. + JsonElement params = paramsNode == null ? new JsonObject() : paramsNode; switch (method) { case GET_TASK_METHOD -> { org.a2aproject.sdk.grpc.GetTaskRequest.Builder builder = org.a2aproject.sdk.grpc.GetTaskRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new GetTaskRequest(version, id, ProtoUtils.FromProto.taskQueryParams(builder)); } case CANCEL_TASK_METHOD -> { org.a2aproject.sdk.grpc.CancelTaskRequest.Builder builder = org.a2aproject.sdk.grpc.CancelTaskRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new CancelTaskRequest(version, id, ProtoUtils.FromProto.cancelTaskParams(builder)); } case LIST_TASK_METHOD -> { org.a2aproject.sdk.grpc.ListTasksRequest.Builder builder = org.a2aproject.sdk.grpc.ListTasksRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new ListTasksRequest(version, id, ProtoUtils.FromProto.listTasksParams(builder)); } case SET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> { org.a2aproject.sdk.grpc.TaskPushNotificationConfig.Builder builder = org.a2aproject.sdk.grpc.TaskPushNotificationConfig.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new CreateTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.createTaskPushNotificationConfig(builder)); } case GET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> { org.a2aproject.sdk.grpc.GetTaskPushNotificationConfigRequest.Builder builder = org.a2aproject.sdk.grpc.GetTaskPushNotificationConfigRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new GetTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.getTaskPushNotificationConfigParams(builder)); } case SEND_MESSAGE_METHOD -> { org.a2aproject.sdk.grpc.SendMessageRequest.Builder builder = org.a2aproject.sdk.grpc.SendMessageRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new SendMessageRequest(version, id, ProtoUtils.FromProto.messageSendParams(builder)); } case LIST_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> { org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.Builder builder = org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new ListTaskPushNotificationConfigsRequest(version, id, ProtoUtils.FromProto.listTaskPushNotificationConfigsParams(builder)); } case DELETE_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> { org.a2aproject.sdk.grpc.DeleteTaskPushNotificationConfigRequest.Builder builder = org.a2aproject.sdk.grpc.DeleteTaskPushNotificationConfigRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new DeleteTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.deleteTaskPushNotificationConfigParams(builder)); } @@ -268,13 +273,13 @@ private static A2ARequest parseMethodRequest(String version, Object id, Strin } case SEND_STREAMING_MESSAGE_METHOD -> { org.a2aproject.sdk.grpc.SendMessageRequest.Builder builder = org.a2aproject.sdk.grpc.SendMessageRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new SendStreamingMessageRequest(version, id, ProtoUtils.FromProto.messageSendParams(builder)); } case SUBSCRIBE_TO_TASK_METHOD -> { org.a2aproject.sdk.grpc.SubscribeToTaskRequest.Builder builder = org.a2aproject.sdk.grpc.SubscribeToTaskRequest.newBuilder(); - parseRequestBody(paramsNode, builder, id); + parseRequestBody(params, builder, id); setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant); return new SubscribeToTaskRequest(version, id, ProtoUtils.FromProto.taskIdParams(builder)); } diff --git a/spec-grpc/src/test/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtilsTest.java b/spec-grpc/src/test/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtilsTest.java index c072c1df9..967954a08 100644 --- a/spec-grpc/src/test/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtilsTest.java +++ b/spec-grpc/src/test/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtilsTest.java @@ -1,10 +1,16 @@ package org.a2aproject.sdk.grpc.utils; import static org.a2aproject.sdk.grpc.utils.JSONRPCUtils.ERROR_MESSAGE; +import static org.a2aproject.sdk.spec.A2AMethods.CANCEL_TASK_METHOD; +import static org.a2aproject.sdk.spec.A2AMethods.DELETE_TASK_PUSH_NOTIFICATION_CONFIG_METHOD; import static org.a2aproject.sdk.spec.A2AMethods.GET_TASK_METHOD; import static org.a2aproject.sdk.spec.A2AMethods.GET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD; +import static org.a2aproject.sdk.spec.A2AMethods.LIST_TASK_METHOD; +import static org.a2aproject.sdk.spec.A2AMethods.LIST_TASK_PUSH_NOTIFICATION_CONFIG_METHOD; import static org.a2aproject.sdk.spec.A2AMethods.SEND_MESSAGE_METHOD; +import static org.a2aproject.sdk.spec.A2AMethods.SEND_STREAMING_MESSAGE_METHOD; import static org.a2aproject.sdk.spec.A2AMethods.SET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD; +import static org.a2aproject.sdk.spec.A2AMethods.SUBSCRIBE_TO_TASK_METHOD; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -32,6 +38,7 @@ import org.a2aproject.sdk.jsonrpc.common.wrappers.GetTaskPushNotificationConfigRequest; import org.a2aproject.sdk.jsonrpc.common.wrappers.GetTaskPushNotificationConfigResponse; import org.a2aproject.sdk.jsonrpc.common.wrappers.GetTaskResponse; +import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksRequest; import org.a2aproject.sdk.jsonrpc.common.wrappers.SendMessageRequest; import org.a2aproject.sdk.spec.DataPart; import org.a2aproject.sdk.spec.GetExtendedAgentCardParams; @@ -45,6 +52,8 @@ import org.a2aproject.sdk.spec.TextPart; import org.a2aproject.sdk.spec.util.ErrorDetail; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class JSONRPCUtilsTest { @@ -228,6 +237,113 @@ public void testParseInvalidParams_ThrowsInvalidParamsJsonMappingException() { assertEquals(3, exception.getId()); } + @ParameterizedTest + @ValueSource(strings = { + GET_TASK_METHOD, CANCEL_TASK_METHOD, LIST_TASK_METHOD, SET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD, + GET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD, SEND_MESSAGE_METHOD, LIST_TASK_PUSH_NOTIFICATION_CONFIG_METHOD, + DELETE_TASK_PUSH_NOTIFICATION_CONFIG_METHOD, SEND_STREAMING_MESSAGE_METHOD, SUBSCRIBE_TO_TASK_METHOD + }) + public void testParseOmittedParams_SameAsEmptyParams(String method) { + // JSON-RPC 2.0 section 4: "params" MAY be omitted. An omitted "params" must be parsed + // like an empty params object instead of escaping parseRequestBody as a NullPointerException. + String omittedParamsRequest = """ + {"jsonrpc": "2.0", "method": "%s", "id": 7} + """.formatted(method); + String emptyParamsRequest = """ + {"jsonrpc": "2.0", "method": "%s", "id": 7, "params": {}} + """.formatted(method); + + assertEquals(parseOutcome(emptyParamsRequest), parseOutcome(omittedParamsRequest)); + } + + private static Object parseOutcome(String body) { + try { + return JSONRPCUtils.parseRequestBody(body, null).getParams(); + } catch (JsonProcessingException e) { + return e.getClass().getName() + ": " + e.getMessage(); + } + } + + @Test + public void testParseOmittedParams_ListTasksWithoutParams() throws Exception { + // All ListTasks fields are optional, so a request without "params" is valid. + String omittedParamsRequest = """ + { + "jsonrpc": "2.0", + "method": "ListTasks", + "id": 8 + } + """; + + ListTasksRequest request = assertInstanceOf(ListTasksRequest.class, + JSONRPCUtils.parseRequestBody(omittedParamsRequest, null)); + assertEquals(8, request.getId()); + assertNotNull(request.getParams()); + } + + @Test + public void testParseExplicitNullParams_IsNotTreatedAsOmitted() { + // Only an omitted "params" is normalized to {}. An explicit "params": null keeps its + // existing handling, so ListTasks (valid without params) still rejects it. + String explicitNullParamsRequest = """ + {"jsonrpc": "2.0", "method": "ListTasks", "id": 9, "params": null} + """; + + assertThrows( + JsonMappingException.class, + () -> JSONRPCUtils.parseRequestBody(explicitNullParamsRequest, null) + ); + } + + @ParameterizedTest + @ValueSource(strings = { + "{\"jsonrpc\": \"2.0\", \"method\": \"GetExtendedAgentCard\", \"id\": 10}", + "{\"jsonrpc\": \"2.0\", \"method\": \"GetExtendedAgentCard\", \"id\": 10, \"params\": null}" + }) + public void testParseGetExtendedAgentCard_AbsentParamsUnchanged(String body) throws Exception { + // GetExtendedAgentCard keeps its own handling of absent params. + GetExtendedAgentCardRequest request = assertInstanceOf(GetExtendedAgentCardRequest.class, + JSONRPCUtils.parseRequestBody(body, null)); + assertEquals(10, request.getId()); + } + + @ParameterizedTest + @ValueSource(strings = {SEND_MESSAGE_METHOD, SEND_STREAMING_MESSAGE_METHOD}) + public void testParseOmittedParams_RequiredFieldsMissing_ThrowsInvalidParams(String method) { + // "message" is required, and the existing validation rejects "params": {} with Invalid params + // (-32602). An omitted "params" must hit the same validation and keep the request id, + // instead of failing with an internal error. + String omittedParamsRequest = """ + {"jsonrpc": "2.0", "method": "%s", "id": 7} + """.formatted(method); + + InvalidParamsJsonMappingException exception = assertThrows( + InvalidParamsJsonMappingException.class, + () -> JSONRPCUtils.parseRequestBody(omittedParamsRequest, null) + ); + assertEquals(7, exception.getId()); + } + + @ParameterizedTest + @ValueSource(strings = { + "[]", + "[{\"jsonrpc\": \"2.0\", \"method\": \"GetTask\", \"id\": 1, \"params\": {\"id\": \"task-1\"}}]", + "\"GetTask\"", + "42", + "true", + "null" + }) + public void testParseNonObjectBody_ThrowsJsonMappingException(String body) { + // JSON-RPC 2.0 section 5.1: a body that is valid JSON but not a Request object + // (an array, including a batch, or a primitive) must yield InvalidRequest (-32600), + // which the server routes derive from a plain JsonMappingException. + JsonMappingException exception = assertThrows( + JsonMappingException.class, + () -> JSONRPCUtils.parseRequestBody(body, null) + ); + assertEquals(JsonMappingException.class, exception.getClass()); + } + @Test public void testParseInvalidProtoStructure_ThrowsInvalidParamsJsonMappingException() { String invalidStructure = """