Bound HTTP client reads
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
This commit is contained in:
parent
6a41dedaa0
commit
ef037dae7b
5 changed files with 1198 additions and 28 deletions
|
|
@ -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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -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<Void> connect() {
|
||||
return transport().connect(Function.identity());
|
||||
}
|
||||
|
||||
private Mono<Void> sendMessage(HttpClientSseClientTransport transport) {
|
||||
JSONRPCRequest request = new JSONRPCRequest(McpSchema.JSONRPC_VERSION, "test-method", "test-id",
|
||||
Map.of("key", "value"));
|
||||
return transport.sendMessage(request);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -81,7 +81,7 @@ class HttpClientSseClientTransportTests {
|
|||
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() {
|
||||
|
|
|
|||
|
|
@ -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<Void> 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));
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in a new issue