Skip to content

Commit a51b6b3

Browse files
Zhuoxi2000Zhuoxi2000
authored
fix(jsonrpc): handle omitted params and non-object request bodies (#1198)
* fix(jsonrpc): handle omitted params and non-object request bodies JSON-RPC 2.0 allows a request to omit "params", but parseRequestBody passed the missing member down as null and every method except GetExtendedAgentCard failed with a NullPointerException. A body that is valid JSON but not an object (for example "[]") failed with an IllegalStateException from getAsJsonObject(). Neither exception is mapped by A2AServerRoutes, so both were logged at ERROR level and returned to the client as -32603 Internal error. An omitted "params" is now parsed like an empty params object, and a non-object body is rejected with a JsonMappingException, which the JSON-RPC route maps to -32600 Invalid Request. An explicit "params": null keeps its existing handling. JSON-RPC 2.0 (section 5) also requires "id" in every response, set to null when the request id could not be determined. writeJsonRpcId called nullValue(), but error responses are written with a JsonWriter that does not serialize nulls, so the member was dropped. Enable null serialization just for that value. This fixes #1196 * test(jsonrpc): cover writeJsonRpcId with a null id Check that "id": null is written whether or not the writer serializes nulls, and that the writer's own setting is restored afterwards, so other null members keep following it. This fixes #1196
1 parent a6cbb0a commit a51b6b3

5 files changed

Lines changed: 248 additions & 11 deletions

File tree

‎jsonrpc-common/src/main/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtil.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,13 @@ public static String toJsonStreamingEvent(StreamingEventKind data) throws JsonPr
174174
public static void writeJsonRpcId(JsonWriter out, @Nullable Object id) throws java.io.IOException {
175175
out.name("id");
176176
if (id == null) {
177+
// JSON-RPC 2.0 section 5: "id" is required in a response and must be null when the
178+
// request id could not be determined. A JsonWriter that does not serialize nulls would
179+
// otherwise drop the member.
180+
boolean serializeNulls = out.getSerializeNulls();
181+
out.setSerializeNulls(true);
177182
out.nullValue();
183+
out.setSerializeNulls(serializeNulls);
178184
} else if (id instanceof Number n) {
179185
if (id instanceof Long || id instanceof Integer || id instanceof Short || id instanceof Byte) {
180186
out.value(n.longValue());

‎jsonrpc-common/src/test/java/org/a2aproject/sdk/jsonrpc/common/json/JsonUtilTest.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertTrue;
55

6+
import java.io.StringWriter;
67
import java.util.Map;
78

89
import com.google.gson.JsonObject;
910
import com.google.gson.JsonParser;
11+
import com.google.gson.stream.JsonWriter;
1012
import org.junit.jupiter.api.Test;
13+
import org.junit.jupiter.params.ParameterizedTest;
14+
import org.junit.jupiter.params.provider.ValueSource;
1115

1216
public class JsonUtilTest {
1317

@@ -58,4 +62,27 @@ public void testReadMetadataStringConsistentWithJsonObjectOverload() throws Exce
5862

5963
assertEquals(fromJsonObject, fromString);
6064
}
65+
66+
// writeJsonRpcId tests
67+
68+
@ParameterizedTest
69+
@ValueSource(booleans = {true, false})
70+
public void testWriteJsonRpcIdWritesNullIdAndRestoresSerializeNulls(boolean serializeNulls) throws Exception {
71+
StringWriter result = new StringWriter();
72+
JsonWriter out = new JsonWriter(result);
73+
out.setSerializeNulls(serializeNulls);
74+
75+
out.beginObject();
76+
JsonUtil.writeJsonRpcId(out, null);
77+
assertEquals(serializeNulls, out.getSerializeNulls());
78+
out.name("other").nullValue();
79+
out.endObject();
80+
out.close();
81+
82+
// "id": null is always written; other null members follow the writer's own setting
83+
JsonObject json = JsonParser.parseString(result.toString()).getAsJsonObject();
84+
assertTrue(json.has("id"));
85+
assertTrue(json.get("id").isJsonNull());
86+
assertEquals(serializeNulls, json.has("other"));
87+
}
6188
}

‎reference/jsonrpc/src/test/java/org/a2aproject/sdk/server/apps/quarkus/A2AServerRoutesTest.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616
import static io.vertx.core.http.HttpHeaders.CONTENT_TYPE;
1717
import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
1818
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
import static org.junit.jupiter.api.Assertions.assertFalse;
1920
import static org.junit.jupiter.api.Assertions.assertNotNull;
2021
import static org.junit.jupiter.api.Assertions.assertNull;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
2123
import static org.mockito.ArgumentMatchers.any;
2224
import static org.mockito.ArgumentMatchers.anyString;
2325
import static org.mockito.Mockito.mock;
@@ -29,6 +31,9 @@
2931
import java.util.concurrent.Executor;
3032
import java.util.concurrent.Flow;
3133

34+
import com.google.gson.JsonObject;
35+
import com.google.gson.JsonParser;
36+
3237
import jakarta.enterprise.inject.Instance;
3338

3439
import org.a2aproject.sdk.jsonrpc.common.wrappers.CancelTaskRequest;
@@ -43,6 +48,9 @@
4348
import org.a2aproject.sdk.jsonrpc.common.wrappers.GetTaskResponse;
4449
import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTaskPushNotificationConfigsRequest;
4550
import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTaskPushNotificationConfigsResponse;
51+
import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksRequest;
52+
import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksResponse;
53+
import org.a2aproject.sdk.jsonrpc.common.wrappers.ListTasksResult;
4654
import org.a2aproject.sdk.jsonrpc.common.wrappers.SendMessageRequest;
4755
import org.a2aproject.sdk.jsonrpc.common.wrappers.SendMessageResponse;
4856
import org.a2aproject.sdk.jsonrpc.common.wrappers.SendStreamingMessageRequest;
@@ -69,6 +77,8 @@
6977
import io.vertx.ext.web.RoutingContext;
7078
import org.junit.jupiter.api.BeforeEach;
7179
import org.junit.jupiter.api.Test;
80+
import org.junit.jupiter.params.ParameterizedTest;
81+
import org.junit.jupiter.params.provider.ValueSource;
7282
import org.mockito.ArgumentCaptor;
7383

7484
/**
@@ -728,6 +738,79 @@ public void testMethodNotFound_ContentTypeIsApplicationJson() {
728738
verify(mockHttpResponse).putHeader(CONTENT_TYPE, APPLICATION_JSON);
729739
}
730740

741+
@Test
742+
public void testOmittedParams_ReturnsInvalidParamsError() {
743+
// Arrange - "params" is omitted, which JSON-RPC 2.0 allows
744+
String jsonRpcRequest = """
745+
{
746+
"jsonrpc": "2.0",
747+
"id": 1,
748+
"method": "SendMessage"
749+
}""";
750+
when(mockRequestBody.asString()).thenReturn(jsonRpcRequest);
751+
752+
// Act
753+
routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext);
754+
755+
// Assert - missing message is reported as Invalid params, not as an internal error,
756+
// and the response keeps the request id
757+
JsonObject response = captureResponse();
758+
assertEquals(-32602, response.getAsJsonObject("error").get("code").getAsInt());
759+
assertEquals(1, response.get("id").getAsInt());
760+
}
761+
762+
@Test
763+
public void testOmittedParams_ListTasksIsDispatched() {
764+
// Arrange - "params" is omitted and every ListTasks field is optional
765+
String jsonRpcRequest = """
766+
{
767+
"jsonrpc": "2.0",
768+
"id": 1,
769+
"method": "ListTasks"
770+
}""";
771+
when(mockRequestBody.asString()).thenReturn(jsonRpcRequest);
772+
when(mockJsonRpcHandler.onListTasks(any(ListTasksRequest.class), any(ServerCallContext.class)))
773+
.thenReturn(new ListTasksResponse(1, new ListTasksResult(Collections.emptyList())));
774+
775+
// Act
776+
routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext);
777+
778+
// Assert - the request reaches the handler and the response carries no error
779+
verify(mockJsonRpcHandler).onListTasks(any(ListTasksRequest.class), any(ServerCallContext.class));
780+
JsonObject response = captureResponse();
781+
assertFalse(response.has("error"));
782+
assertEquals(1, response.get("id").getAsInt());
783+
}
784+
785+
@ParameterizedTest
786+
@ValueSource(strings = {
787+
"[]",
788+
"[{\"jsonrpc\": \"2.0\", \"id\": 1, \"method\": \"GetTask\", \"params\": {\"id\": \"task-1\"}}]",
789+
"\"SendMessage\"",
790+
"1",
791+
"true",
792+
"null"
793+
})
794+
public void testNonObjectBody_ReturnsInvalidRequestError(String jsonRpcRequest) {
795+
// Arrange - valid JSON, but not a JSON-RPC request object
796+
when(mockRequestBody.asString()).thenReturn(jsonRpcRequest);
797+
798+
// Act
799+
routes.invokeJSONRPCHandler(jsonRpcRequest, mockRoutingContext);
800+
801+
// Assert - Invalid Request, and "id": null because no request id can be read
802+
JsonObject response = captureResponse();
803+
assertEquals(-32600, response.getAsJsonObject("error").get("code").getAsInt());
804+
assertTrue(response.has("id"));
805+
assertTrue(response.get("id").isJsonNull());
806+
}
807+
808+
private JsonObject captureResponse() {
809+
ArgumentCaptor<String> bodyCaptor = ArgumentCaptor.forClass(String.class);
810+
verify(mockHttpResponse).end(bodyCaptor.capture());
811+
return JsonParser.parseString(bodyCaptor.getValue()).getAsJsonObject();
812+
}
813+
731814
@Test
732815
public void testGetAgentCardReturnsNullWhenNoPublicCard() throws Exception {
733816
when(mockJsonRpcHandler.getAgentCard()).thenReturn(null);

‎spec-grpc/src/main/java/org/a2aproject/sdk/grpc/utils/JSONRPCUtils.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ public class JSONRPCUtils {
181181

182182
public static A2ARequest<?> parseRequestBody(String body, @Nullable String tenant) throws JsonMappingException, JsonProcessingException {
183183
JsonElement jelement = JsonParser.parseString(body);
184+
if (!jelement.isJsonObject()) {
185+
throw new JsonMappingException(null, "Invalid JSON-RPC request: the request must be a JSON object.");
186+
}
184187
JsonObject jsonRpc = jelement.getAsJsonObject();
185188
if (!jsonRpc.has("method")) {
186189
throw new IdJsonMappingException(
@@ -204,53 +207,55 @@ private static void setTenantIfAbsent(Supplier<String> existingTenantGetter, Con
204207
}
205208
}
206209

207-
private static A2ARequest<?> parseMethodRequest(String version, Object id, String method, JsonElement paramsNode, @Nullable String tenant) throws InvalidParamsError, MethodNotFoundJsonMappingException, JsonProcessingException {
210+
private static A2ARequest<?> parseMethodRequest(String version, Object id, String method, @Nullable JsonElement paramsNode, @Nullable String tenant) throws InvalidParamsError, MethodNotFoundJsonMappingException, JsonProcessingException {
211+
// JSON-RPC 2.0 allows "params" to be omitted: parse it like an empty params object.
212+
JsonElement params = paramsNode == null ? new JsonObject() : paramsNode;
208213
switch (method) {
209214
case GET_TASK_METHOD -> {
210215
org.a2aproject.sdk.grpc.GetTaskRequest.Builder builder = org.a2aproject.sdk.grpc.GetTaskRequest.newBuilder();
211-
parseRequestBody(paramsNode, builder, id);
216+
parseRequestBody(params, builder, id);
212217
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
213218
return new GetTaskRequest(version, id, ProtoUtils.FromProto.taskQueryParams(builder));
214219
}
215220
case CANCEL_TASK_METHOD -> {
216221
org.a2aproject.sdk.grpc.CancelTaskRequest.Builder builder = org.a2aproject.sdk.grpc.CancelTaskRequest.newBuilder();
217-
parseRequestBody(paramsNode, builder, id);
222+
parseRequestBody(params, builder, id);
218223
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
219224
return new CancelTaskRequest(version, id, ProtoUtils.FromProto.cancelTaskParams(builder));
220225
}
221226
case LIST_TASK_METHOD -> {
222227
org.a2aproject.sdk.grpc.ListTasksRequest.Builder builder = org.a2aproject.sdk.grpc.ListTasksRequest.newBuilder();
223-
parseRequestBody(paramsNode, builder, id);
228+
parseRequestBody(params, builder, id);
224229
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
225230
return new ListTasksRequest(version, id, ProtoUtils.FromProto.listTasksParams(builder));
226231
}
227232
case SET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> {
228233
org.a2aproject.sdk.grpc.TaskPushNotificationConfig.Builder builder = org.a2aproject.sdk.grpc.TaskPushNotificationConfig.newBuilder();
229-
parseRequestBody(paramsNode, builder, id);
234+
parseRequestBody(params, builder, id);
230235
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
231236
return new CreateTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.createTaskPushNotificationConfig(builder));
232237
}
233238
case GET_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> {
234239
org.a2aproject.sdk.grpc.GetTaskPushNotificationConfigRequest.Builder builder = org.a2aproject.sdk.grpc.GetTaskPushNotificationConfigRequest.newBuilder();
235-
parseRequestBody(paramsNode, builder, id);
240+
parseRequestBody(params, builder, id);
236241
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
237242
return new GetTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.getTaskPushNotificationConfigParams(builder));
238243
}
239244
case SEND_MESSAGE_METHOD -> {
240245
org.a2aproject.sdk.grpc.SendMessageRequest.Builder builder = org.a2aproject.sdk.grpc.SendMessageRequest.newBuilder();
241-
parseRequestBody(paramsNode, builder, id);
246+
parseRequestBody(params, builder, id);
242247
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
243248
return new SendMessageRequest(version, id, ProtoUtils.FromProto.messageSendParams(builder));
244249
}
245250
case LIST_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> {
246251
org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.Builder builder = org.a2aproject.sdk.grpc.ListTaskPushNotificationConfigsRequest.newBuilder();
247-
parseRequestBody(paramsNode, builder, id);
252+
parseRequestBody(params, builder, id);
248253
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
249254
return new ListTaskPushNotificationConfigsRequest(version, id, ProtoUtils.FromProto.listTaskPushNotificationConfigsParams(builder));
250255
}
251256
case DELETE_TASK_PUSH_NOTIFICATION_CONFIG_METHOD -> {
252257
org.a2aproject.sdk.grpc.DeleteTaskPushNotificationConfigRequest.Builder builder = org.a2aproject.sdk.grpc.DeleteTaskPushNotificationConfigRequest.newBuilder();
253-
parseRequestBody(paramsNode, builder, id);
258+
parseRequestBody(params, builder, id);
254259
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
255260
return new DeleteTaskPushNotificationConfigRequest(version, id, ProtoUtils.FromProto.deleteTaskPushNotificationConfigParams(builder));
256261
}
@@ -268,13 +273,13 @@ private static A2ARequest<?> parseMethodRequest(String version, Object id, Strin
268273
}
269274
case SEND_STREAMING_MESSAGE_METHOD -> {
270275
org.a2aproject.sdk.grpc.SendMessageRequest.Builder builder = org.a2aproject.sdk.grpc.SendMessageRequest.newBuilder();
271-
parseRequestBody(paramsNode, builder, id);
276+
parseRequestBody(params, builder, id);
272277
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
273278
return new SendStreamingMessageRequest(version, id, ProtoUtils.FromProto.messageSendParams(builder));
274279
}
275280
case SUBSCRIBE_TO_TASK_METHOD -> {
276281
org.a2aproject.sdk.grpc.SubscribeToTaskRequest.Builder builder = org.a2aproject.sdk.grpc.SubscribeToTaskRequest.newBuilder();
277-
parseRequestBody(paramsNode, builder, id);
282+
parseRequestBody(params, builder, id);
278283
setTenantIfAbsent(builder::getTenant, builder::setTenant, tenant);
279284
return new SubscribeToTaskRequest(version, id, ProtoUtils.FromProto.taskIdParams(builder));
280285
}

0 commit comments

Comments
 (0)