refactor!: centralize server validation by kabir · Pull Request #1213 · a2aproject/a2a-java

kabir · 2026-10-12T15:48:00Z

This fixes #1093

BREAKING CHANGE
It changes how integrations interact with the transport/* handlers.
The public user API remains the same

ehsavoie

Medium Severity

AgentCardOperation.java:12-16 — Latent NPE from nullable capabilityFailure
capabilityFailure is @nullable but AgentCardRequestPolicy.validateCapability calls Objects.requireNonNull() on it. Any caller constructing new AgentCardOperation(predicate, null) will NPE at runtime. Only safe today because AgentCardOperation.none() pairs null with an always-true predicate. Add constructor validation or a typed factory.
DefaultRequestHandler.java:1273 — isBlank() vs isNullOrEmpty() inconsistency
validateRequiredId uses id.isBlank() while authorizeTaskAccess uses Assert.isNullOrEmpty() (which accepts whitespace). A whitespace-only ID passes one guard but fails the other — inconsistent behavior within the same class. Unify on one check.
RestHandler_v0_3.java (8+ sites) — Redundant isNullOrEmpty + checkNotNullParam pattern
Since isNullOrEmpty returning false already guarantees non-null, the follow-up checkNotNullParam call is only there for NullAway. Extract a requireNonEmpty helper or use @SuppressWarnings.
JSONRPCHandler.java:729 — Dead TenantNotFoundException catch block
preflight() at line 745-751 already catches TenantNotFoundException and wraps it as InvalidParamsError. resolveExtendedDiscoveryCard returns null, never throws it. Branch is unreachable and misleading.

Low Severity

RestHandler_v0_3.java:228-237 — Negative historyLength returns 500 instead of 422
Integer.parseInt("-1") succeeds with no bounds check — negative historyLength escapes format parsing and causes a 500 InternalError downstream instead of a 422 InvalidParamsError. Add historyLength < 0 → InvalidParamsError_v0_3.
GrpcHandler.java:743,750 — Per-request allocation of policy objects
AgentCardResolver and AgentCardRequestPolicy are allocated on every request via createAgentCardResolver(). JSONRPCHandler and RestHandler create them once in @PostConstruct. Cache in a field or use lazy init.
AgentCardResolver.java:31-32 — Tenant normalized twice
AgentCardRequestPolicy.normalizeTenant() normalizes before calling resolveEffectiveCard(), which internally calls normalizeTenantForResolution() again. Idempotent today, maintenance trap tomorrow.
JSONRPCRequestDecoder_v0_3.java:109-111 — Null body relies on undocumented Gson behavior
Null body rejection relies on JsonParser.parseString(null) throwing rather than an explicit null check. Fragile across Gson version upgrades.
A2AServerRoutesTest.java — Missing blank line before @test
Missing blank line before testEmptyMessageIdDecodeErrorRetainsId — formatting inconsistency with the rest of the file.

I believe I dealt with everything

ehsavoie

Breaking API change: Confirmed. RestHandler.getTask, listTasks, and listTaskPushNotificationConfigurations all changed from typed params (Integer, Boolean, int) to raw String. These are public methods, so any external subclass or caller breaks silently. The commit is labeled feat: instead of feat!: — that's a real oversight.

taskId validation repeated 7× in RestHandler and GrpcHandler canonicalTenant repeated ~10×

ehsavoie · 2026-10-21T16:36:09Z

This is dead code

ehsavoie · 2026-10-21T16:36:21Z

This is dead code

ehsavoie · 2026-10-21T16:36:41Z

This is dead code

ehsavoie · 2026-10-21T16:36:48Z

This is dead code

ehsavoie · 2026-10-21T16:36:55Z

This is dead code

ehsavoie · 2026-10-21T16:40:43Z


    private static final Logger LOGGER = Logger.getLogger(JSONRPCHandler.class.getName());

+    private static final AgentCardOperation STREAMING = new AgentCardOperation(


Duplicated in RestHandler and GrpcHandler

ehsavoie · 2026-10-21T16:41:30Z

+    private static final AgentCardOperation STREAMING = new AgentCardOperation(
+            card -> card.capabilities().streaming(),
+            () -> new UnsupportedOperationError(null, "Streaming is not supported by the agent", null));
+    private static final AgentCardOperation PUSH_NOTIFICATIONS = new AgentCardOperation(


Duplicated in RestHandler and GrpcHandler with PUSH

ehsavoie · 2026-10-21T16:46:33Z

+        } catch (JsonSyntaxException e) {
+            return new DecodeFailure(requestId, methodName, new JSONParseError_v0_3(e.getMessage()));
+        } catch (Throwable t) {
+            return new DecodeFailure(requestId, methodName, new InternalError_v0_3(t.getMessage()));


Should n't this be sanitized ?

ehsavoie · 2026-10-21T17:01:44Z

ehsavoie · 2026-10-21T17:01:46Z

This fixes a2aproject#1093

kabir force-pushed the valiation-unification branch 5 times, most recently from bb224c9 to 6d43ff6 Compare October 20, 2026 12:57

kabir requested a review from ehsavoie October 20, 2026 19:34

ehsavoie previously requested changes Oct 21, 2026

View reviewed changes

kabir force-pushed the valiation-unification branch from 23dd672 to 072e123 Compare October 21, 2026 14:55

kabir requested a review from ehsavoie October 21, 2026 15:20

ehsavoie requested changes Oct 21, 2026

View reviewed changes

ehsavoie reviewed Oct 21, 2026

View reviewed changes

Comment thread server-common/src/main/java/org/a2aproject/sdk/server/AgentCardValidator.java Outdated

ehsavoie Oct 21, 2026

Copy link
Copy Markdown

Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code

ehsavoie reviewed Oct 21, 2026

View reviewed changes

Comment thread server-common/src/main/java/org/a2aproject/sdk/server/AgentCardValidator.java Outdated

ehsavoie Oct 21, 2026

Copy link
Copy Markdown

Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code

kabir changed the title ~~refactor: centralize server validation~~ refactor!: centralize server validation Oct 22, 2026

kabir force-pushed the valiation-unification branch from 072e123 to be20ca2 Compare October 22, 2026 08:23

refactor!: centralize server validation

2f0013f

This fixes a2aproject#1093

kabir force-pushed the valiation-unification branch from be20ca2 to 2f0013f Compare October 23, 2026 13:32

ehsavoie approved these changes Oct 23, 2026

View reviewed changes

kabir merged commit 2e63cf5 into a2aproject:main Oct 23, 2026
14 checks passed


		private static final Logger LOGGER = Logger.getLogger(JSONRPCHandler.class.getName());

		private static final AgentCardOperation STREAMING = new AgentCardOperation(

Conversation

kabir commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehsavoie left a comment

Choose a reason for hiding this comment

Uh oh!

ehsavoie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kabir commented Oct 12, 2026 •

edited

Loading