{{ message }}
Repository navigation
fix: guard limitTaskHistory against negative historyLength - #1039
Merged
Merged
Conversation
kabir
requested changes
Aug 11, 2026
kabir
left a comment
kabir
left a comment
Collaborator
There was a problem hiding this comment.
Hi @ez-lbz thank you for the PR.
I've looked and I don't think this is a viable scenario. Essentially, TaskQueryParams has a guard against negative history lengths. And this class is used both to send from the client, as well as used when deserializing requests on the server. So it seems its check will kick in before limitTaskHistory can ever be called.
Let me know if you disagree - there are quite a few PRs to go through today :-)
Contributor
Author
1 of 2 tasks
ehsavoie
pushed a commit
that referenced
this pull request
Aug 11, 2026
- Fix import ordering of TaskQueryParams in DefaultRequestHandlerTest - Remove fragile reflection-based test for negative historyLength guard (the two public-API tests already exercise the limitTaskHistory path) - Trim verbose 4-line comment on negative historyLength guard to one line - Fix Javadoc on bufferFlushDelayMs(): negative values are clamped to 0, not returned as the default - Replace fully-qualified class names with imports in EventQueueTest Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
1. Guard
limitTaskHistoryagainst negativehistoryLengthProblem: The guard in
limitTaskHistoryonly checkedhistoryLength >= task.history().size(), which does not catch negative values. A negativehistoryLengthfell through tosubList(size - historyLength, size)— i.e.subList(size + 1, size)— throwingIndexOutOfBoundsException. WhileTaskQueryParamsrejects negative values at construction today,limitTaskHistorylacked defensive validation and would crash if a negative value ever reached it.Fix (server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java):
historyLength < 0, so a negative value returns the task untouched instead of crashing.apply_history_length): negative → history untouched;0→ empty history;N >= history size→ no limiting. The existing0 → empty historybehavior is preserved (unlike a<= 0guard, which would have broken it).Fix (server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java):
testLimitTaskHistoryNegativeHistoryLengthReturnsTaskUnchanged— invokes the privatelimitTaskHistoryvia reflection with-1and asserts no exception is thrown and the task is returned unchanged.testOnGetTaskHistoryLengthLimitsHistory—historyLength=2returns only the 2 most recent messages.testOnGetTaskHistoryLengthZeroReturnsEmptyHistory— locks in thathistoryLength=0still returns an empty history.Behavior change: none for valid inputs (
>= 0). NegativehistoryLengthpreviously crashed; it now returns the task unchanged.Testing
mvn -pl server-common test— 450 tests run, 0 failures, 0 errors, 0 skipped (BUILD SUCCESS), including the 3 new regression tests.