Skip to content

Reject subscriptions/listen without notifications - #1208

Open
subhashpolisetti wants to merge 1 commit into
modelcontextprotocol:mainfrom
subhashpolisetti:fix/subscriptions-listen-requires-notifications
Open

subhashpolisetti wants to merge 1 commit into
modelcontextprotocol:mainfrom
subhashpolisetti:fix/subscriptions-listen-requires-notifications

Conversation

@subhashpolisetti

Copy link
Copy Markdown

Fixes #1196

Motivation and Context

notifications is a required field of SubscriptionsListenRequestParams. When a client sends subscriptions/listen without it, or with "notifications": null, SubscriptionsFeature#handle falls back to SubscriptionFilter.EMPTY, acknowledges the subscription and keeps an idle stream open. SubscriptionsListenRequest.fromJson makes the same substitution, so the missing field can no longer be detected after decoding.

JSONRPCRequest request = new JSONRPCRequest("subscriptions/listen", 1, Map.of("_meta", meta()));
server.handle(McpTransportContext.EMPTY, request).block(); // Streaming response instead of an error

This change checks the raw params before decoding and fails with McpException.invalidParams("params.notifications is required"), the same way CompletionsFeature#handle checks argument, which the CompleteRequest JSON creator also fills in when it is missing. HttpServletMcpTransport already maps -32602 to 400. An explicit empty filter ({}) is still accepted.

How Has This Been Tested?

Added three tests to SubscriptionsFeatureTests:

  • listenWithoutNotificationsIsRejectedAsInvalidParams
  • listenWithNullNotificationsIsRejectedAsInvalidParams
  • listenWithEmptyNotificationsIsAccepted

The first two fail on main (the server returns a streaming response) and pass with this change. The third passes on both and keeps {} valid.

Those tests use GsonMcpJsonMapper, like the rest of the file, so I also added listenWithoutNotificationsIsInvalidParams to HttpServletMcpTransportIntegrationTests. It goes through the servlet transport with the default Jackson mapper, where SubscriptionsListenRequest.fromJson would fill in the missing filter, and checks the 400 / -32602 response. It fails on main (status 200) and passes with this change on both the Jackson 3 and -Pjackson2 builds. It reads the body as a stream, so a regression fails on the status instead of hanging on an open subscription.

./mvnw -pl mcp-core test -Dtest=SubscriptionsFeatureTests
./mvnw -pl mcp-test -am test -Dtest=HttpServletMcpTransportIntegrationTests -Dsurefire.failIfNoSpecifiedTests=false
./mvnw clean verify
./mvnw -pl mcp-test -am -Pjackson2 test

SubscriptionsFeatureTests: 11 run, 0 failures. HttpServletMcpTransportIntegrationTests: 24 run, 0 failures. clean verify: BUILD SUCCESS on all modules (mcp-core 603, mcp-json-jackson3 45, mcp-json-jackson2 45, mcp-test 1250; 0 failures, 0 errors, 1 skipped by an existing assumeTrue in StdioMcpAsyncClientTests). The -Pjackson2 run passes with the same counts.

Breaking Changes

None for the 2025-11-25 API. On the 2026-07-28 server (first shipped in 2.2.0-M1), a listen request without notifications is now rejected with -32602 instead of opening an empty subscription, as the spec and #1196 describe.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

I kept SubscriptionsListenRequest.fromJson lenient and put the check in the feature, as CompletionsFeature does for CompleteRequest, so the record's deserialization is unchanged. Non-object values ("", true, []) were already rejected as -32602 by the existing "Malformed SubscriptionsListenRequest" path; I checked that on the Jackson 3, Jackson 2 and Gson mappers.

Happy to adjust, or close if you'd rather fold this into the other 2.2.0-M2 work.

  • AI-assisted; reviewed by a human who is accountable for it

I used Claude Code to explore the codebase and draft the fix and tests; I reviewed the change and ran the tests myself.

notifications is required in SubscriptionsListenRequestParams. Check the
raw params before decoding, as CompletionsFeature does, and fail with
-32602 instead of opening an empty subscription. An explicit empty
filter is still accepted.

Fixes modelcontextprotocol#1196

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reject subscriptions/listen without notifications

1 participant