From 6ebcbebcc4d9376718d4497bea2c4064a774f94d Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Tue, 23 Jun 2026 12:04:17 +0200 Subject: [PATCH 1/2] Bound HTTP server reads Signed-off-by: Daniel Garnier-Moiroux --- .../HttpClientStreamableHttpTransport.java | 4 + .../transport/HttpServletRequestUtils.java | 32 ++++ ...HttpServletSseServerTransportProvider.java | 51 +++++-- .../HttpServletStatelessServerTransport.java | 61 ++++++-- ...vletStreamableServerTransportProvider.java | 62 +++++--- .../transport/MaxSizeExceededException.java | 18 +++ .../HttpServletRequestUtilsTests.java | 141 ++++++++++++++++++ ...stractMcpClientServerIntegrationTests.java | 45 ++++++ .../HttpServletSseIntegrationTests.java | 82 ++++++++++ .../HttpServletStatelessIntegrationTests.java | 113 +++++++++++++- ...HttpServletStreamableIntegrationTests.java | 48 ++++++ 11 files changed, 619 insertions(+), 38 deletions(-) create mode 100644 mcp-core/src/main/java/io/modelcontextprotocol/server/transport/MaxSizeExceededException.java create mode 100644 mcp-core/src/test/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtilsTests.java diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java index d8cbe2f0b..ba9c6e505 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java @@ -681,6 +681,10 @@ else if (statusCode == BAD_REQUEST) { return Flux.error(new McpTransportException( "Bad Request. Status code:" + statusCode + ", response-event:" + responseEvent)); } + else if (statusCode >= 400 && statusCode < 500) { + return Flux.error( + new McpTransportException("Invalid request. Status code: " + statusCode)); + } return Flux.error( new RuntimeException("Failed to send message: " + responseEvent)); diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtils.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtils.java index 32246948c..721bdf195 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtils.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtils.java @@ -4,6 +4,10 @@ package io.modelcontextprotocol.server.transport; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; @@ -37,4 +41,32 @@ static Map> extractHeaders(HttpServletRequest request) { return headers; } + /** + * Reads the request body, decoded using the request's character encoding (or UTF-8 if + * not specified), while bounding the number of bytes read. + * @param request The HTTP servlet request + * @param maxSize The maximum number of bytes to read from the request body + * @return The decoded request body + * @throws MaxSizeExceededException If the body exceeds {@code maxSize} + * @throws IOException If an I/O error occurs while reading the request body + */ + static String readBody(HttpServletRequest request, int maxSize) throws MaxSizeExceededException, IOException { + InputStream inputStream = request.getInputStream(); + ByteArrayOutputStream bodyBytes = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + int totalBytes = 0; + int readBytes; + while ((readBytes = inputStream.read(buf, 0, buf.length)) != -1) { + totalBytes += readBytes; + if (totalBytes > maxSize) { + throw new MaxSizeExceededException( + "Request body exceeds the maximum allowed size of " + maxSize + " bytes"); + } + bodyBytes.write(buf, 0, readBytes); + } + String charset = request.getCharacterEncoding() != null ? request.getCharacterEncoding() + : StandardCharsets.UTF_8.name(); + return bodyBytes.toString(charset); + } + } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java index 69d73f7ab..05dd862e9 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java @@ -4,7 +4,6 @@ package io.modelcontextprotocol.server.transport; -import java.io.BufferedReader; import java.io.IOException; import java.io.PrintWriter; import java.time.Duration; @@ -76,6 +75,11 @@ @WebServlet(asyncSupported = true) public class HttpServletSseServerTransportProvider extends HttpServlet implements McpServerTransportProvider { + /** + * Default maximum size of a single request body: 16 MiB (16 * 1024 * 1024 bytes). + */ + private static final int DEFAULT_REQUEST_MAX_SIZE = 16 * 1024 * 1024; + /** * Logger for this class */ @@ -111,6 +115,11 @@ public class HttpServletSseServerTransportProvider extends HttpServlet implement */ private final McpJsonMapper jsonMapper; + /** + * Maximum size, in bytes, of a single request body accepted by this transport. + */ + private final int requestMaxSize; + /** * Base URL for the server transport */ @@ -166,17 +175,20 @@ public class HttpServletSseServerTransportProvider extends HttpServlet implement * keep-alive functionality * @param contextExtractor The extractor for transport context from the request. * @param securityValidator The security validator for validating HTTP requests. + * @param requestMaxSize The maximum size, in bytes, of a single request body. Must be + * positive. */ private HttpServletSseServerTransportProvider(McpJsonMapper jsonMapper, String baseUrl, String messageEndpoint, String sseEndpoint, Duration keepAliveInterval, McpTransportContextExtractor contextExtractor, - ServerTransportSecurityValidator securityValidator) { + ServerTransportSecurityValidator securityValidator, int requestMaxSize) { Assert.notNull(jsonMapper, "JsonMapper must not be null"); Assert.notNull(messageEndpoint, "messageEndpoint must not be null"); Assert.notNull(sseEndpoint, "sseEndpoint must not be null"); Assert.notNull(contextExtractor, "Context extractor must not be null"); Assert.notNull(securityValidator, "Security validator must not be null"); + Assert.isTrue(requestMaxSize > 0, "requestMaxSize must be positive"); this.jsonMapper = jsonMapper; this.baseUrl = baseUrl; @@ -184,6 +196,7 @@ private HttpServletSseServerTransportProvider(McpJsonMapper jsonMapper, String b this.sseEndpoint = sseEndpoint; this.contextExtractor = contextExtractor; this.securityValidator = securityValidator; + this.requestMaxSize = requestMaxSize; if (keepAliveInterval != null) { @@ -346,6 +359,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) return; } + if (request.getContentLengthLong() > this.requestMaxSize) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + return; + } + String requestURI = request.getRequestURI(); if (!requestURI.endsWith(messageEndpoint)) { response.sendError(HttpServletResponse.SC_NOT_FOUND); @@ -392,15 +410,10 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) } try { - BufferedReader reader = request.getReader(); - StringBuilder body = new StringBuilder(); - String line; - while ((line = reader.readLine()) != null) { - body.append(line); - } + String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize); final McpTransportContext transportContext = this.contextExtractor.extract(request); - McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body.toString()); + McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body); // Process the message through the session's handle method // Block for Servlet compatibility @@ -408,6 +421,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) response.setStatus(HttpServletResponse.SC_OK); } + catch (MaxSizeExceededException e) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } catch (Exception e) { logger.error("Error processing message: {}", e.getMessage()); try { @@ -605,6 +621,8 @@ public static class Builder { private ServerTransportSecurityValidator securityValidator = ServerTransportSecurityValidator.NOOP; + private int requestMaxSize = DEFAULT_REQUEST_MAX_SIZE; + /** * Sets the JsonMapper implementation to use for serialization/deserialization. If * not specified, a JacksonJsonMapper will be created from the configured @@ -691,6 +709,19 @@ public Builder securityValidator(ServerTransportSecurityValidator securityValida return this; } + /** + * Sets the maximum size, in bytes, of a single request body accepted by this + * transport. Requests whose body exceeds this size are rejected with a 413 + * (Payload Too Large) response. Defaults to 16 MiB if not set. + * @param requestMaxSize The maximum request body size, in bytes. Must be + * positive. + * @return This builder instance + */ + public Builder maxRequestSize(int requestMaxSize) { + this.requestMaxSize = requestMaxSize; + return this; + } + /** * Builds a new instance of HttpServletSseServerTransportProvider with the * configured settings. @@ -703,7 +734,7 @@ public HttpServletSseServerTransportProvider build() { } return new HttpServletSseServerTransportProvider( jsonMapper == null ? McpJsonDefaults.getMapper() : jsonMapper, baseUrl, messageEndpoint, - sseEndpoint, keepAliveInterval, contextExtractor, securityValidator); + sseEndpoint, keepAliveInterval, contextExtractor, securityValidator, requestMaxSize); } } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java index 047aeebe8..54f0ac030 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java @@ -4,7 +4,6 @@ package io.modelcontextprotocol.server.transport; -import java.io.BufferedReader; import java.io.IOException; import java.io.PrintWriter; import java.util.List; @@ -39,6 +38,11 @@ @WebServlet(asyncSupported = true) public class HttpServletStatelessServerTransport extends HttpServlet implements McpStatelessServerTransport { + /** + * Default maximum size of a single request body: 16 MiB (16 * 1024 * 1024 bytes). + */ + private static final int DEFAULT_REQUEST_MAX_SIZE = 16 * 1024 * 1024; + private static final Logger logger = LoggerFactory.getLogger(HttpServletStatelessServerTransport.class); public static final String UTF_8 = "UTF-8"; @@ -66,18 +70,37 @@ public class HttpServletStatelessServerTransport extends HttpServlet implements */ private final ServerTransportSecurityValidator securityValidator; + /** + * Maximum size, in bytes, of a single request body accepted by this transport. + */ + private final int requestMaxSize; + + /** + * Constructs a new HttpServletStatelessServerTransport instance. + * @param jsonMapper The JsonMapper to use for JSON serialization/deserialization of + * messages. + * @param mcpEndpoint The endpoint URI where clients should send their JSON-RPC + * messages. + * @param contextExtractor The extractor for transport context from the request. + * @param securityValidator The security validator for validating HTTP requests. + * @param requestMaxSize The maximum size, in bytes, of a single request body. Must be + * positive. + * @throws IllegalArgumentException if any parameter is null + */ private HttpServletStatelessServerTransport(McpJsonMapper jsonMapper, String mcpEndpoint, McpTransportContextExtractor contextExtractor, - ServerTransportSecurityValidator securityValidator) { + ServerTransportSecurityValidator securityValidator, int requestMaxSize) { Assert.notNull(jsonMapper, "jsonMapper must not be null"); Assert.notNull(mcpEndpoint, "mcpEndpoint must not be null"); Assert.notNull(contextExtractor, "contextExtractor must not be null"); Assert.notNull(securityValidator, "Security validator must not be null"); + Assert.isTrue(requestMaxSize > 0, "requestMaxSize must be positive"); this.jsonMapper = jsonMapper; this.mcpEndpoint = mcpEndpoint; this.contextExtractor = contextExtractor; this.securityValidator = securityValidator; + this.requestMaxSize = requestMaxSize; } @Override @@ -133,6 +156,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) return; } + if (request.getContentLengthLong() > this.requestMaxSize) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + return; + } + try { Map> headers = HttpServletRequestUtils.extractHeaders(request); this.securityValidator.validateHeaders(headers); @@ -154,14 +182,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) } try { - BufferedReader reader = request.getReader(); - StringBuilder body = new StringBuilder(); - String line; - while ((line = reader.readLine()) != null) { - body.append(line); - } + String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize); - McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body.toString()); + McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body); if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { try { @@ -209,6 +232,9 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { .build()); } } + catch (MaxSizeExceededException e) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, @@ -276,6 +302,8 @@ public static class Builder { private ServerTransportSecurityValidator securityValidator = ServerTransportSecurityValidator.NOOP; + private int requestMaxSize = DEFAULT_REQUEST_MAX_SIZE; + private Builder() { // used by a static method } @@ -333,6 +361,19 @@ public Builder securityValidator(ServerTransportSecurityValidator securityValida return this; } + /** + * Sets the maximum size, in bytes, of a single request body accepted by this + * transport. Requests whose body exceeds this size are rejected with a 413 + * (Payload Too Large) response. Defaults to 16 MiB if not set. + * @param requestMaxSize The maximum request body size, in bytes. Must be + * positive. + * @return this builder instance + */ + public Builder maxRequestSize(int requestMaxSize) { + this.requestMaxSize = requestMaxSize; + return this; + } + /** * Builds a new instance of {@link HttpServletStatelessServerTransport} with the * configured settings. @@ -343,7 +384,7 @@ public HttpServletStatelessServerTransport build() { Assert.notNull(mcpEndpoint, "Message endpoint must be set"); return new HttpServletStatelessServerTransport( jsonMapper == null ? McpJsonDefaults.getMapper() : jsonMapper, mcpEndpoint, contextExtractor, - securityValidator); + securityValidator, requestMaxSize); } } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java index e6af4fd0f..324a2ecd3 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java @@ -4,7 +4,6 @@ package io.modelcontextprotocol.server.transport; -import java.io.BufferedReader; import java.io.IOException; import java.io.PrintWriter; import java.time.Duration; @@ -14,12 +13,10 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import io.modelcontextprotocol.json.TypeRef; - import io.modelcontextprotocol.common.McpTransportContext; +import io.modelcontextprotocol.json.McpJsonDefaults; +import io.modelcontextprotocol.json.McpJsonMapper; +import io.modelcontextprotocol.json.TypeRef; import io.modelcontextprotocol.server.McpTransportContextExtractor; import io.modelcontextprotocol.spec.HttpHeaders; import io.modelcontextprotocol.spec.McpError; @@ -28,8 +25,6 @@ import io.modelcontextprotocol.spec.McpStreamableServerTransport; import io.modelcontextprotocol.spec.McpStreamableServerTransportProvider; import io.modelcontextprotocol.util.Assert; -import io.modelcontextprotocol.json.McpJsonDefaults; -import io.modelcontextprotocol.json.McpJsonMapper; import io.modelcontextprotocol.util.KeepAliveScheduler; import jakarta.servlet.AsyncContext; import jakarta.servlet.ServletException; @@ -37,6 +32,8 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -62,6 +59,11 @@ public class HttpServletStreamableServerTransportProvider extends HttpServlet implements McpStreamableServerTransportProvider { + /** + * Default maximum size of a single request body: 16 MiB (16 * 1024 * 1024 bytes). + */ + private static final int DEFAULT_REQUEST_MAX_SIZE = 16 * 1024 * 1024; + private static final Logger logger = LoggerFactory.getLogger(HttpServletStreamableServerTransportProvider.class); /** @@ -100,6 +102,11 @@ public class HttpServletStreamableServerTransportProvider extends HttpServlet private final McpJsonMapper jsonMapper; + /** + * Maximum size, in bytes, of a single request body accepted by this transport. + */ + private final int requestMaxSize; + private McpStreamableServerSession.Factory sessionFactory; /** @@ -136,21 +143,25 @@ public class HttpServletStreamableServerTransportProvider extends HttpServlet * @param keepAliveInterval The interval for keep-alive pings. If null, no keep-alive * will be scheduled. * @param securityValidator The security validator for validating HTTP requests. + * @param requestMaxSize The maximum size, in bytes, of a single request body. Must be + * positive. * @throws IllegalArgumentException if any parameter is null */ private HttpServletStreamableServerTransportProvider(McpJsonMapper jsonMapper, String mcpEndpoint, boolean disallowDelete, McpTransportContextExtractor contextExtractor, - Duration keepAliveInterval, ServerTransportSecurityValidator securityValidator) { + Duration keepAliveInterval, ServerTransportSecurityValidator securityValidator, int requestMaxSize) { Assert.notNull(jsonMapper, "JsonMapper must not be null"); Assert.notNull(mcpEndpoint, "MCP endpoint must not be null"); Assert.notNull(contextExtractor, "Context extractor must not be null"); Assert.notNull(securityValidator, "Security validator must not be null"); + Assert.isTrue(requestMaxSize > 0, "requestMaxSize must be positive"); this.jsonMapper = jsonMapper; this.mcpEndpoint = mcpEndpoint; this.disallowDelete = disallowDelete; this.contextExtractor = contextExtractor; this.securityValidator = securityValidator; + this.requestMaxSize = requestMaxSize; if (keepAliveInterval != null) { @@ -397,6 +408,10 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE, "Server is shutting down"); return; } + if (request.getContentLengthLong() > this.requestMaxSize) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + return; + } try { Map> headers = HttpServletRequestUtils.extractHeaders(request); @@ -420,14 +435,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) McpTransportContext transportContext = this.contextExtractor.extract(request); try { - BufferedReader reader = request.getReader(); - StringBuilder body = new StringBuilder(); - String line; - while ((line = reader.readLine()) != null) { - body.append(line); - } + String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize); - McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body.toString()); + McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body); // Handle initialization request if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest @@ -535,6 +545,9 @@ else if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message("Unknown message type").build()); } } + catch (MaxSizeExceededException e) { + response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, @@ -830,6 +843,8 @@ public static class Builder { private ServerTransportSecurityValidator securityValidator = ServerTransportSecurityValidator.NOOP; + private int requestMaxSize = DEFAULT_REQUEST_MAX_SIZE; + /** * Sets the JsonMapper to use for JSON serialization/deserialization of MCP * messages. @@ -901,6 +916,19 @@ public Builder securityValidator(ServerTransportSecurityValidator securityValida return this; } + /** + * Sets the maximum size, in bytes, of a single request body accepted by this + * transport. Requests whose body exceeds this size are rejected with a 413 + * (Payload Too Large) response. Defaults to 16 MiB if not set. + * @param requestMaxSize The maximum request body size, in bytes. Must be + * positive. + * @return this builder instance + */ + public Builder maxRequestSize(int requestMaxSize) { + this.requestMaxSize = requestMaxSize; + return this; + } + /** * Builds a new instance of {@link HttpServletStreamableServerTransportProvider} * with the configured settings. @@ -911,7 +939,7 @@ public HttpServletStreamableServerTransportProvider build() { Assert.notNull(this.mcpEndpoint, "MCP endpoint must be set"); return new HttpServletStreamableServerTransportProvider( jsonMapper == null ? McpJsonDefaults.getMapper() : jsonMapper, mcpEndpoint, disallowDelete, - contextExtractor, keepAliveInterval, securityValidator); + contextExtractor, keepAliveInterval, securityValidator, requestMaxSize); } } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/MaxSizeExceededException.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/MaxSizeExceededException.java new file mode 100644 index 000000000..ddca5dbdc --- /dev/null +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/MaxSizeExceededException.java @@ -0,0 +1,18 @@ +/* + * Copyright 2026-2026 the original author or authors. + */ + +package io.modelcontextprotocol.server.transport; + +/** + * Thrown when reading an inbound message would exceed the configured maximum size. + * + * @author Daniel Garnier-Moiroux + */ +class MaxSizeExceededException extends Exception { + + MaxSizeExceededException(String message) { + super(message); + } + +} diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtilsTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtilsTests.java new file mode 100644 index 000000000..48c997c04 --- /dev/null +++ b/mcp-core/src/test/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtilsTests.java @@ -0,0 +1,141 @@ +/* + * Copyright 2026-2026 the original author or authors. + */ + +package io.modelcontextprotocol.server.transport; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author Daniel Garnier-Moiroux + */ +class HttpServletRequestUtilsTests { + + @Test + void readsBodyWithinLimit() throws Exception { + HttpServletRequest request = requestWithBody("hello world", null); + + String body = HttpServletRequestUtils.readBody(request, 1024); + + assertThat(body).isEqualTo("hello world"); + } + + @Test + void readsEmptyBody() throws Exception { + HttpServletRequest request = requestWithBody("", null); + + String body = HttpServletRequestUtils.readBody(request, 1024); + + assertThat(body).isEmpty(); + } + + @Test + void allowsBodyExactlyAtLimit() throws Exception { + HttpServletRequest request = requestWithBody("12345", null); + + String body = HttpServletRequestUtils.readBody(request, 5); + + assertThat(body).isEqualTo("12345"); + } + + @Test + void rejectsBodyLargerThanLimit() throws Exception { + HttpServletRequest request = requestWithBody("123456", null); + + assertThatThrownBy(() -> HttpServletRequestUtils.readBody(request, 5)) + .isInstanceOf(MaxSizeExceededException.class); + } + + @Test + void rejectsBodySpanningMultipleReadBuffers() throws Exception { + // larger than the internal 8192-byte read buffer, to exercise multiple loop + // iterations before the limit is exceeded + String largeBody = "a".repeat(9000); + HttpServletRequest request = requestWithBody(largeBody, null); + + assertThatThrownBy(() -> HttpServletRequestUtils.readBody(request, 8500)) + .isInstanceOf(MaxSizeExceededException.class); + } + + @Test + void readsBodySpanningMultipleReadBuffersWithinLimit() throws Exception { + String largeBody = "a".repeat(9000); + HttpServletRequest request = requestWithBody(largeBody, null); + + String body = HttpServletRequestUtils.readBody(request, 9000); + + assertThat(body).isEqualTo(largeBody); + } + + @Test + void defaultsToUtf8WhenCharacterEncodingIsMissing() throws Exception { + HttpServletRequest request = requestWithBody("héllo wörld", null); + + String body = HttpServletRequestUtils.readBody(request, 1024); + + assertThat(body).isEqualTo("héllo wörld"); + } + + @Test + void honorsRequestCharacterEncoding() throws Exception { + HttpServletRequest request = mock(HttpServletRequest.class); + byte[] bytes = "café".getBytes(StandardCharsets.ISO_8859_1); + when(request.getInputStream()).thenReturn(servletInputStream(bytes)); + when(request.getCharacterEncoding()).thenReturn("ISO-8859-1"); + + String body = HttpServletRequestUtils.readBody(request, 1024); + + assertThat(body).isEqualTo("café"); + } + + private static HttpServletRequest requestWithBody(String body, String characterEncoding) throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getInputStream()).thenReturn(servletInputStream(body.getBytes(StandardCharsets.UTF_8))); + when(request.getCharacterEncoding()).thenReturn(characterEncoding); + return request; + } + + private static ServletInputStream servletInputStream(byte[] data) { + ByteArrayInputStream delegate = new ByteArrayInputStream(data); + return new ServletInputStream() { + + @Override + public boolean isFinished() { + return delegate.available() == 0; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener readListener) { + } + + @Override + public int read() { + return delegate.read(); + } + + @Override + public int read(byte[] b, int off, int len) { + return delegate.read(b, off, len); + } + + }; + } + +} diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java index 80a711da1..f6b547aa1 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java @@ -49,6 +49,7 @@ import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; +import io.modelcontextprotocol.spec.McpTransportException; import io.modelcontextprotocol.util.Utils; import net.javacrumbs.jsonunit.core.Option; import org.junit.jupiter.api.Test; @@ -69,6 +70,8 @@ public abstract class AbstractMcpClientServerIntegrationTests { + protected static final int MAX_REQUEST_SIZE = 2048; + abstract protected McpServer.AsyncSpecification prepareAsyncServerBuilder(); abstract protected McpServer.SyncSpecification prepareSyncServerBuilder(); @@ -2198,6 +2201,48 @@ void testResourceSubscription_afterUnsubscribe_noNotification() { } } + // Bounded read + @Test + void testRejectsWhenContentLengthHeaderExceedsLimit() throws Exception { + String inputSchema = """ + { + "type": "object", + "properties": { + "message": { "type": "string" } + }, + "required": ["message"] + } + """; + + McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() + .tool(Tool.builder("tool1", McpJsonDefaults.getMapper(), inputSchema) + .description("tool1 description") + .build()) + .callHandler((exchange, request) -> CallToolResult.builder() + .addContent(TextContent.builder(request.arguments().get("message").toString()).build()) + .build()) + .build(); + + var mcpServer = prepareSyncServerBuilder().capabilities(ServerCapabilities.builder().tools(false).build()) + .tools(tool1) + .build(); + + try (var mcpClient = getMcpClientBuilder().build()) { + String oversizedBody = "a".repeat(MAX_REQUEST_SIZE + 1); + + mcpClient.initialize(); + assertThat(mcpClient.listTools().tools()).contains(tool1.tool()); + + assertThatThrownBy(() -> mcpClient.callTool( + McpSchema.CallToolRequest.builder("tool1").arguments(Map.of("message", oversizedBody)).build())) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("413"); + } + finally { + mcpServer.closeGracefully(); + } + } + private double evaluateExpression(String expression) { // Simple expression evaluator for testing return switch (expression) { diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java index 5b861edb9..c1ebe7bf1 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java @@ -4,6 +4,12 @@ package io.modelcontextprotocol.server; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Map; import java.util.stream.Stream; @@ -17,11 +23,13 @@ import io.modelcontextprotocol.server.transport.HttpServletSseServerTransportProvider; import io.modelcontextprotocol.server.transport.TomcatTestUtil; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleState; import org.apache.catalina.startup.Tomcat; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.provider.Arguments; @@ -51,6 +59,7 @@ public void before() { .contextExtractor(TEST_CONTEXT_EXTRACTOR) .messageEndpoint(CUSTOM_MESSAGE_ENDPOINT) .sseEndpoint(CUSTOM_SSE_ENDPOINT) + .maxRequestSize(MAX_REQUEST_SIZE) .build(); tomcat = TomcatTestUtil.createTomcatServer("", PORT, mcpServerTransportProvider); @@ -98,6 +107,79 @@ public void after() { } } + @Test + void rejectsWhenBodyBytesExceedLimitWithoutContentLengthHeader() throws Exception { + var httpClient = HttpClient.newHttpClient(); + + // Establish an SSE session to obtain a valid session ID + prepareAsyncServerBuilder().build(); + var sseRequest = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + PORT + CUSTOM_SSE_ENDPOINT)) + .header("Accept", "text/event-stream") + .GET() + .build(); + var sseResponseRef = new java.util.concurrent.atomic.AtomicReference>(); + var sessionIdFuture = new java.util.concurrent.CompletableFuture(); + httpClient.sendAsync(sseRequest, HttpResponse.BodyHandlers.ofInputStream()).thenAccept(response -> { + sseResponseRef.set(response); + try (var reader = new java.io.BufferedReader( + new java.io.InputStreamReader(response.body(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.startsWith("data:") && line.contains("sessionId=")) { + String data = line.substring("data:".length()).strip(); + String sessionId = data.substring(data.indexOf("sessionId=") + "sessionId=".length()); + sessionIdFuture.complete(sessionId); + return; + } + } + sessionIdFuture.completeExceptionally(new RuntimeException("sessionId not found in SSE stream")); + response.body().close(); + } + catch (Exception e) { + sessionIdFuture.completeExceptionally(e); + } + }); + String sessionId = sessionIdFuture.get(5, java.util.concurrent.TimeUnit.SECONDS); + + // Send POST request with an over-sized body + byte[] oversizedBody = "a".repeat(MAX_REQUEST_SIZE + 1).getBytes(StandardCharsets.UTF_8); + HttpRequest.BodyPublisher chunkedPublisher = new HttpRequest.BodyPublisher() { + @Override + public long contentLength() { + // A publisher with unknown content length forces chunked transfer + // encoding, bypassing the Content-Length header check and exercising the + // body byte count + return -1; + } + + @Override + public void subscribe(java.util.concurrent.Flow.Subscriber subscriber) { + subscriber.onSubscribe(new java.util.concurrent.Flow.Subscription() { + @Override + public void request(long n) { + subscriber.onNext(ByteBuffer.wrap(oversizedBody)); + subscriber.onComplete(); + } + + @Override + public void cancel() { + } + }); + } + }; + + var request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + PORT + CUSTOM_MESSAGE_ENDPOINT + "?sessionId=" + sessionId)) + .header("Content-Type", "application/json") + .header("Accept", "text/event-stream, application/json") + .POST(chunkedPublisher) + .build(); + + var response = httpClient.send(request, HttpResponse.BodyHandlers.discarding()); + assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } + static McpTransportContextExtractor TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext .create(Map.of("important", "value")); diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java index 6acc77349..6f4ecb54a 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java @@ -4,13 +4,24 @@ package io.modelcontextprotocol.server; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.function.Function; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -38,6 +49,7 @@ import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.ProtocolVersions; +import jakarta.servlet.http.HttpServletResponse; import net.javacrumbs.jsonunit.core.Option; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleState; @@ -49,10 +61,14 @@ import org.slf4j.LoggerFactory; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import org.slf4j.LoggerFactory; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.web.client.RestClient; + import static io.modelcontextprotocol.server.transport.HttpServletStatelessServerTransport.APPLICATION_JSON; import static io.modelcontextprotocol.server.transport.HttpServletStatelessServerTransport.TEXT_EVENT_STREAM; import static io.modelcontextprotocol.util.McpJsonMapperUtils.JSON_MAPPER; @@ -71,6 +87,8 @@ class HttpServletStatelessIntegrationTests { private static final String CUSTOM_MESSAGE_ENDPOINT = "/otherPath/mcp/message"; + private static final int MAX_REQUEST_SIZE = 2048; + private HttpServletStatelessServerTransport mcpStatelessServerTransport; private final McpClient.SyncSpec clientBuilder = McpClient @@ -86,6 +104,7 @@ class HttpServletStatelessIntegrationTests { public void before() { this.mcpStatelessServerTransport = HttpServletStatelessServerTransport.builder() .messageEndpoint(CUSTOM_MESSAGE_ENDPOINT) + .maxRequestSize(MAX_REQUEST_SIZE) .build(); tomcat = TomcatTestUtil.createTomcatServer("", PORT, mcpStatelessServerTransport); @@ -455,7 +474,7 @@ void testStructuredOutputOfObjectArrayValidationSuccess() { "type", "object", "properties", Map.of( "name", Map.of("type", "string"), - "age", Map.of("type", "number")), + "age", Map.of("type", "number")), "required", List.of("name", "age"))); // @formatter:on Tool calculatorTool = Tool.builder("getMembers") @@ -871,6 +890,98 @@ void testRootsListChangedNotificationDoesNotLogWarn() { assertThat(logAppender.list).noneMatch(event -> event.getLevel() == Level.WARN); } + // --------------------------------------- + // Bounded read + // --------------------------------------- + @Test + void testRejectsWhenContentLengthHeaderExceedsLimit() { + String inputSchema = """ + { + "type": "object", + "properties": { + "message": { "type": "string" } + }, + "required": ["message"] + } + """; + + McpStatelessServerFeatures.SyncToolSpecification tool1 = McpStatelessServerFeatures.SyncToolSpecification + .builder() + .tool(Tool.builder("tool1", JSON_MAPPER, inputSchema).description("tool1 description").build()) + .callHandler((transportContext, request) -> CallToolResult.builder() + .addContent(TextContent.builder(request.arguments().get("message").toString()).build()) + .build()) + .build(); + + var mcpServer = McpServer.sync(mcpStatelessServerTransport) + .capabilities(ServerCapabilities.builder().tools(false).build()) + .tools(tool1) + .build(); + + try (var mcpClient = clientBuilder.build()) { + String oversizedBody = "a".repeat(MAX_REQUEST_SIZE + 1); + + mcpClient.initialize(); + assertThat(mcpClient.listTools().tools()).contains(tool1.tool()); + + assertThatThrownBy(() -> mcpClient.callTool( + McpSchema.CallToolRequest.builder("tool1").arguments(Map.of("message", oversizedBody)).build())) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("413"); + } + finally { + mcpServer.closeGracefully(); + } + } + + @Test + void rejectsWhenBodyBytesExceedLimitWithoutContentLengthHeader() throws Exception { + var mcpServer = McpServer.sync(mcpStatelessServerTransport).build(); + + try { + var httpClient = HttpClient.newHttpClient(); + + // A publisher with unknown content length forces chunked transfer + // encoding, bypassing the Content-Length header check and exercising the + // body byte count + byte[] oversizedBody = "a".repeat(MAX_REQUEST_SIZE + 1).getBytes(StandardCharsets.UTF_8); + HttpRequest.BodyPublisher chunkedPublisher = new HttpRequest.BodyPublisher() { + @Override + public long contentLength() { + return -1; + } + + @Override + public void subscribe(java.util.concurrent.Flow.Subscriber subscriber) { + subscriber.onSubscribe(new java.util.concurrent.Flow.Subscription() { + @Override + public void request(long n) { + subscriber.onNext(ByteBuffer.wrap(oversizedBody)); + subscriber.onComplete(); + } + + @Override + public void cancel() { + } + }); + } + }; + + var request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + PORT + CUSTOM_MESSAGE_ENDPOINT)) + .header("Content-Type", "application/json") + .header("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM) + .POST(chunkedPublisher) + .build(); + + var response = httpClient.send(request, HttpResponse.BodyHandlers.discarding()); + assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } + finally { + mcpServer.closeGracefully(); + } + } + private double evaluateExpression(String expression) { // Simple expression evaluator for testing return switch (expression) { diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java index 2c9d14030..cfeb16b19 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java @@ -4,6 +4,12 @@ package io.modelcontextprotocol.server; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; @@ -20,6 +26,7 @@ import io.modelcontextprotocol.server.transport.TomcatTestUtil; import io.modelcontextprotocol.spec.McpSchema; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleState; import org.apache.catalina.startup.Tomcat; @@ -56,6 +63,7 @@ public void before() { .contextExtractor(TEST_CONTEXT_EXTRACTOR) .mcpEndpoint(MESSAGE_ENDPOINT) .keepAliveInterval(Duration.ofSeconds(1)) + .maxRequestSize(MAX_REQUEST_SIZE) .build(); tomcat = TomcatTestUtil.createTomcatServer("", PORT, mcpServerTransportProvider); @@ -147,4 +155,44 @@ void testMissingHandlerReturnsMethodNotFoundError() { static McpTransportContextExtractor TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext .create(Map.of("important", "value")); + @Test + void rejectsWhenBodyBytesExceedLimitWithoutContentLengthHeader() throws Exception { + var httpClient = HttpClient.newHttpClient(); + // A publisher with unknown content length forces chunked transfer encoding, + // bypassing the Content-Length header check and exercising the body byte + // count + byte[] oversizedBody = "a".repeat(MAX_REQUEST_SIZE + 1).getBytes(StandardCharsets.UTF_8); + HttpRequest.BodyPublisher chunkedPublisher = new HttpRequest.BodyPublisher() { + @Override + public long contentLength() { + return -1; + } + + @Override + public void subscribe(java.util.concurrent.Flow.Subscriber subscriber) { + subscriber.onSubscribe(new java.util.concurrent.Flow.Subscription() { + @Override + public void request(long n) { + subscriber.onNext(ByteBuffer.wrap(oversizedBody)); + subscriber.onComplete(); + } + + @Override + public void cancel() { + } + }); + } + }; + + var request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:" + PORT + MESSAGE_ENDPOINT)) + .header("Content-Type", "application/json") + .header("Accept", "text/event-stream, application/json") + .POST(chunkedPublisher) + .build(); + + var response = httpClient.send(request, HttpResponse.BodyHandlers.discarding()); + assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + } + } From 5c191bafa54f7a42040ce56e82ac7e56364c7e70 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Wed, 8 Jul 2026 11:48:22 +0200 Subject: [PATCH 2/2] Bound HTTP client reads Signed-off-by: Daniel Garnier-Moiroux --- .../HttpClientSseClientTransport.java | 60 +++- .../HttpClientStreamableHttpTransport.java | 48 ++- .../client/transport/ResponseSubscribers.java | 314 +++++++++++++++- .../transport/BoundedBodySubscriberTests.java | 334 ++++++++++++++++++ .../HttpClientBoundedReadTestSupport.java | 123 +++++++ ...entSseClientTransportBoundedReadTests.java | 108 ++++++ .../HttpClientSseClientTransportTests.java | 2 +- ...reamableHttpTransportBoundedReadTests.java | 111 ++++++ ...leHttpTransportDeleteBoundedReadTests.java | 126 +++++++ 9 files changed, 1198 insertions(+), 28 deletions(-) create mode 100644 mcp-core/src/test/java/io/modelcontextprotocol/client/transport/BoundedBodySubscriberTests.java create mode 100644 mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientBoundedReadTestSupport.java create mode 100644 mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportBoundedReadTests.java create mode 100644 mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportBoundedReadTests.java create mode 100644 mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportDeleteBoundedReadTests.java diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java index 874da905e..3e4b613ff 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java @@ -88,6 +88,11 @@ public class HttpClientSseClientTransport implements McpClientTransport { /** Default SSE endpoint path */ private static final String DEFAULT_SSE_ENDPOINT = "/sse"; + /** + * Default maximum number of bytes read for a single inbound message. + */ + private static final int DEFAULT_MAX_RESPONSE_SIZE = 16 * 1024 * 1024; // 16MiB + /** Base URI for the MCP server */ private final URI baseUri; @@ -128,6 +133,12 @@ public class HttpClientSseClientTransport implements McpClientTransport { */ private final SseMessageEndpointValidator messageEndpointValidator; + /** + * Maximum number of bytes read for a single inbound message, whether it arrives on + * the SSE stream or as the response to a posted message. + */ + private final int maxResponseSize; + /** * Creates a new transport instance with custom HTTP client builder, object mapper, * and headers. @@ -139,11 +150,13 @@ public class HttpClientSseClientTransport implements McpClientTransport { * @param httpRequestCustomizer customizer for the requestBuilder before executing * requests * @param messageEndpointValidator validator for the message endpoint + * @param maxResponseSize the maximum number of bytes read for a single inbound + * message * @throws IllegalArgumentException if objectMapper, clientBuilder, or headers is null */ HttpClientSseClientTransport(HttpClient httpClient, HttpRequest.Builder requestBuilder, String baseUri, String sseEndpoint, McpJsonMapper jsonMapper, McpAsyncHttpClientRequestCustomizer httpRequestCustomizer, - SseMessageEndpointValidator messageEndpointValidator) { + SseMessageEndpointValidator messageEndpointValidator, int maxResponseSize) { Assert.notNull(jsonMapper, "jsonMapper must not be null"); Assert.hasText(baseUri, "baseUri must not be empty"); Assert.hasText(sseEndpoint, "sseEndpoint must not be empty"); @@ -151,6 +164,7 @@ public class HttpClientSseClientTransport implements McpClientTransport { Assert.notNull(requestBuilder, "requestBuilder must not be null"); Assert.notNull(httpRequestCustomizer, "httpRequestCustomizer must not be null"); Assert.notNull(messageEndpointValidator, "messageEndpointValidator must not be null"); + Assert.isTrue(maxResponseSize > 0, "maxResponseSize must be positive"); this.baseUri = URI.create(baseUri); this.sseEndpoint = sseEndpoint; this.jsonMapper = jsonMapper; @@ -158,6 +172,7 @@ public class HttpClientSseClientTransport implements McpClientTransport { this.requestBuilder = requestBuilder; this.httpRequestCustomizer = httpRequestCustomizer; this.messageEndpointValidator = messageEndpointValidator; + this.maxResponseSize = maxResponseSize; } @Override @@ -195,6 +210,8 @@ public static class Builder { private SseMessageEndpointValidator messageEndpointValidator = new DefaultSseMessageEndpointValidator(); + private int maxResponseSize = DEFAULT_MAX_RESPONSE_SIZE; + /** * Creates a new builder instance. */ @@ -326,6 +343,26 @@ public Builder messageEndpointValidator(SseMessageEndpointValidator messageEndpo return this; } + /** + * Sets the maximum number of bytes read for a single inbound message, whether it + * arrives on the SSE stream or as the response to a posted message. A peer that + * sends a larger message (or never terminates one) has its stream aborted instead + * of forcing the transport to buffer it in memory. Defaults to 16MiB. + * + *

