Skip to content

Commit 68f6db8

Browse files
KehrlannKehrlann
committed
Reject non application/json POST requests
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
1 parent 61d1987 commit 68f6db8

8 files changed

Lines changed: 265 additions & 2 deletions

File tree

‎mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtils.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
*/
2424
final class HttpServletRequestUtils {
2525

26+
private static final String APPLICATION_JSON = "application/json";
27+
2628
private HttpServletRequestUtils() {
2729
}
2830

@@ -41,6 +43,27 @@ static Map<String, List<String>> extractHeaders(HttpServletRequest request) {
4143
return headers;
4244
}
4345

46+
/**
47+
* Checks whether a {@code Content-Type} header value denotes
48+
* {@code application/json}. Only the media type is compared, case-insensitively;
49+
* parameters such as {@code charset} are ignored. This is not a substring search, so
50+
* a value like {@code text/plain; a=application/json} is rejected.
51+
* <p>
52+
* Requiring {@code application/json} prevents browsers from sending cross-origin
53+
* JSON-RPC messages as CORS "simple requests" (e.g. with {@code text/plain}), which
54+
* would otherwise reach the server without a preflight.
55+
* @param contentType The {@code Content-Type} header value, may be {@code null}
56+
* @return {@code true} if the media type is {@code application/json}
57+
*/
58+
static boolean isJsonContentType(String contentType) {
59+
if (contentType == null) {
60+
return false;
61+
}
62+
int parametersStart = contentType.indexOf(';');
63+
String mediaType = parametersStart == -1 ? contentType : contentType.substring(0, parametersStart);
64+
return APPLICATION_JSON.equalsIgnoreCase(mediaType.trim());
65+
}
66+
4467
/**
4568
* Reads the request body, decoded using the request's character encoding (or UTF-8 if
4669
* not specified), while bounding the number of bytes read.

‎mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,14 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
373373
return;
374374
}
375375

376+
if (!HttpServletRequestUtils.isJsonContentType(request.getContentType())) {
377+
this.responseError(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE,
378+
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
379+
.message("Unsupported Media Type: Content-Type must be application/json")
380+
.build());
381+
return;
382+
}
383+
376384
// Get the session ID from the request parameter
377385
String sessionId = request.getParameter("sessionId");
378386
if (sessionId == null) {
@@ -477,6 +485,16 @@ private void sendEvent(PrintWriter writer, String eventType, String data) throws
477485
}
478486
}
479487

488+
private void responseError(HttpServletResponse response, int httpCode, McpError mcpError) throws IOException {
489+
response.setContentType(APPLICATION_JSON);
490+
response.setCharacterEncoding(UTF_8);
491+
response.setStatus(httpCode);
492+
String jsonError = jsonMapper.writeValueAsString(mcpError);
493+
PrintWriter writer = response.getWriter();
494+
writer.write(jsonError);
495+
writer.flush();
496+
}
497+
480498
/**
481499
* Cleans up resources when the servlet is being destroyed.
482500
* <p>

‎mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,13 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
170170
return;
171171
}
172172

173-
McpTransportContext transportContext = this.contextExtractor.extract(request);
173+
if (!HttpServletRequestUtils.isJsonContentType(request.getContentType())) {
174+
this.responseError(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE,
175+
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
176+
.message("Unsupported Media Type: Content-Type must be application/json")
177+
.build());
178+
return;
179+
}
174180

175181
String accept = request.getHeader(ACCEPT);
176182
if (accept == null || !(accept.contains(APPLICATION_JSON) && accept.contains(TEXT_EVENT_STREAM))) {
@@ -181,6 +187,8 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
181187
return;
182188
}
183189

190+
McpTransportContext transportContext = this.contextExtractor.extract(request);
191+
184192
try {
185193
String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize);
186194

‎mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,14 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
440440
badRequestErrors.add("application/json required in Accept header");
441441
}
442442

443+
if (!HttpServletRequestUtils.isJsonContentType(request.getContentType())) {
444+
this.responseError(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE,
445+
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
446+
.message("Unsupported Media Type: Content-Type must be application/json")
447+
.build());
448+
return;
449+
}
450+
443451
McpTransportContext transportContext = this.contextExtractor.extract(request);
444452

445453
try {

‎mcp-core/src/test/java/io/modelcontextprotocol/server/transport/HttpServletRequestUtilsTests.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
import jakarta.servlet.ServletInputStream;
1313
import jakarta.servlet.http.HttpServletRequest;
1414
import org.junit.jupiter.api.Test;
15+
import org.junit.jupiter.params.ParameterizedTest;
16+
import org.junit.jupiter.params.provider.NullAndEmptySource;
17+
import org.junit.jupiter.params.provider.ValueSource;
1518

1619
import static org.assertj.core.api.Assertions.assertThat;
1720
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -100,6 +103,22 @@ void honorsRequestCharacterEncoding() throws Exception {
100103
assertThat(body).isEqualTo("café");
101104
}
102105

106+
@ParameterizedTest
107+
@ValueSource(strings = { "application/json", "application/json; charset=utf-8", "application/json;charset=UTF-8",
108+
"Application/JSON", " application/json ; charset=utf-8" })
109+
void acceptsJsonContentType(String contentType) {
110+
assertThat(HttpServletRequestUtils.isJsonContentType(contentType)).isTrue();
111+
}
112+
113+
@ParameterizedTest
114+
@NullAndEmptySource
115+
@ValueSource(strings = { "text/plain", "text/plain;charset=UTF-8", "text/plain; a=application/json",
116+
"application/x-www-form-urlencoded", "multipart/form-data", "application/json-seq", "application/jsonp",
117+
"application/json, text/plain", "text/event-stream" })
118+
void rejectsNonJsonContentType(String contentType) {
119+
assertThat(HttpServletRequestUtils.isJsonContentType(contentType)).isFalse();
120+
}
121+
103122
private static HttpServletRequest requestWithBody(String body, String characterEncoding) throws IOException {
104123
HttpServletRequest request = mock(HttpServletRequest.class);
105124
when(request.getInputStream()).thenReturn(servletInputStream(body.getBytes(StandardCharsets.UTF_8)));

‎mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
package io.modelcontextprotocol.server;
66

7+
import java.io.BufferedReader;
8+
import java.io.InputStream;
9+
import java.io.InputStreamReader;
710
import java.net.URI;
811
import java.net.http.HttpClient;
912
import java.net.http.HttpRequest;
@@ -12,6 +15,9 @@
1215
import java.nio.charset.StandardCharsets;
1316
import java.time.Duration;
1417
import java.util.Map;
18+
import java.util.concurrent.CompletableFuture;
19+
import java.util.concurrent.TimeUnit;
20+
import java.util.concurrent.atomic.AtomicBoolean;
1521
import java.util.stream.Stream;
1622

1723
import io.modelcontextprotocol.AbstractMcpClientServerIntegrationTests;
@@ -22,6 +28,7 @@
2228
import io.modelcontextprotocol.server.McpServer.SyncSpecification;
2329
import io.modelcontextprotocol.server.transport.HttpServletSseServerTransportProvider;
2430
import io.modelcontextprotocol.server.transport.TomcatTestUtil;
31+
import io.modelcontextprotocol.spec.McpSchema;
2532
import jakarta.servlet.http.HttpServletRequest;
2633
import jakarta.servlet.http.HttpServletResponse;
2734
import org.apache.catalina.LifecycleException;
@@ -31,8 +38,12 @@
3138
import org.junit.jupiter.api.BeforeEach;
3239
import org.junit.jupiter.api.Test;
3340
import org.junit.jupiter.api.Timeout;
41+
import org.junit.jupiter.params.ParameterizedTest;
3442
import org.junit.jupiter.params.provider.Arguments;
43+
import org.junit.jupiter.params.provider.ValueSource;
44+
import reactor.core.publisher.Mono;
3545

46+
import static io.modelcontextprotocol.util.ToolsUtils.EMPTY_JSON_SCHEMA;
3647
import static org.assertj.core.api.Assertions.assertThat;
3748

3849
@Timeout(15)
@@ -181,6 +192,55 @@ public void cancel() {
181192
assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
182193
}
183194

195+
@ParameterizedTest
196+
@ValueSource(strings = { "text/plain;charset=UTF-8", "application/x-www-form-urlencoded", "multipart/form-data" })
197+
void rejectsNonJsonContentType(String contentType) throws Exception {
198+
var httpClient = HttpClient.newHttpClient();
199+
var toolCalled = new AtomicBoolean();
200+
prepareAsyncServerBuilder().capabilities(McpSchema.ServerCapabilities.builder().tools(true).build())
201+
.tools(McpServerFeatures.AsyncToolSpecification.builder()
202+
.tool(McpSchema.Tool.builder().name("tool1").inputSchema(EMPTY_JSON_SCHEMA).build())
203+
.callHandler((exchange, request) -> {
204+
toolCalled.set(true);
205+
return Mono.just(McpSchema.CallToolResult.builder().build());
206+
})
207+
.build())
208+
.build();
209+
210+
// Establish an SSE session to obtain a valid session ID
211+
var sseRequest = HttpRequest.newBuilder()
212+
.uri(URI.create("http://localhost:" + PORT + CUSTOM_SSE_ENDPOINT))
213+
.header("Accept", "text/event-stream")
214+
.GET()
215+
.build();
216+
HttpResponse<InputStream> sseResponse = httpClient.send(sseRequest, HttpResponse.BodyHandlers.ofInputStream());
217+
try (var reader = new BufferedReader(new InputStreamReader(sseResponse.body(), StandardCharsets.UTF_8))) {
218+
var sessionIdFuture = CompletableFuture.supplyAsync(() -> reader.lines()
219+
.filter(line -> line.startsWith("data:") && line.contains("sessionId="))
220+
.map(line -> line.substring(line.indexOf("sessionId=") + "sessionId=".length()).strip())
221+
.findFirst()
222+
.orElseThrow(() -> new IllegalStateException("sessionId not found in SSE stream")));
223+
String sessionId = sessionIdFuture.get(5, TimeUnit.SECONDS);
224+
225+
// CORS-safelisted content types can be sent cross-origin by a browser without
226+
// a
227+
// preflight, so they must be rejected before the message is handled
228+
var request = HttpRequest.newBuilder()
229+
.uri(URI.create("http://localhost:" + PORT + CUSTOM_MESSAGE_ENDPOINT + "?sessionId=" + sessionId))
230+
.header("Content-Type", contentType)
231+
.POST(HttpRequest.BodyPublishers.ofString(
232+
"""
233+
{"jsonrpc":"2.0","id":"call-1","method":"tools/call","params":{"name":"tool1","arguments":{}}}"""))
234+
.build();
235+
236+
var response = httpClient.send(request, HttpResponse.BodyHandlers.ofString());
237+
238+
assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE);
239+
assertThat(response.body()).contains("Unsupported Media Type: Content-Type must be application/json");
240+
assertThat(toolCalled).isFalse();
241+
}
242+
}
243+
184244
static McpTransportContextExtractor<HttpServletRequest> TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext
185245
.create(Map.of("important", "value"));
186246

‎mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.List;
1515
import java.util.Map;
1616
import java.util.concurrent.ConcurrentHashMap;
17+
import java.util.concurrent.atomic.AtomicBoolean;
1718
import java.util.concurrent.atomic.AtomicReference;
1819
import java.util.function.BiFunction;
1920

@@ -787,6 +788,45 @@ public void cancel() {
787788
}
788789
}
789790

791+
@ParameterizedTest
792+
@ValueSource(strings = { "text/plain;charset=UTF-8", "application/x-www-form-urlencoded", "multipart/form-data" })
793+
void rejectsNonJsonContentType(String contentType) throws Exception {
794+
AtomicBoolean toolCalled = new AtomicBoolean();
795+
var mcpServer = McpServer.sync(mcpStatelessServerTransport)
796+
.capabilities(ServerCapabilities.builder().tools(false).build())
797+
.tools(McpStatelessServerFeatures.SyncToolSpecification.builder()
798+
.tool(Tool.builder().name("tool1").inputSchema(EMPTY_JSON_SCHEMA).build())
799+
.callHandler((transportContext, request) -> {
800+
toolCalled.set(true);
801+
return CallToolResult.builder().build();
802+
})
803+
.build())
804+
.build();
805+
806+
try {
807+
// CORS-safelisted content types can be sent cross-origin by a browser without
808+
// a preflight, so they must be rejected before the message is handled
809+
var request = HttpRequest.newBuilder()
810+
.uri(URI.create("http://localhost:" + PORT + CUSTOM_MESSAGE_ENDPOINT))
811+
.header("Content-Type", contentType)
812+
.header("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM)
813+
.POST(HttpRequest.BodyPublishers.ofString(
814+
"""
815+
{"jsonrpc":"2.0","id":"call-1","method":"tools/call","params":{"name":"tool1","arguments":{}}}"""))
816+
.build();
817+
818+
var response = HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString());
819+
820+
assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE);
821+
assertThatJson(response.body()).inPath("message")
822+
.isEqualTo("Unsupported Media Type: Content-Type must be application/json");
823+
assertThat(toolCalled).isFalse();
824+
}
825+
finally {
826+
mcpServer.closeGracefully();
827+
}
828+
}
829+
790830
private double evaluateExpression(String expression) {
791831
// Simple expression evaluator for testing
792832
return switch (expression) {

0 commit comments

Comments
 (0)