Skip to content

Bound no-arg list operations to prevent unbounded pagination - #1100

Open
ZYZ666-RGB wants to merge 1 commit into
modelcontextprotocol:mainfrom
ZYZ666-RGB:fix/1084-bounded-pagination
Open

Bound no-arg list operations to prevent unbounded pagination#1100
ZYZ666-RGB wants to merge 1 commit into
modelcontextprotocol:mainfrom
ZYZ666-RGB:fix/1084-bounded-pagination

Conversation

@ZYZ666-RGB

@ZYZ666-RGB ZYZ666-RGB commented Aug 18, 2026

Copy link
Copy Markdown

Motivation and Context

The no-arg list operations (listTools(), listResources(), listResourceTemplates(),
listPrompts()) follow the server-provided nextCursor chain via Mono.expand with
no page limit, no duplicate-cursor detection and no total deadline. A server that
returns an endless stream of non-empty cursors makes the client:

  • issue an unbounded number of requests (request storm),
  • accumulate unbounded memory (heap exhaustion),
  • block synchronous callers forever (McpSyncClient.listTools() has no duration).

requestTimeout does not help: it bounds each individual request, not the number of
requests. The existing #954 fix only stops servers that signal the end with an empty
cursor. Fixes #1084.

Changes

Add a client-side pagination guard applied to all four no-arg list operations:

  • Page-count limit: maxPaginationPages (builder-configurable, default 100)
    caps the total number of pages fetched. 0 disables the limit.
  • Duplicate-cursor detection: if the server returns a cursor it already returned,
    the operation aborts immediately — an unambiguous sign of a loop.
  • Total deadline: paginationTimeout (builder-configurable, disabled by default)
    bounds the wall-clock time of the whole list operation.

When a bound is exceeded the operation fails with a new
McpPaginationException carrying a clear message. Configuration is exposed on both
McpClient.SyncSpec and McpClient.AsyncSpec (maxPaginationPages(int),
paginationTimeout(Duration)). No existing API or behavior changes for servers that
terminate pagination normally — the guard state is created per subscription, so shared
Monos can be subscribed multiple times safely.

How Has This Been Tested?

New McpAsyncClientPaginationTests (6 tests):

  • normal multi-page aggregation ends at a null cursor (tools + resources),
  • duplicate cursor loop → McpPaginationException,
  • ever-changing cursor loop → McpPaginationException after maxPaginationPages pages,
  • ever-changing cursor loop → McpPaginationException after paginationTimeout,
  • synchronous client (McpSyncClient.listTools()) propagates the same exception.

Verified locally: mvn -pl mcp-core test (360 tests) and the mcp-test client test
classes (17 tests) all pass, with spring-javaformat validation green.

Breaking Changes

None. The new builder options are additive; defaults keep the API source-compatible and
only add protection against misbehaving servers.

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

Design note: Mono.expand runs per subscription, so a shared list Mono subscribed
twice would otherwise share stale guard state; the guard is therefore created inside
Mono.defer per subscription.

The no-arg listTools(), listResources(), listResourceTemplates() and listPrompts() follow the server-provided nextCursor chain via Mono.expand with no page limit, no duplicate-cursor detection and no total deadline. A server that returns an endless stream of non-empty cursors makes the client issue an unbounded number of requests, accumulate unbounded memory, and block synchronous callers forever.

Add a client-side pagination guard: each no-arg list operation now tracks the cursors it has already seen and the number of pages fetched, and aborts with a new McpPaginationException when the configured maxPaginationPages (default 100) or paginationTimeout (disabled by default) is exceeded, or when a cursor is returned more than once.

The bounds are configurable on both McpClient.SyncSpec and McpClient.AsyncSpec (maxPaginationPages(int), paginationTimeout(Duration)); existing behavior and APIs are unchanged for servers that terminate pagination normally.

Adds McpAsyncClientPaginationTests covering normal multi-page aggregation, duplicate-cursor loops, ever-changing cursor loops (page limit and total timeout) and the synchronous client path.

Fixes modelcontextprotocol#1084

Signed-off-by: zhaoyuzhe <1991039819@qq.com>
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.

listTools() and the other no-arg list*() methods follow the cursor chain without any bound

1 participant