+ * The bound applies per message, not to the stream as a whole: a long-lived SSE + * stream may deliver any number of messages, each up to this size. SSE field + * framing is allowed a small amount of headroom on top of this size, so a message + * of exactly this many bytes is still accepted. + * @param maxResponseSize the maximum inbound message size, in bytes + * @return this builder + */ + public Builder maxResponseSize(int maxResponseSize) { + Assert.isTrue(maxResponseSize > 0, "maxResponseSize must be positive"); + this.maxResponseSize = maxResponseSize; + return this; + } + /** * Builds a new {@link HttpClientSseClientTransport} instance. * @return a new transport instance @@ -334,7 +371,7 @@ public HttpClientSseClientTransport build() { HttpClient httpClient = this.clientBuilder.connectTimeout(this.connectTimeout).build(); return new HttpClientSseClientTransport(httpClient, requestBuilder, baseUri, sseEndpoint, jsonMapper == null ? McpJsonDefaults.getMapper() : jsonMapper, httpRequestCustomizer, - messageEndpointValidator); + messageEndpointValidator, maxResponseSize); } } @@ -353,13 +390,15 @@ public Mono connect(Function, Mono> h var transportContext = ctx.getOrDefault(McpTransportContext.KEY, McpTransportContext.EMPTY); return Mono.from(this.httpRequestCustomizer.customize(builder, "GET", uri, null, transportContext)); }).flatMap(requestBuilder -> Mono.create(sink -> { - Disposable connection = Flux.create(sseSink -> this.httpClient - .sendAsync(requestBuilder.build(), - responseInfo -> ResponseSubscribers.sseToBodySubscriber(responseInfo, sseSink)) - .exceptionallyCompose(e -> { - sseSink.error(e); - return CompletableFuture.failedFuture(e); - })) + Disposable connection = Flux.create( + sseSink -> this.httpClient + .sendAsync(requestBuilder.build(), + responseInfo -> ResponseSubscribers.sseToBodySubscriber(responseInfo, sseSink, + this.maxResponseSize)) + .exceptionallyCompose(e -> { + sseSink.error(e); + return CompletableFuture.failedFuture(e); + })) .map(responseEvent -> (ResponseSubscribers.SseResponseEvent) responseEvent) .flatMap(responseEvent -> { if (isClosing) { @@ -490,7 +529,8 @@ private Mono> sendHttpPost(final String endpoint, final Str return Mono.from(this.httpRequestCustomizer.customize(builder, "POST", requestUri, body, transportContext)); }).flatMap(customizedBuilder -> { var request = customizedBuilder.build(); - return Mono.fromFuture(httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString())); + return Mono.fromFuture( + httpClient.sendAsync(request, ResponseSubscribers.boundedStringBodyHandler(this.maxResponseSize))); }); } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java index ba9c6e505..07a8f5e23 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransport.java @@ -87,6 +87,11 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport { private static final String DEFAULT_ENDPOINT = "/mcp"; + /** + * Default maximum number of bytes read for a single inbound message. + */ + private static final int DEFAULT_MAX_RESPONSE_SIZE = 16 * 1024 * 1024; // 16MiB + /** * HTTP client for sending messages to the server. Uses HTTP POST over the message * endpoint @@ -161,11 +166,18 @@ static boolean isMessageEvent(String eventName) { private final String latestSupportedProtocolVersion; + /** + * Maximum number of bytes read for a single inbound message, whether it arrives on an + * SSE stream or as a JSON response body. + */ + private final int maxResponseSize; + private HttpClientStreamableHttpTransport(McpJsonMapper jsonMapper, HttpClient httpClient, HttpRequest.Builder requestBuilder, String baseUri, String endpoint, boolean resumableStreams, boolean openConnectionOnStartup, McpAsyncHttpClientRequestCustomizer httpRequestCustomizer, McpHttpClientTransportAuthorizationErrorHandler authorizationErrorHandler, - List supportedProtocolVersions) { + List supportedProtocolVersions, int maxResponseSize) { + Assert.isTrue(maxResponseSize > 0, "maxResponseSize must be positive"); this.jsonMapper = jsonMapper; this.httpClient = httpClient; this.requestBuilder = requestBuilder; @@ -181,6 +193,7 @@ private HttpClientStreamableHttpTransport(McpJsonMapper jsonMapper, HttpClient h .sorted(Comparator.reverseOrder()) .findFirst() .get(); + this.maxResponseSize = maxResponseSize; } @Override @@ -229,7 +242,8 @@ private Publisher createDelete(String sessionId) { return Mono.from(this.httpRequestCustomizer.customize(builder, "DELETE", uri, null, transportContext)); }).flatMap(requestBuilder -> { var request = requestBuilder.build(); - return Mono.fromFuture(() -> this.httpClient.sendAsync(request, HttpResponse.BodyHandlers.ofString())); + return Mono.fromFuture(() -> this.httpClient.sendAsync(request, + ResponseSubscribers.boundedStringBodyHandler(this.maxResponseSize))); }).then(); } @@ -470,16 +484,16 @@ private BodyHandler toSendMessageBodySubscriber(FluxSink si if (contentType.contains(TEXT_EVENT_STREAM)) { // For SSE streams, use line subscriber that returns Void logger.debug("Received SSE stream response, using line subscriber"); - return ResponseSubscribers.sseToBodySubscriber(responseInfo, sink); + return ResponseSubscribers.sseToBodySubscriber(responseInfo, sink, this.maxResponseSize); } else if (contentType.contains(APPLICATION_JSON)) { // For JSON responses and others, use string subscriber logger.debug("Received response, using string subscriber"); - return ResponseSubscribers.aggregateBodySubscriber(responseInfo, sink); + return ResponseSubscribers.aggregateBodySubscriber(responseInfo, sink, this.maxResponseSize); } logger.debug("Received Bodyless response, using discarding subscriber"); - return ResponseSubscribers.bodilessBodySubscriber(responseInfo, sink); + return ResponseSubscribers.bodilessBodySubscriber(responseInfo, sink, this.maxResponseSize); }; return responseBodyHandler; @@ -754,6 +768,8 @@ public static class Builder { private McpHttpClientTransportAuthorizationErrorHandler authorizationErrorHandler = McpHttpClientTransportAuthorizationErrorHandler.NOOP; + private int maxResponseSize = DEFAULT_MAX_RESPONSE_SIZE; + /** * Creates a new builder with the specified base URI. * @param baseUri the base URI of the MCP server @@ -952,6 +968,26 @@ public Builder supportedProtocolVersions(List supportedProtocolVersions) return this; } + /** + * Sets the maximum number of bytes read for a single inbound message, whether it + * arrives on an SSE stream or as a JSON response body. A peer that sends a larger + * message (or never terminates one) has its stream aborted instead of forcing the + * transport to buffer it in memory. Defaults to 16MiB. + * + *

