Skip to content

Commit 09b2893

Browse files
kabirkabir
committed
fix: ListTasks validation and serialization across all transports
 - Add includingDefaultValueFields() to prevent missing fields in JSON responses - Fix parameter validation: enum UNRECOGNIZED, negative timestamps, zeroToNull - REST: accept multiple timestamp/status formats and add validation - JSON-RPC: add "Invalid enum value:" pattern for -32602 error code
1 parent 5938c78 commit 09b2893

File tree

10 files changed

+256
-29
lines changed

10 files changed

+256
-29
lines changed

‎extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,10 @@ public ListTasksResult list(ListTasksParams params) {
362362
private Task transformTask(Task task, int historyLength, boolean includeArtifacts) {
363363
// Limit history if needed (keep most recent N messages)
364364
List<Message> history = task.history();
365-
if (historyLength > 0 && history != null && history.size() > historyLength) {
365+
if (historyLength == 0) {
366+
// When historyLength is 0, return empty history
367+
history = List.of();
368+
} else if (historyLength > 0 && history != null && history.size() > historyLength) {
366369
history = history.subList(history.size() - historyLength, history.size());
367370
}
368371

‎server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,10 @@ public ListTasksResult list(ListTasksParams params) {
144144
private Task transformTask(Task task, int historyLength, boolean includeArtifacts) {
145145
// Limit history if needed (keep most recent N messages)
146146
List<Message> history = task.history();
147-
if (historyLength > 0 && history != null && history.size() > historyLength) {
147+
if (historyLength == 0) {
148+
// When historyLength is 0, return empty history
149+
history = List.of();
150+
} else if (historyLength > 0 && history != null && history.size() > historyLength) {
148151
history = history.subList(history.size() - historyLength, history.size());
149152
}
150153

‎spec-grpc/javac.20260115_142908.args‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@/private/var/folders/sb/_l7q4kx90j7g25290tyqhhqw0000gn/T/org.codehaus.plexus.compiler.javac.JavacCompiler15111804077731140562arguments

‎spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ default Map<String, Object> metadataFromProto(Struct struct) {
285285
*/
286286
@Named("zeroToNull")
287287
default Integer zeroToNull(int value) {
288-
return value > 0 ? value : null;
288+
return value != 0 ? value : null;
289289
}
290290

291291
/**
@@ -337,35 +337,49 @@ default long instantToMillis(Instant instant) {
337337
* Converts protobuf milliseconds-since-epoch (int64) to domain Instant.
338338
* <p>
339339
* Returns null if input is 0 (protobuf default for unset field).
340+
* Throws InvalidParamsError for negative values (invalid timestamps).
340341
* Use this with {@code @Mapping(qualifiedByName = "millisToInstant")}.
341342
*
342343
* @param millis milliseconds since epoch
343344
* @return domain Instant, or null if millis is 0
345+
* @throws InvalidParamsError if millis is negative
344346
*/
345347
@Named("millisToInstant")
346348
default Instant millisToInstant(long millis) {
349+
if (millis < 0L) {
350+
throw new InvalidParamsError(null,
351+
"lastUpdatedAfter must be a valid timestamp (non-negative milliseconds since epoch), got: " + millis,
352+
null);
353+
}
347354
return millis > 0L ? Instant.ofEpochMilli(millis) : null;
348355
}
349356

350357
// ========================================================================
351358
// Enum Conversions (handling UNSPECIFIED/UNKNOWN)
352359
// ========================================================================
353360
/**
354-
* Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED/UNKNOWN as null.
361+
* Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED as null.
355362
* <p>
356-
* Protobuf enums default to UNSPECIFIED (0 value) when unset. The domain may also have
357-
* UNKNOWN for unparseable values. Both should map to null for optional fields.
363+
* Protobuf enums default to UNSPECIFIED (0 value) when unset, which maps to null for optional fields.
364+
* However, UNRECOGNIZED (invalid enum values from JSON) throws InvalidParamsError for proper validation.
358365
* Use this with {@code @Mapping(qualifiedByName = "taskStateOrNull")}.
359366
*
360367
* @param state the protobuf TaskState
361-
* @return domain TaskState or null if UNSPECIFIED/UNKNOWN
368+
* @return domain TaskState or null if UNSPECIFIED
369+
* @throws InvalidParamsError if state is UNRECOGNIZED (invalid enum value)
362370
*/
363371
@Named("taskStateOrNull")
364372
default io.a2a.spec.TaskState taskStateOrNull(io.a2a.grpc.TaskState state) {
365373
if (state == null || state == io.a2a.grpc.TaskState.TASK_STATE_UNSPECIFIED) {
366374
return null;
367375
}
376+
// Reject invalid enum values (e.g., "INVALID_STATUS" from JSON)
377+
if (state == io.a2a.grpc.TaskState.UNRECOGNIZED) {
378+
throw new InvalidParamsError(null,
379+
"Invalid task state value. Must be one of: SUBMITTED, WORKING, INPUT_REQUIRED, AUTH_REQUIRED, COMPLETED, CANCELED, FAILED, REJECTED",
380+
null);
381+
}
368382
io.a2a.spec.TaskState result = TaskStateMapper.INSTANCE.fromProto(state);
369-
return result == io.a2a.spec.TaskState.UNKNOWN ? null : result;
383+
return result;
370384
}
371385
}

‎spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,10 @@ private static JsonProcessingException convertProtoBufExceptionToJsonProcessingE
467467
if (message.contains(prefix)) {
468468
return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id);
469469
}
470+
prefix = "Invalid enum value:";
471+
if (message.contains(prefix)) {
472+
return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id);
473+
}
470474

471475
// Try to extract specific error details using regex patterns
472476
Matcher matcher = EXTRACT_WRONG_TYPE.matcher(message);
@@ -548,7 +552,7 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method,
548552
output.name("method").value(method);
549553
}
550554
if (payload != null) {
551-
String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(payload);
555+
String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(payload);
552556
output.name("params").jsonValue(resultValue);
553557
}
554558
output.endObject();
@@ -571,7 +575,7 @@ public static String toJsonRPCResultResponse(Object requestId, com.google.protob
571575
output.name("id").value(number.longValue());
572576
}
573577
}
574-
String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(builder);
578+
String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(builder);
575579
output.name("result").jsonValue(resultValue);
576580
output.endObject();
577581
return result.toString();

‎spec/src/main/java/io/a2a/spec/ListTasksParams.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public record ListTasksParams(
2929
) {
3030
/**
3131
* Compact constructor for validation.
32-
* Validates that the tenant parameter is not null.
32+
* Validates that the tenant parameter is not null and parameters are within valid ranges.
3333
*
3434
* @param contextId filter by context ID
3535
* @param status filter by task status
@@ -39,9 +39,22 @@ public record ListTasksParams(
3939
* @param lastUpdatedAfter filter by last update timestamp
4040
* @param includeArtifacts whether to include artifacts
4141
* @param tenant the tenant identifier
42+
* @throws InvalidParamsError if pageSize or historyLength are out of valid range
4243
*/
4344
public ListTasksParams {
4445
Assert.checkNotNullParam("tenant", tenant);
46+
47+
// Validate pageSize (1-100)
48+
if (pageSize != null && (pageSize < 1 || pageSize > 100)) {
49+
throw new InvalidParamsError(null,
50+
"pageSize must be between 1 and 100, got: " + pageSize, null);
51+
}
52+
53+
// Validate historyLength (>= 0)
54+
if (historyLength != null && historyLength < 0) {
55+
throw new InvalidParamsError(null,
56+
"historyLength must be non-negative, got: " + historyLength, null);
57+
}
4558
}
4659
/**
4760
* Default constructor for listing all tasks.

‎transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import io.a2a.grpc.GetTaskRequest;
2222
import io.a2a.grpc.ListTaskPushNotificationConfigRequest;
2323
import io.a2a.grpc.ListTaskPushNotificationConfigResponse;
24+
import io.a2a.grpc.ListTasksRequest;
25+
import io.a2a.grpc.ListTasksResponse;
2426
import io.a2a.grpc.Message;
2527
import io.a2a.grpc.Part;
2628
import io.a2a.grpc.PushNotificationConfig;
@@ -1209,6 +1211,53 @@ private <V> void assertGrpcError(StreamRecorder<V> streamRecorder, Status.Code e
12091211
Assertions.assertTrue(streamRecorder.getValues().isEmpty());
12101212
}
12111213

1214+
@Test
1215+
public void testListTasksNegativeTimestampReturnsInvalidArgument() {
1216+
TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor);
1217+
StreamRecorder<ListTasksResponse> responseObserver = StreamRecorder.create();
1218+
1219+
// Negative timestamp should trigger validation error
1220+
ListTasksRequest request = ListTasksRequest.newBuilder()
1221+
.setLastUpdatedAfter(-1L)
1222+
.setTenant("")
1223+
.build();
1224+
1225+
handler.listTasks(request, responseObserver);
1226+
1227+
// Should return error with INVALID_ARGUMENT status
1228+
Assertions.assertTrue(responseObserver.getError() != null);
1229+
StatusRuntimeException error = (StatusRuntimeException) responseObserver.getError();
1230+
assertEquals(Status.INVALID_ARGUMENT.getCode(), error.getStatus().getCode());
1231+
}
1232+
1233+
@Test
1234+
public void testListTasksEmptyResultIncludesAllFields() throws Exception {
1235+
TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor);
1236+
StreamRecorder<ListTasksResponse> responseObserver = StreamRecorder.create();
1237+
1238+
// Query for a context that doesn't exist - should return empty result
1239+
ListTasksRequest request = ListTasksRequest.newBuilder()
1240+
.setContextId("nonexistent-context-id")
1241+
.setTenant("")
1242+
.build();
1243+
1244+
handler.listTasks(request, responseObserver);
1245+
1246+
// Should succeed with empty result
1247+
Assertions.assertNull(responseObserver.getError());
1248+
List<ListTasksResponse> responses = responseObserver.getValues();
1249+
assertEquals(1, responses.size());
1250+
1251+
ListTasksResponse response = responses.get(0);
1252+
// Verify all fields are present (even if empty/default)
1253+
Assertions.assertNotNull(response.getTasksList(), "tasks field should not be null");
1254+
assertEquals(0, response.getTasksList().size(), "tasks should be empty list");
1255+
assertEquals(0, response.getTotalSize(), "totalSize should be 0");
1256+
assertEquals(0, response.getPageSize(), "pageSize should be 0");
1257+
// nextPageToken will be empty string for empty results (protobuf default)
1258+
Assertions.assertNotNull(response.getNextPageToken());
1259+
}
1260+
12121261
private static class TestGrpcHandler extends GrpcHandler {
12131262
private final AgentCard card;
12141263
private final RequestHandler handler;

‎transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,15 @@
3030
import io.a2a.jsonrpc.common.wrappers.GetTaskResponse;
3131
import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigRequest;
3232
import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigResponse;
33+
import io.a2a.jsonrpc.common.wrappers.ListTasksResult;
3334
import io.a2a.jsonrpc.common.wrappers.SendMessageRequest;
3435
import io.a2a.jsonrpc.common.wrappers.SendMessageResponse;
3536
import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageRequest;
3637
import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageResponse;
3738
import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigRequest;
3839
import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigResponse;
40+
import io.a2a.jsonrpc.common.wrappers.ListTasksRequest;
41+
import io.a2a.jsonrpc.common.wrappers.ListTasksResponse;
3942
import io.a2a.jsonrpc.common.wrappers.SubscribeToTaskRequest;
4043
import io.a2a.server.ServerCallContext;
4144
import io.a2a.server.auth.UnauthenticatedUser;
@@ -56,7 +59,9 @@
5659
import io.a2a.spec.Event;
5760
import io.a2a.spec.GetTaskPushNotificationConfigParams;
5861
import io.a2a.spec.InternalError;
62+
import io.a2a.spec.InvalidParamsError;
5963
import io.a2a.spec.InvalidRequestError;
64+
import io.a2a.spec.ListTasksParams;
6065
import io.a2a.spec.ListTaskPushNotificationConfigParams;
6166
import io.a2a.spec.Message;
6267
import io.a2a.spec.MessageSendParams;
@@ -1959,4 +1964,27 @@ public void testNoVersionDefaultsToCurrentVersionSuccess() {
19591964
assertNull(response.getError());
19601965
Assertions.assertSame(message, response.getResult());
19611966
}
1967+
1968+
@Test
1969+
public void testListTasksEmptyResultIncludesAllFields() {
1970+
JSONRPCHandler handler = new JSONRPCHandler(CARD, requestHandler, internalExecutor);
1971+
1972+
// Query for a context that doesn't exist - should return empty result
1973+
ListTasksParams params = ListTasksParams.builder()
1974+
.contextId("nonexistent-context-id")
1975+
.tenant("")
1976+
.build();
1977+
ListTasksRequest request = new ListTasksRequest("1", params);
1978+
ListTasksResponse response = handler.onListTasks(request, callContext);
1979+
1980+
// Should return success with all fields present (not null)
1981+
assertNull(response.getError());
1982+
ListTasksResult result = response.getResult();
1983+
Assertions.assertNotNull(result);
1984+
Assertions.assertNotNull(result.tasks(), "tasks field should not be null");
1985+
assertEquals(0, result.tasks().size(), "tasks should be empty list");
1986+
assertEquals(0, result.totalSize(), "totalSize should be 0");
1987+
assertEquals(0, result.pageSize(), "pageSize should be 0");
1988+
// nextPageToken can be null for empty results
1989+
}
19621990
}

‎transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java‎

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -219,21 +219,41 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s
219219
paramsBuilder.contextId(contextId);
220220
}
221221
if (status != null) {
222+
TaskState taskState = null;
223+
224+
// Try JSON format first (e.g., "working", "completed")
222225
try {
223-
paramsBuilder.status(TaskState.fromString(status));
226+
taskState = TaskState.fromString(status);
224227
} catch (IllegalArgumentException e) {
225-
try {
226-
paramsBuilder.status(TaskState.valueOf(status));
227-
} catch (IllegalArgumentException valueOfError) {
228-
String validStates = Arrays.stream(TaskState.values())
229-
.map(TaskState::asString)
230-
.collect(Collectors.joining(", "));
231-
Map<String, Object> errorData = new HashMap<>();
232-
errorData.put("parameter", "status");
233-
errorData.put("reason", "Must be one of: " + validStates);
234-
throw new InvalidParamsError(null, "Invalid params", errorData);
228+
// Try protobuf enum format (e.g., "TASK_STATE_WORKING")
229+
if (status.startsWith("TASK_STATE_")) {
230+
String enumName = status.substring("TASK_STATE_".length());
231+
try {
232+
taskState = TaskState.valueOf(enumName);
233+
} catch (IllegalArgumentException protoError) {
234+
// Fall through to error handling below
235+
}
236+
} else {
237+
// Try enum constant name directly (e.g., "WORKING")
238+
try {
239+
taskState = TaskState.valueOf(status);
240+
} catch (IllegalArgumentException valueOfError) {
241+
// Fall through to error handling below
242+
}
235243
}
236244
}
245+
246+
if (taskState == null) {
247+
String validStates = Arrays.stream(TaskState.values())
248+
.map(TaskState::asString)
249+
.collect(Collectors.joining(", "));
250+
Map<String, Object> errorData = new HashMap<>();
251+
errorData.put("parameter", "status");
252+
errorData.put("reason", "Must be one of: " + validStates);
253+
throw new InvalidParamsError(null, "Invalid params", errorData);
254+
}
255+
256+
paramsBuilder.status(taskState);
237257
}
238258
if (pageSize != null) {
239259
paramsBuilder.pageSize(pageSize);
@@ -247,12 +267,25 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s
247267
paramsBuilder.tenant(tenant);
248268
if (lastUpdatedAfter != null) {
249269
try {
250-
paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter));
251-
} catch (DateTimeParseException e) {
252-
Map<String, Object> errorData = new HashMap<>();
253-
errorData.put("parameter", "lastUpdatedAfter");
254-
errorData.put("reason", "Must be valid ISO-8601 timestamp");
255-
throw new InvalidParamsError(null, "Invalid params", errorData);
270+
// Try parsing as Unix milliseconds first (integer)
271+
long millis = Long.parseLong(lastUpdatedAfter);
272+
if (millis < 0L) {
273+
Map<String, Object> errorData = new HashMap<>();
274+
errorData.put("parameter", "lastUpdatedAfter");
275+
errorData.put("reason", "Must be a non-negative timestamp value, got: " + millis);
276+
throw new InvalidParamsError(null, "Invalid params", errorData);
277+
}
278+
paramsBuilder.lastUpdatedAfter(Instant.ofEpochMilli(millis));
279+
} catch (NumberFormatException nfe) {
280+
// Fall back to ISO-8601 format
281+
try {
282+
paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter));
283+
} catch (DateTimeParseException e) {
284+
Map<String, Object> errorData = new HashMap<>();
285+
errorData.put("parameter", "lastUpdatedAfter");
286+
errorData.put("reason", "Must be valid Unix milliseconds or ISO-8601 timestamp");
287+
throw new InvalidParamsError(null, "Invalid params", errorData);
288+
}
256289
}
257290
}
258291
if (includeArtifacts != null) {
@@ -338,7 +371,8 @@ private void validate(String json) {
338371

339372
private HTTPRestResponse createSuccessResponse(int statusCode, com.google.protobuf.Message.Builder builder) {
340373
try {
341-
String jsonBody = JsonFormat.printer().print(builder);
374+
// Include default value fields to ensure empty arrays, zeros, etc. are present in JSON
375+
String jsonBody = JsonFormat.printer().includingDefaultValueFields().print(builder);
342376
return new HTTPRestResponse(statusCode, "application/json", jsonBody);
343377
} catch (InvalidProtocolBufferException e) {
344378
return createErrorResponse(new InternalError("Failed to serialize response: " + e.getMessage()));

0 commit comments

Comments
 (0)