fix: guard against null Prompt.arguments() in completion handlers#933
Closed
sainathreddyb wants to merge 1 commit intomodelcontextprotocol:mainfrom
Closed
fix: guard against null Prompt.arguments() in completion handlers#933sainathreddyb wants to merge 1 commit intomodelcontextprotocol:mainfrom
sainathreddyb wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
The 2.0 forward-compat refactor (#972) changed Prompt constructors to stop coercing null arguments to an empty list, but the completion validation code in McpAsyncServer and McpStatelessAsyncServer still called .arguments().stream() without a null guard, causing a NullPointerException when a Prompt with null arguments receives a completion/complete request. Additionally, MIGRATION-2.0.md references Prompt.withDefaults() as the migration path for callers that relied on the old null-to-empty-list coercion, but this factory method was never implemented. This commit: - Adds a null check before .arguments().stream() in both McpAsyncServer and McpStatelessAsyncServer - Adds Prompt.withDefaults(name, description, arguments) factory that coerces null to an empty list, matching the 1.x behaviour
Member
|
Thanks for the proposal to fix this, however the fix here would result in actually executing the handler while the previous behavior was to return an empty response without calling the handler. I addressed this together with a test in #934 |
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
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.
Fixes #932
The 2.0 forward-compat refactor (#972) changed
Promptconstructors to stop coercingnullarguments to an empty list, but the completion validation code inMcpAsyncServerandMcpStatelessAsyncServerstill called.arguments().stream()without a null guard, causing aNullPointerExceptionwhen aPromptwith null arguments receives acompletion/completerequest.Additionally,
MIGRATION-2.0.mdreferencesPrompt.withDefaults()as the migration path, but this factory method was never implemented.This commit:
.arguments().stream()in bothMcpAsyncServerandMcpStatelessAsyncServerPrompt.withDefaults(name, description, arguments)factory that coercesnullto an empty list, matching the 1.x behaviourMotivation and Context
After #972 merged, any MCP server that registers a
Promptwithnullarguments and a completion handler will crash with aNullPointerExceptionwhen acompletion/completerequest arrives. This is the SDK's own internal code failing to handle the null-semantics change it introduced — not a user error.How Has This Been Tested?
completion/completerequest — confirmed the NPE./mvnw clean test)Breaking Changes
None. This is a bugfix that makes the SDK more tolerant.
Types of changes
Checklist
Additional context
The
Prompt.withDefaults()factory was documented inMIGRATION-2.0.mdas the recommended migration path but never implemented. This PR adds it so the migration guide is accurate.