+ * The bound applies per message, not to the stream as a whole: a long-lived SSE + * stream may deliver any number of messages, each up to this size. SSE field + * framing is allowed a small amount of headroom on top of this size, so a message + * of exactly this many bytes is still accepted. + * @param maxResponseSize the maximum inbound message size, in bytes + * @return this builder + */ + public Builder maxResponseSize(int maxResponseSize) { + Assert.isTrue(maxResponseSize > 0, "maxResponseSize must be positive"); + this.maxResponseSize = maxResponseSize; + return this; + } + /** * Construct a fresh instance of {@link HttpClientStreamableHttpTransport} using * the current builder configuration. @@ -961,7 +997,7 @@ public HttpClientStreamableHttpTransport build() { HttpClient httpClient = this.clientBuilder.connectTimeout(this.connectTimeout).build(); return new HttpClientStreamableHttpTransport(jsonMapper == null ? McpJsonDefaults.getMapper() : jsonMapper, httpClient, requestBuilder, baseUri, endpoint, resumableStreams, openConnectionOnStartup, - httpRequestCustomizer, authorizationErrorHandler, supportedProtocolVersions); + httpRequestCustomizer, authorizationErrorHandler, supportedProtocolVersions, maxResponseSize); } } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java index 29dc23c35..b19904de6 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java @@ -5,8 +5,13 @@ package io.modelcontextprotocol.client.transport; import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandler; import java.net.http.HttpResponse.BodySubscriber; import java.net.http.HttpResponse.ResponseInfo; +import java.nio.ByteBuffer; +import java.util.List; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.Flow; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Pattern; @@ -30,11 +35,21 @@ * * @author Christian Tzolov * @author Dariusz Jędrzejczyk + * @author Daniel Garnier-Moiroux */ class ResponseSubscribers { private static final Logger logger = LoggerFactory.getLogger(ResponseSubscribers.class); + /** + * Bytes of SSE field framing a single line may carry on top of the message payload: + * {@code "event: "} is the longest field prefix this parser recognises. Line + * terminators are not counted, as they reset the running line length. Without this + * allowance, an event carrying exactly the maximum message size would be rejected + * because of the bytes the SSE wire format adds around it. + */ + private static final int SSE_FRAMING_OVERHEAD = "event: ".length(); + record SseEvent(String id, String event, String data) { } @@ -54,19 +69,80 @@ record SseResponseEvent(ResponseInfo responseInfo, SseEvent sseEvent) implements record AggregateResponseEvent(ResponseInfo responseInfo, String data) implements ResponseEvent { } - static BodySubscriber sseToBodySubscriber(ResponseInfo responseInfo, FluxSink sink) { - return HttpResponse.BodySubscribers - .fromLineSubscriber(FlowAdapters.toFlowSubscriber(new SseLineSubscriber(responseInfo, sink))); + /** + * Creates a {@link BodySubscriber} that parses a Server-Sent Events stream, bounding + * how much memory a single inbound message may occupy. Both the size of an individual + * line (as read off the wire before a terminator is seen) and the accumulated size of + * a multi-line SSE event are capped at {@code maxSize}; a peer exceeding either limit + * has its stream aborted instead of forcing the transport to buffer it in memory. The + * line bound is allowed {@link #SSE_FRAMING_OVERHEAD} extra bytes so that the SSE + * framing around a payload does not count against the payload's own budget. + * @param responseInfo the HTTP response information + * @param sink the sink to emit parsed events to + * @param maxSize the maximum number of bytes read for a single inbound message + */ + static BodySubscriber sseToBodySubscriber(ResponseInfo responseInfo, FluxSink sink, + int maxSize) { + BodySubscriber lineSubscriber = HttpResponse.BodySubscribers + .fromLineSubscriber(FlowAdapters.toFlowSubscriber(new SseLineSubscriber(responseInfo, sink, maxSize))); + return new BoundedLineBodySubscriber(lineSubscriber, plusFramingOverhead(maxSize)); + } + + /** + * Adds {@link #SSE_FRAMING_OVERHEAD} to {@code maxSize}, saturating at + * {@link Integer#MAX_VALUE} rather than overflowing into a negative bound that would + * reject everything. + */ + private static int plusFramingOverhead(int maxSize) { + return maxSize > Integer.MAX_VALUE - SSE_FRAMING_OVERHEAD ? Integer.MAX_VALUE : maxSize + SSE_FRAMING_OVERHEAD; } - static BodySubscriber aggregateBodySubscriber(ResponseInfo responseInfo, FluxSink sink) { - return HttpResponse.BodySubscribers - .fromLineSubscriber(FlowAdapters.toFlowSubscriber(new AggregateSubscriber(responseInfo, sink))); + /** + * Creates a {@link BodySubscriber} that aggregates the whole response body into a + * single event, bounding how much memory it may occupy. Both the size of an + * individual line (as read off the wire before a terminator is seen) and the total + * accumulated body are capped at {@code maxSize}; a peer exceeding either limit has + * its response aborted instead of forcing the transport to buffer it in memory. + * @param responseInfo the HTTP response information + * @param sink the sink to emit the aggregated event to + * @param maxSize the maximum number of bytes read for the response body + */ + static BodySubscriber aggregateBodySubscriber(ResponseInfo responseInfo, FluxSink sink, + int maxSize) { + BodySubscriber lineSubscriber = HttpResponse.BodySubscribers + .fromLineSubscriber(FlowAdapters.toFlowSubscriber(new AggregateSubscriber(responseInfo, sink, maxSize))); + return new BoundedLineBodySubscriber(lineSubscriber, maxSize); } - static BodySubscriber bodilessBodySubscriber(ResponseInfo responseInfo, FluxSink sink) { - return HttpResponse.BodySubscribers + /** + * Creates a {@link BodySubscriber} that discards the response body, bounding how much + * memory reading it may occupy. The body is discarded as it arrives, but the + * underlying line subscriber still buffers each line before handing it over, so a + * peer sending a line longer than {@code maxSize} has its response aborted. + * @param responseInfo the HTTP response information + * @param sink the sink to emit the completion event to + * @param maxSize the maximum number of bytes read for a single line + */ + static BodySubscriber bodilessBodySubscriber(ResponseInfo responseInfo, FluxSink sink, + int maxSize) { + BodySubscriber lineSubscriber = HttpResponse.BodySubscribers .fromLineSubscriber(FlowAdapters.toFlowSubscriber(new BodilessResponseLineSubscriber(responseInfo, sink))); + return new BoundedLineBodySubscriber(lineSubscriber, maxSize); + } + + /** + * Creates a {@link BodyHandler} that reads the response body into a string, bounding + * how much memory it may occupy. A peer sending more than {@code maxSize} bytes has + * its response aborted instead of forcing the transport to buffer it in memory. + * + *

+ * Decoding matches {@link HttpResponse.BodyHandlers#ofString()}, including its + * handling of the charset declared in the {@code Content-Type} header. + * @param maxSize the maximum number of bytes read for the response body + */ + static BodyHandler boundedStringBodyHandler(int maxSize) { + BodyHandler delegate = HttpResponse.BodyHandlers.ofString(); + return responseInfo -> new BoundedTotalBodySubscriber<>(delegate.apply(responseInfo), maxSize); } static class SseLineSubscriber extends BaseSubscriber { @@ -112,18 +188,30 @@ static class SseLineSubscriber extends BaseSubscriber { */ private ResponseInfo responseInfo; + /** + * The maximum number of bytes that may accumulate for a single SSE event. A peer + * that never terminates an event (e.g. an endless stream of {@code data:} lines) + * has its stream aborted instead of exhausting memory. The accumulated data is + * measured in characters, which for UTF-8 is never more than the number of bytes + * it was decoded from. + */ + private final int maxSize; + /** * Creates a new LineSubscriber that will emit parsed SSE events to the provided * sink. * @param sink the {@link FluxSink} to emit parsed {@link ResponseEvent} objects * to + * @param maxSize the maximum number of bytes that may accumulate for a single SSE + * event */ - public SseLineSubscriber(ResponseInfo responseInfo, FluxSink sink) { + public SseLineSubscriber(ResponseInfo responseInfo, FluxSink sink, int maxSize) { this.sink = sink; this.eventBuilder = new StringBuilder(); this.currentEventId = new AtomicReference<>(); this.currentEventType = new AtomicReference<>(); this.responseInfo = responseInfo; + this.maxSize = maxSize; } @Override @@ -155,7 +243,18 @@ protected void hookOnNext(String line) { if (line.startsWith("data:")) { var matcher = EVENT_DATA_PATTERN.matcher(line); if (matcher.find()) { - this.eventBuilder.append(matcher.group(1).trim()).append("\n"); + String data = matcher.group(1).trim(); + // Measured before appending, so that an event carrying exactly + // maxSize of data is accepted: the trailing separator below is + // stripped again before the event is emitted. + if (this.eventBuilder.length() + data.length() > this.maxSize) { + upstream().cancel(); + this.sink.error( + new McpTransportException("Inbound SSE event exceeds the maximum allowed size of " + + this.maxSize + " bytes")); + return; + } + this.eventBuilder.append(data).append("\n"); } upstream().request(1); } @@ -225,15 +324,26 @@ static class AggregateSubscriber extends BaseSubscriber { volatile boolean hasRequestedDemand = false; + /** + * The maximum number of bytes that may accumulate for the aggregated response + * body. A peer that sends a larger body has its response aborted instead of + * exhausting memory. The accumulated body is measured in characters, which for + * UTF-8 is never more than the number of bytes it was decoded from. + */ + private final int maxSize; + /** * Creates a new JsonLineSubscriber that will emit parsed JSON-RPC messages. * @param sink the {@link FluxSink} to emit parsed {@link ResponseEvent} objects * to + * @param maxSize the maximum number of bytes that may accumulate for the + * aggregated response body */ - public AggregateSubscriber(ResponseInfo responseInfo, FluxSink sink) { + public AggregateSubscriber(ResponseInfo responseInfo, FluxSink sink, int maxSize) { this.sink = sink; this.eventBuilder = new StringBuilder(); this.responseInfo = responseInfo; + this.maxSize = maxSize; } @Override @@ -252,6 +362,15 @@ protected void hookOnSubscribe(Subscription subscription) { @Override protected void hookOnNext(String line) { + // Measured before appending, so that a body of exactly maxSize is accepted. + // The separator this adds back for each line stands in for the terminator the + // peer sent, which the line subscriber has already stripped. + if (this.eventBuilder.length() + line.length() > this.maxSize) { + upstream().cancel(); + this.sink.error(new McpTransportException( + "Inbound response body exceeds the maximum allowed size of " + this.maxSize + " bytes")); + return; + } this.eventBuilder.append(line).append("\n"); } @@ -324,4 +443,177 @@ protected void hookOnError(Throwable throwable) { } + /** + * Base for {@link BodySubscriber} wrappers that transparently forward the response + * body to a delegate, but abort it once the peer exceeds a size bound. + * + *

+ * Aborting cancels the upstream subscription, which closes the connection, and + * signals a {@link McpTransportException} to the delegate so the failure surfaces + * both through the body's {@link CompletionStage} and through any sink the delegate + * feeds. + */ + abstract static class BoundedBodySubscriber implements BodySubscriber { + + private final BodySubscriber delegate; + + protected final int maxSize; + + /** + * What the bound applies to, e.g. {@code "Inbound line"}, used to build the + * failure message. + */ + private final String boundedEntity; + + private Flow.Subscription subscription; + + private volatile boolean done = false; + + BoundedBodySubscriber(BodySubscriber delegate, int maxSize, String boundedEntity) { + this.delegate = delegate; + this.maxSize = maxSize; + this.boundedEntity = boundedEntity; + } + + @Override + public CompletionStage getBody() { + return this.delegate.getBody(); + } + + @Override + public void onSubscribe(Flow.Subscription subscription) { + this.subscription = subscription; + this.delegate.onSubscribe(subscription); + } + + @Override + public void onNext(List buffers) { + if (this.done) { + return; + } + for (ByteBuffer buffer : buffers) { + if (!checkSize(buffer)) { + this.done = true; + this.subscription.cancel(); + this.delegate.onError(new McpTransportException( + this.boundedEntity + " exceeds the maximum allowed size of " + this.maxSize + " bytes")); + return; + } + } + this.delegate.onNext(buffers); + } + + /** + * Accounts for the bytes in {@code buffer}, which must be inspected with absolute + * reads only so the delegate still sees the original position. + * @param buffer the buffer about to be handed to the delegate + * @return {@code true} to accept the buffer, or {@code false} to abort the + * response because the bound has been exceeded + */ + protected abstract boolean checkSize(ByteBuffer buffer); + + @Override + public void onError(Throwable throwable) { + if (this.done) { + return; + } + this.done = true; + this.delegate.onError(throwable); + } + + @Override + public void onComplete() { + if (this.done) { + return; + } + this.done = true; + this.delegate.onComplete(); + } + + } + + /** + * A {@link BoundedBodySubscriber} that aborts the response once a single line (a run + * of bytes with no CR/LF terminator) exceeds {@code maxSize} bytes. + * + *

+ * {@link HttpResponse.BodySubscribers#fromLineSubscriber} buffers characters until it + * encounters a line terminator, so a peer that never terminates a line (or sends an + * enormous one) would force the transport to buffer it in memory. This wrapper counts + * bytes as they arrive off the wire and cancels the subscription before that buffer + * can grow without bound. + */ + static final class BoundedLineBodySubscriber extends BoundedBodySubscriber { + + private long bytesSinceLineTerminator = 0; + + BoundedLineBodySubscriber(BodySubscriber delegate, int maxSize) { + super(delegate, maxSize, "Inbound line"); + } + + @Override + protected boolean checkSize(ByteBuffer buffer) { + int position = buffer.position(); + int limit = buffer.limit(); + if (position == limit) { + return true; + } + if (this.bytesSinceLineTerminator + (limit - position) <= this.maxSize) { + // No line ending in this buffer can exceed the limit, because there are + // not enough bytes since the last terminator for one to. Only the + // trailing (still unterminated) run matters, so scan back to the last + // terminator instead of walking every byte. + this.bytesSinceLineTerminator = lengthOfTrailingRun(buffer, position, limit); + return true; + } + // The limit is within reach, so account for every line exactly. + for (int i = position; i < limit; i++) { + byte b = buffer.get(i); + if (b == '\n' || b == '\r') { + this.bytesSinceLineTerminator = 0; + } + else if (++this.bytesSinceLineTerminator > this.maxSize) { + return false; + } + } + return true; + } + + /** + * Returns the number of bytes after the last line terminator in the buffer, or + * the whole span added to the running count when the buffer holds no terminator. + */ + private long lengthOfTrailingRun(ByteBuffer buffer, int position, int limit) { + for (int i = limit - 1; i >= position; i--) { + byte b = buffer.get(i); + if (b == '\n' || b == '\r') { + return limit - 1 - i; + } + } + return this.bytesSinceLineTerminator + (limit - position); + } + + } + + /** + * A {@link BoundedBodySubscriber} that aborts the response once the body as a whole + * exceeds {@code maxSize} bytes. Suitable for delegates that aggregate the entire + * body in memory, such as {@link HttpResponse.BodyHandlers#ofString()}. + */ + static final class BoundedTotalBodySubscriber extends BoundedBodySubscriber { + + private long totalBytes = 0; + + BoundedTotalBodySubscriber(BodySubscriber delegate, int maxSize) { + super(delegate, maxSize, "Inbound response body"); + } + + @Override + protected boolean checkSize(ByteBuffer buffer) { + this.totalBytes += buffer.remaining(); + return this.totalBytes <= this.maxSize; + } + + } + } diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/client/transport/BoundedBodySubscriberTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/client/transport/BoundedBodySubscriberTests.java new file mode 100644 index 000000000..5f350319e --- /dev/null +++ b/mcp-core/src/test/java/io/modelcontextprotocol/client/transport/BoundedBodySubscriberTests.java @@ -0,0 +1,334 @@ +/* + * Copyright 2024-2026 the original author or authors. + */ + +package io.modelcontextprotocol.client.transport; + +import java.net.http.HttpResponse.BodySubscriber; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.Flow; + +import io.modelcontextprotocol.client.transport.ResponseSubscribers.BoundedLineBodySubscriber; +import io.modelcontextprotocol.client.transport.ResponseSubscribers.BoundedTotalBodySubscriber; +import io.modelcontextprotocol.spec.McpTransportException; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests the size accounting in {@link ResponseSubscribers.BoundedBodySubscriber} and its + * two implementations. These bound how much of a response the transport will buffer, so + * the accounting is exercised directly rather than only through a live HTTP exchange: + * buffer boundaries, line terminators split across buffers, and the exact limit are all + * places where an off-by-one either lets a peer past the bound or rejects a legitimate + * message. + * + * @author Daniel Garnier-Moiroux + */ +class BoundedBodySubscriberTests { + + private static final int MAX_SIZE = 16; + + private final RecordingBodySubscriber delegate = new RecordingBodySubscriber(); + + private final RecordingSubscription subscription = new RecordingSubscription(); + + // --- BoundedLineBodySubscriber: per-line accounting ----------------------- + + @Test + void lineSubscriberAcceptsEmptyBuffer() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer(""))).isTrue(); + } + + @Test + void lineSubscriberAcceptsLineOfExactlyMaxSize() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(MAX_SIZE)))).isTrue(); + } + + @Test + void lineSubscriberRejectsLineOneByteOverMaxSize() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(MAX_SIZE + 1)))).isFalse(); + } + + @Test + void lineSubscriberAccumulatesAcrossBuffers() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(10)))).isTrue(); + assertThat(subscriber.checkSize(buffer("a".repeat(6)))).isTrue(); + // 17th byte of the same unterminated line. + assertThat(subscriber.checkSize(buffer("a"))).isFalse(); + } + + @Test + void lineSubscriberAcceptsUnboundedTotalOfTerminatedLines() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + // Far more than MAX_SIZE in total, but no single line comes close to it. + for (int i = 0; i < 100; i++) { + assertThat(subscriber.checkSize(buffer("aaaa\n"))).isTrue(); + } + } + + @Test + void lineSubscriberResetsOnTerminatorAtEndOfBuffer() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(10) + "\n"))).isTrue(); + // A fresh line, so the previous 10 bytes must not count towards it. + assertThat(subscriber.checkSize(buffer("a".repeat(MAX_SIZE)))).isTrue(); + } + + @Test + void lineSubscriberResetsOnTerminatorAtStartOfBuffer() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(MAX_SIZE)))).isTrue(); + assertThat(subscriber.checkSize(buffer("\n" + "a".repeat(MAX_SIZE)))).isTrue(); + } + + @Test + void lineSubscriberHandlesCrLfSplitAcrossBuffers() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(12) + "\r"))).isTrue(); + assertThat(subscriber.checkSize(buffer("\n" + "a".repeat(MAX_SIZE)))).isTrue(); + } + + @Test + void lineSubscriberAcceptsBufferLargerThanMaxSizeHoldingOnlyShortLines() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + // Forces the exact per-byte accounting path: the buffer alone is well over the + // limit, yet every line in it is legitimate. + assertThat(subscriber.checkSize(buffer("aaaa\n".repeat(20)))).isTrue(); + } + + @Test + void lineSubscriberRejectsRunSpanningManyBuffers() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + boolean accepted = true; + for (int i = 0; i < 10 && accepted; i++) { + accepted = subscriber.checkSize(buffer("aa")); + } + + assertThat(accepted).isFalse(); + } + + @Test + void lineSubscriberOnlyCountsFromTheBufferPosition() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + ByteBuffer partiallyConsumed = buffer("a".repeat(MAX_SIZE * 2)); + partiallyConsumed.position(MAX_SIZE * 2 - 4); + + assertThat(subscriber.checkSize(partiallyConsumed)).isTrue(); + } + + @Test + void lineSubscriberDoesNotConsumeTheBuffer() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + ByteBuffer buffer = buffer("aaaa\nbbbb"); + buffer.position(2); + + subscriber.checkSize(buffer); + + assertThat(buffer.position()).isEqualTo(2); + assertThat(buffer.limit()).isEqualTo(9); + } + + // --- BoundedTotalBodySubscriber: whole-body accounting -------------------- + + @Test + void totalSubscriberAcceptsBodyOfExactlyMaxSize() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(8)))).isTrue(); + assertThat(subscriber.checkSize(buffer("a".repeat(8)))).isTrue(); + } + + @Test + void totalSubscriberRejectsBodyOneByteOverMaxSize() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + + assertThat(subscriber.checkSize(buffer("a".repeat(8)))).isTrue(); + assertThat(subscriber.checkSize(buffer("a".repeat(9)))).isFalse(); + } + + @Test + void totalSubscriberIsNotResetByLineTerminators() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + + // Unlike the per-line bound, terminated lines still count towards the total. + assertThat(subscriber.checkSize(buffer("aaaa\n".repeat(3)))).isTrue(); + assertThat(subscriber.checkSize(buffer("aaaa\n"))).isFalse(); + } + + @Test + void totalSubscriberOnlyCountsFromTheBufferPosition() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + ByteBuffer partiallyConsumed = buffer("a".repeat(MAX_SIZE * 2)); + partiallyConsumed.position(MAX_SIZE); + + assertThat(subscriber.checkSize(partiallyConsumed)).isTrue(); + } + + @Test + void totalSubscriberDoesNotConsumeTheBuffer() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + ByteBuffer buffer = buffer("aaaa"); + + subscriber.checkSize(buffer); + + assertThat(buffer.position()).isZero(); + assertThat(buffer.remaining()).isEqualTo(4); + } + + // --- onNext: what a failed check does ------------------------------------ + + @Test + void forwardsBuffersWhileWithinBounds() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + List buffers = List.of(buffer("aaaa\n"), buffer("bbbb\n")); + + subscriber.onNext(buffers); + + assertThat(this.delegate.received).containsExactly(buffers); + assertThat(this.delegate.error).isNull(); + assertThat(this.subscription.cancellations).isZero(); + } + + @Test + void abortsTheResponseWhenTheLineBoundIsExceeded() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + subscriber.onNext(List.of(buffer("a".repeat(MAX_SIZE + 1)))); + + assertThat(this.subscription.cancellations).isEqualTo(1); + assertThat(this.delegate.received).isEmpty(); + assertThat(this.delegate.error).isInstanceOf(McpTransportException.class) + .hasMessage("Inbound line exceeds the maximum allowed size of " + MAX_SIZE + " bytes"); + } + + @Test + void abortsTheResponseWhenTheTotalBoundIsExceeded() { + BoundedTotalBodySubscriber subscriber = totalSubscriber(); + + subscriber.onNext(List.of(buffer("a".repeat(MAX_SIZE + 1)))); + + assertThat(this.subscription.cancellations).isEqualTo(1); + assertThat(this.delegate.received).isEmpty(); + assertThat(this.delegate.error).isInstanceOf(McpTransportException.class) + .hasMessage("Inbound response body exceeds the maximum allowed size of " + MAX_SIZE + " bytes"); + } + + @Test + void withholdsTheWholeListWhenALaterBufferExceedsTheBound() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + + subscriber.onNext(List.of(buffer("a".repeat(8)), buffer("a".repeat(9)))); + + assertThat(this.delegate.received).isEmpty(); + assertThat(this.delegate.error).isInstanceOf(McpTransportException.class); + } + + @Test + void ignoresFurtherSignalsOnceAborted() { + BoundedLineBodySubscriber subscriber = lineSubscriber(); + subscriber.onNext(List.of(buffer("a".repeat(MAX_SIZE + 1)))); + Throwable firstError = this.delegate.error; + + // The HTTP client may still signal after the subscription is cancelled. + subscriber.onNext(List.of(buffer("aaaa"))); + subscriber.onError(new RuntimeException("late failure")); + subscriber.onComplete(); + + assertThat(this.subscription.cancellations).isEqualTo(1); + assertThat(this.delegate.received).isEmpty(); + assertThat(this.delegate.error).isSameAs(firstError); + assertThat(this.delegate.completed).isFalse(); + } + + // --- fixtures ------------------------------------------------------------ + + private BoundedLineBodySubscriber lineSubscriber() { + BoundedLineBodySubscriber subscriber = new BoundedLineBodySubscriber(this.delegate, MAX_SIZE); + subscriber.onSubscribe(this.subscription); + return subscriber; + } + + private BoundedTotalBodySubscriber totalSubscriber() { + BoundedTotalBodySubscriber subscriber = new BoundedTotalBodySubscriber<>(this.delegate, MAX_SIZE); + subscriber.onSubscribe(this.subscription); + return subscriber; + } + + private static ByteBuffer buffer(String content) { + return ByteBuffer.wrap(content.getBytes(StandardCharsets.US_ASCII)); + } + + private static final class RecordingBodySubscriber implements BodySubscriber { + + private final List> received = new ArrayList<>(); + + private final CompletableFuture body = new CompletableFuture<>(); + + private Throwable error; + + private boolean completed; + + @Override + public CompletionStage getBody() { + return this.body; + } + + @Override + public void onSubscribe(Flow.Subscription subscription) { + } + + @Override + public void onNext(List item) { + this.received.add(item); + } + + @Override + public void onError(Throwable throwable) { + this.error = throwable; + this.body.completeExceptionally(throwable); + } + + @Override + public void onComplete() { + this.completed = true; + this.body.complete(null); + } + + } + + private static final class RecordingSubscription implements Flow.Subscription { + + private int cancellations; + + @Override + public void request(long n) { + } + + @Override + public void cancel() { + this.cancellations++; + } + + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientBoundedReadTestSupport.java b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientBoundedReadTestSupport.java new file mode 100644 index 000000000..a22dc1301 --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientBoundedReadTestSupport.java @@ -0,0 +1,123 @@ +/* + * Copyright 2024-2026 the original author or authors. + */ + +package io.modelcontextprotocol.client.transport; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.Executors; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import io.modelcontextprotocol.server.transport.TomcatTestUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +/** + * Shared fixture for the transport bounded-read tests: a bare {@link HttpServer} whose + * response body is written by a per-test {@link Responder}. + * + * @author Daniel Garnier-Moiroux + */ +abstract class HttpClientBoundedReadTestSupport { + + protected static final int MAX_SIZE = 1024; + + private HttpServer server; + + protected String host; + + /** + * Writes a response body to the exchange. + */ + @FunctionalInterface + protected interface Responder { + + void respond(OutputStream body) throws IOException; + + } + + /** + * The path the transport under test talks to. + */ + protected abstract String endpoint(); + + @BeforeEach + void startServer() throws IOException { + int port = TomcatTestUtil.findAvailablePort(); + this.host = "http://localhost:" + port; + this.server = HttpServer.create(new InetSocketAddress(port), 0); + this.server.setExecutor(Executors.newCachedThreadPool()); + this.server.start(); + } + + @AfterEach + void stopServer() { + if (this.server != null) { + this.server.stop(0); + } + } + + /** + * Registers a handler that responds with the given content type and body. + */ + protected void respondWith(String path, String contentType, Responder responder) { + this.server.createContext(path, exchange -> { + exchange.getResponseHeaders().set("Content-Type", contentType); + exchange.sendResponseHeaders(200, 0); + try (OutputStream body = exchange.getResponseBody()) { + responder.respond(body); + } + catch (IOException ignored) { + // The client aborts the response once the limit is exceeded, which closes + // the connection and makes further writes fail. That is the behaviour + // under test. + } + finally { + exchange.close(); + } + }); + } + + /** + * A responder that writes {@code chunks} blocks of {@code 'a'} with no line + * terminator anywhere, so nothing downstream can ever flush a line. + */ + protected static Responder unterminatedLine(int chunks) { + return body -> { + byte[] chunk = new byte[MAX_SIZE]; + java.util.Arrays.fill(chunk, (byte) 'a'); + for (int i = 0; i < chunks; i++) { + body.write(chunk); + body.flush(); + } + }; + } + + /** + * A responder that writes enough short, properly terminated lines to exceed the limit + * in aggregate. + */ + protected static Responder manyShortLines(String prefix) { + return body -> { + byte[] line = (prefix + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n").getBytes(StandardCharsets.UTF_8); + for (int i = 0; i < (MAX_SIZE / line.length) + 64; i++) { + body.write(line); + body.flush(); + } + }; + } + + protected static boolean messageContains(Throwable t, String expected) { + for (Throwable current = t; current != null; current = current.getCause()) { + if (current.getMessage() != null && current.getMessage().contains(expected)) { + return true; + } + } + return false; + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportBoundedReadTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportBoundedReadTests.java new file mode 100644 index 000000000..df613265f --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportBoundedReadTests.java @@ -0,0 +1,108 @@ +/* + * Copyright 2024-2026 the original author or authors. + */ + +package io.modelcontextprotocol.client.transport; + +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +/** + * Verifies that {@link HttpClientSseClientTransport} bounds the amount of memory a single + * inbound message can occupy, so a malicious or buggy server cannot exhaust the client's + * memory by streaming an unterminated line, an endless event, or an oversized response to + * a posted message. + * + * @author Daniel Garnier-Moiroux + */ +@Timeout(15) +class HttpClientSseClientTransportBoundedReadTests extends HttpClientBoundedReadTestSupport { + + private static final String MESSAGE_ENDPOINT = "/message"; + + private final CountDownLatch keepStreamOpen = new CountDownLatch(1); + + @Override + protected String endpoint() { + return "/sse"; + } + + @AfterEach + void releaseStream() { + this.keepStreamOpen.countDown(); + } + + @Test + void shouldRejectSingleLineExceedingMaxSize() { + // A line that never terminates, so the line buffer underneath the SSE parser + // would grow without limit before any event could be flushed. + respondWith(endpoint(), "text/event-stream", unterminatedLine(8)); + + StepVerifier.create(connect()) + .verifyErrorMatches(t -> messageContains(t, "Inbound line exceeds the maximum allowed size")); + } + + @Test + void shouldRejectEventExceedingMaxSize() { + // Many short, terminated "data:" lines with no blank line to end the event. Each + // line is small, but the accumulated event data would grow without limit. + respondWith(endpoint(), "text/event-stream", manyShortLines("data:")); + + StepVerifier.create(connect()) + .verifyErrorMatches(t -> messageContains(t, "Inbound SSE event exceeds the maximum allowed size")); + } + + @Test + void shouldRejectPostResponseExceedingMaxSize() throws Exception { + // The response to a posted message is read into a string in full, so an oversized + // one must abort rather than accumulate. + respondWith(endpoint(), "text/event-stream", body -> { + body.write(("event:endpoint\ndata:" + MESSAGE_ENDPOINT + "\n\n").getBytes(StandardCharsets.UTF_8)); + body.flush(); + awaitTeardown(); + }); + respondWith(MESSAGE_ENDPOINT, "application/json", unterminatedLine(64)); + + HttpClientSseClientTransport transport = transport(); + transport.connect(Function.identity()).block(Duration.ofSeconds(5)); + + StepVerifier.create(sendMessage(transport)) + .verifyErrorMatches(t -> messageContains(t, "Inbound response body exceeds the maximum allowed size")); + } + + private void awaitTeardown() { + try { + this.keepStreamOpen.await(10, TimeUnit.SECONDS); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + } + + private HttpClientSseClientTransport transport() { + return HttpClientSseClientTransport.builder(this.host).maxResponseSize(MAX_SIZE).build(); + } + + private Mono connect() { + return transport().connect(Function.identity()); + } + + private Mono sendMessage(HttpClientSseClientTransport transport) { + JSONRPCRequest request = new JSONRPCRequest(McpSchema.JSONRPC_VERSION, "test-method", "test-id", + Map.of("key", "value")); + return transport.sendMessage(request); + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportTests.java index 7e6dc094c..8735aaaf1 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransportTests.java @@ -81,7 +81,7 @@ public TestHttpClientSseClientTransport(final String baseUri, SseMessageEndpointValidator sseMessageEndpointValidator) { super(HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).build(), HttpRequest.newBuilder().header("Content-Type", "application/json"), baseUri, "/sse", JSON_MAPPER, - McpAsyncHttpClientRequestCustomizer.NOOP, sseMessageEndpointValidator); + McpAsyncHttpClientRequestCustomizer.NOOP, sseMessageEndpointValidator, 16 * 1024 * 1024); } public int getInboundMessageCount() { diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportBoundedReadTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportBoundedReadTests.java new file mode 100644 index 000000000..856dc88fb --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportBoundedReadTests.java @@ -0,0 +1,111 @@ +/* + * Copyright 2024-2026 the original author or authors. + */ + +package io.modelcontextprotocol.client.transport; + +import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.function.Function; + +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCRequest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +/** + * Verifies that {@link HttpClientStreamableHttpTransport} bounds the amount of memory a + * single inbound message can occupy, so a malicious or buggy server cannot exhaust the + * client's memory by streaming an unterminated line or an endless event. + * + * @author Daniel Garnier-Moiroux + */ +@Timeout(15) +class HttpClientStreamableHttpTransportBoundedReadTests extends HttpClientBoundedReadTestSupport { + + @Override + protected String endpoint() { + return "/mcp"; + } + + @Test + void shouldRejectSingleLineExceedingMaxSize() { + // A line that never terminates, so the line buffer underneath the SSE parser + // would grow without limit before any event could be flushed. + respondWith(endpoint(), "text/event-stream", unterminatedLine(8)); + + StepVerifier.create(sendMessage()) + .verifyErrorMatches(t -> messageContains(t, "Inbound line exceeds the maximum allowed size")); + } + + @Test + void shouldRejectEventExceedingMaxSize() { + // Many short, terminated "data:" lines with no blank line to end the event. Each + // line is small, but the accumulated event data would grow without limit. + respondWith(endpoint(), "text/event-stream", manyShortLines("data:")); + + StepVerifier.create(sendMessage()) + .verifyErrorMatches(t -> messageContains(t, "Inbound SSE event exceeds the maximum allowed size")); + } + + @Test + void shouldRejectJsonResponseExceedingMaxSize() { + // A multi-line application/json response whose total size exceeds the limit. Each + // line is small, but the aggregated body would grow without limit. + respondWith(endpoint(), "application/json", manyShortLines("")); + + StepVerifier.create(sendMessage()) + .verifyErrorMatches(t -> messageContains(t, "Inbound response body exceeds the maximum allowed size")); + } + + @Test + void shouldRejectDiscardedResponseExceedingMaxSize() { + // A content type the transport neither parses as SSE nor as JSON, so the body is + // discarded. The line subscriber underneath still buffers each line, so an + // unterminated one must abort the response rather than accumulate. + respondWith(endpoint(), "text/plain", unterminatedLine(8)); + + StepVerifier.create(sendMessage()) + .verifyErrorMatches(t -> messageContains(t, "Inbound line exceeds the maximum allowed size")); + } + + @Test + void shouldAcceptEventOfExactlyMaxSize() { + // The bound is inclusive and the SSE framing around the payload is given its own + // headroom, so a message of exactly maxResponseSize must still be delivered. + respondWith(endpoint(), "text/event-stream", body -> body + .write(("data:" + jsonRpcResponseOfExactly(MAX_SIZE) + "\n\n").getBytes(StandardCharsets.UTF_8))); + + StepVerifier.create(sendMessage()).verifyComplete(); + } + + @Test + void shouldAcceptJsonResponseOfExactlyMaxSize() { + // Same inclusive bound on the aggregated body. + respondWith(endpoint(), "application/json", + body -> body.write(jsonRpcResponseOfExactly(MAX_SIZE).getBytes(StandardCharsets.UTF_8))); + + StepVerifier.create(sendMessage()).verifyComplete(); + } + + /** + * A single-line JSON-RPC response padded to exactly {@code size} bytes. + */ + private static String jsonRpcResponseOfExactly(int size) { + String prefix = "{\"jsonrpc\":\"2.0\",\"id\":\"test-id\",\"result\":{\"pad\":\""; + String suffix = "\"}}"; + return prefix + "a".repeat(size - prefix.length() - suffix.length()) + suffix; + } + + private Mono sendMessage() { + HttpClientStreamableHttpTransport transport = HttpClientStreamableHttpTransport.builder(this.host) + .maxResponseSize(MAX_SIZE) + .build(); + JSONRPCRequest request = new JSONRPCRequest(McpSchema.JSONRPC_VERSION, "test-method", "test-id", + Map.of("key", "value")); + return transport.connect(Function.identity()).then(transport.sendMessage(request)); + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportDeleteBoundedReadTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportDeleteBoundedReadTests.java new file mode 100644 index 000000000..29a18a635 --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/client/transport/HttpClientStreamableHttpTransportDeleteBoundedReadTests.java @@ -0,0 +1,126 @@ +/* + * Copyright 2024-2026 the original author or authors. + */ + +package io.modelcontextprotocol.client.transport; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import com.sun.net.httpserver.HttpServer; +import io.modelcontextprotocol.server.transport.TomcatTestUtil; +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCRequest; +import io.modelcontextprotocol.spec.McpTransportException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Verifies that {@link HttpClientStreamableHttpTransport} bounds the response to the + * session-terminating DELETE, so a server cannot exhaust the client's memory on the way + * out. Checks both that the failure surfaces to the caller and that the client actually + * stops reading rather than draining whatever the server sends. + * + * @author Daniel Garnier-Moiroux + */ +@Timeout(30) +class HttpClientStreamableHttpTransportDeleteBoundedReadTests { + + private static final int MAX_SIZE = 1024; + + private static final int FLOOD_BYTES = 32 * 1024 * 1024; + + private HttpServer server; + + private String host; + + private final AtomicLong deleteBytesWritten = new AtomicLong(); + + private final CountDownLatch deleteFinished = new CountDownLatch(1); + + @BeforeEach + void setUp() throws IOException { + int port = TomcatTestUtil.findAvailablePort(); + this.host = "http://localhost:" + port; + this.server = HttpServer.create(new InetSocketAddress(port), 0); + this.server.createContext("/mcp", exchange -> { + if ("DELETE".equals(exchange.getRequestMethod())) { + floodDeleteResponse(exchange); + return; + } + // Hand out a session id so the transport issues a DELETE on close. + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.getResponseHeaders().set("Mcp-Session-Id", "test-session"); + byte[] body = "{\"jsonrpc\":\"2.0\",\"id\":\"test-id\",\"result\":{}}".getBytes(StandardCharsets.UTF_8); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + this.server.setExecutor(Executors.newCachedThreadPool()); + this.server.start(); + } + + private void floodDeleteResponse(com.sun.net.httpserver.HttpExchange exchange) throws IOException { + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, 0); + try (OutputStream body = exchange.getResponseBody()) { + byte[] chunk = new byte[64 * 1024]; + java.util.Arrays.fill(chunk, (byte) 'a'); // no line terminator, ever + while (this.deleteBytesWritten.get() < FLOOD_BYTES) { + body.write(chunk); + body.flush(); + this.deleteBytesWritten.addAndGet(chunk.length); + } + } + catch (IOException ignored) { + // Expected: the client aborts the response once the limit is exceeded. + } + finally { + exchange.close(); + this.deleteFinished.countDown(); + } + } + + @AfterEach + void tearDown() { + if (this.server != null) { + this.server.stop(0); + } + } + + @Test + void shouldStopReadingDeleteResponseOnceLimitExceeded() throws Exception { + HttpClientStreamableHttpTransport transport = HttpClientStreamableHttpTransport.builder(this.host) + .maxResponseSize(MAX_SIZE) + .build(); + transport.connect(m -> m).block(Duration.ofSeconds(5)); + JSONRPCRequest request = new JSONRPCRequest(McpSchema.JSONRPC_VERSION, "test-method", "test-id", + Map.of("key", "value")); + transport.sendMessage(request).block(Duration.ofSeconds(5)); + + assertThatThrownBy(() -> transport.closeGracefully().block(Duration.ofSeconds(10))) + .isInstanceOf(McpTransportException.class) + .hasMessageContaining("Inbound response body exceeds the maximum allowed size"); + + assertThat(this.deleteFinished.await(15, TimeUnit.SECONDS)) + .as("the server's DELETE response should be cut short by the client") + .isTrue(); + assertThat(this.deleteBytesWritten.get()).as("bytes the client let the server stream on DELETE") + .isLessThan(FLOOD_BYTES); + } + +}