This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 987b54b0d42 Handle MCP servlet exceptions safely (#39732)
987b54b0d42 is described below
commit 987b54b0d42b33021a18ec981f357925a59d9fc8
Author: Liang Zhang <[email protected]>
AuthorDate: Sat Sep 5 12:30:27 2026 +0800
Handle MCP servlet exceptions safely (#39732)
---
.../server/http/StreamableHttpMCPServlet.java | 9 +++
.../server/http/StreamableHttpMCPServletTest.java | 66 ++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git
a/mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServlet.java
b/mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServlet.java
index f7bb40275c2..571ba12d75d 100644
---
a/mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServlet.java
+++
b/mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServlet.java
@@ -31,6 +31,7 @@ import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponseWrapper;
+import lombok.extern.slf4j.Slf4j;
import org.apache.shardingsphere.infra.exception.ShardingSpherePreconditions;
import org.apache.shardingsphere.mcp.api.session.MCPSessionIdentity;
import
org.apache.shardingsphere.mcp.bootstrap.config.HttpTransportConfiguration;
@@ -56,6 +57,7 @@ import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.StampedLock;
+@Slf4j
final class StreamableHttpMCPServlet extends HttpServlet implements
McpStreamableServerTransportProvider {
private static final long serialVersionUID = -2320345528569140021L;
@@ -141,6 +143,13 @@ final class StreamableHttpMCPServlet extends HttpServlet
implements McpStreamabl
}
try {
super.service(request, response);
+ } catch (final IOException | ServletException ex) {
+ if (response.isCommitted()) {
+ throw ex;
+ }
+ log.error("Unexpected MCP HTTP request failure.", ex);
+ response.reset();
+ response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
} finally {
lifecycleLock.unlockRead(lifecycleStamp);
}
diff --git
a/mcp/bootstrap/src/test/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServletTest.java
b/mcp/bootstrap/src/test/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServletTest.java
index 548b6b42173..f428cdde09b 100644
---
a/mcp/bootstrap/src/test/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServletTest.java
+++
b/mcp/bootstrap/src/test/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/StreamableHttpMCPServletTest.java
@@ -58,6 +58,7 @@ import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -65,6 +66,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.never;
@@ -198,6 +200,39 @@ class StreamableHttpMCPServletTest {
verify(response).setCharacterEncoding("UTF-8");
}
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("requestMethods")
+ void assertServiceHandleUncommittedException(final String name, final
String requestMethod) throws IOException {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn(requestMethod);
+
when(request.getHeaderNames()).thenReturn(Collections.enumeration(List.of("Origin")));
+
when(request.getHeaders("Origin")).thenReturn(Collections.enumeration(List.of("https://example.com";)));
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ doThrow(new IOException("foo_failure")).when(response).getWriter();
+ StreamableHttpMCPServlet actual =
createServlet(mock(MCPSessionManager.class));
+ assertDoesNotThrow(() -> actual.service(request, response));
+ verify(response).reset();
+
verify(response).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ }
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("requestMethods")
+ void assertServiceRethrowCommittedException(final String name, final
String requestMethod) throws IOException {
+ IOException expectedException = new IOException("foo_failure");
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn(requestMethod);
+
when(request.getHeaderNames()).thenReturn(Collections.enumeration(List.of("Origin")));
+
when(request.getHeaders("Origin")).thenReturn(Collections.enumeration(List.of("https://example.com";)));
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ when(response.isCommitted()).thenReturn(true);
+ doThrow(expectedException).when(response).getWriter();
+ StreamableHttpMCPServlet actual =
createServlet(mock(MCPSessionManager.class));
+ IOException actualException = assertThrows(IOException.class, () ->
actual.service(request, response));
+ assertThat(actualException, sameInstance(expectedException));
+ verify(response, never()).reset();
+ verify(response,
never()).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ }
+
private static Stream<Arguments> requestMethods() {
return Stream.of(
Arguments.of("get request", "GET"),
@@ -331,6 +366,22 @@ class StreamableHttpMCPServletTest {
assertFalse(sessionManager.hasSession("session-id"));
}
+ @Test
+ void assertServicePostRollbackSessionWhenDelegateFails() throws
ServletException, IOException {
+ MCPSessionManager sessionManager = new MCPSessionManager(Map.of());
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn("POST");
+
when(request.getHeaderNames()).thenReturn(Collections.emptyEnumeration());
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StreamableHttpMCPServlet actual = createServlet(sessionManager);
+ doAnswer(invocation -> {
+ ((HttpServletResponse)
invocation.getArgument(1)).setHeader(HttpHeaders.MCP_SESSION_ID, "session-id");
+ throw new ServletException("foo_failure");
+ }).when(getDelegate()).service(any(HttpServletRequest.class),
any(HttpServletResponse.class));
+ assertDoesNotThrow(() -> actual.service(request, response));
+ assertFalse(sessionManager.hasSession("session-id"));
+ }
+
@Test
void assertServicePostWithJsonContentType() throws ServletException,
IOException {
HttpServletRequest request = mock(HttpServletRequest.class);
@@ -462,6 +513,21 @@ class StreamableHttpMCPServletTest {
assertTrue(sessionManager.hasSession("session-id"));
}
+ @Test
+ void assertServiceDeletePreserveSessionWhenDelegateFails() throws
ServletException, IOException {
+ MCPSessionManager sessionManager = new
MCPSessionManager(Collections.emptyMap());
+ sessionManager.createSession(new MCPSessionIdentity("session-id", "",
"", Map.of()));
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getMethod()).thenReturn("DELETE");
+
when(request.getHeaderNames()).thenReturn(Collections.emptyEnumeration());
+
when(request.getHeader(HttpHeaders.MCP_SESSION_ID)).thenReturn("session-id");
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StreamableHttpMCPServlet actual = createServlet(sessionManager);
+ doThrow(new
ServletException("foo_failure")).when(getDelegate()).service(any(HttpServletRequest.class),
any(HttpServletResponse.class));
+ assertDoesNotThrow(() -> actual.service(request, response));
+ assertTrue(sessionManager.hasSession("session-id"));
+ }
+
@Test
void assertCloseGracefully() {
MCPSessionManager sessionManager = new
MCPSessionManager(Collections.emptyMap());
