Bound HTTP server reads
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
This commit is contained in:
parent
1fcb40430c
commit
6a41dedaa0
4 changed files with 619 additions and 38 deletions
|
|
@ -49,6 +49,7 @@ import io.modelcontextprotocol.spec.McpSchema.Root;
|
|||
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 @@ import static org.mockito.Mockito.mock;
|
|||
|
||||
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 @@ public abstract class AbstractMcpClientServerIntegrationTests {
|
|||
}
|
||||
}
|
||||
|
||||
// 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) {
|
||||
|
|
|
|||
|
|
@ -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.McpServer.SyncSpecification;
|
|||
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 @@ class HttpServletSseIntegrationTests extends AbstractMcpClientServerIntegrationT
|
|||
.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 @@ class HttpServletSseIntegrationTests extends AbstractMcpClientServerIntegrationT
|
|||
}
|
||||
}
|
||||
|
||||
@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<HttpResponse<java.io.InputStream>>();
|
||||
var sessionIdFuture = new java.util.concurrent.CompletableFuture<String>();
|
||||
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<? super ByteBuffer> 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<HttpServletRequest> TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext
|
||||
.create(Map.of("important", "value"));
|
||||
|
||||
|
|
|
|||
|
|
@ -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.ServerCapabilities;
|
|||
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.junit.jupiter.api.Timeout;
|
|||
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 @@ class HttpServletStatelessIntegrationTests {
|
|||
"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 @@ class HttpServletStatelessIntegrationTests {
|
|||
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<? super ByteBuffer> 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) {
|
||||
|
|
|
|||
|
|
@ -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.HttpServletStreamableServerTrans
|
|||
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 @@ class HttpServletStreamableIntegrationTests extends AbstractMcpClientServerInteg
|
|||
.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 @@ class HttpServletStreamableIntegrationTests extends AbstractMcpClientServerInteg
|
|||
static McpTransportContextExtractor<HttpServletRequest> 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<? super ByteBuffer> 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);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue