Fix SSE event classification to follow spec for missing event field (#913)
Per the SSE specification (WHATWG HTML Living Standard 9.2.6), an event with no explicit event field MUST be dispatched as a message event. HttpClientStreamableHttpTransport previously used strict equality and silently dropped such frames in the reconnect/GET stream path, causing server-initiated notifications to never reach the handler. Extract classification into a package-private isMessageEvent helper and cover with parameterized unit tests. Closes gh-885
This commit is contained in:
parent
fd004989b9
commit
f5aab36a2f
2 changed files with 75 additions and 1 deletions
|
|
@ -114,6 +114,29 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport {
|
|||
|
||||
public static int BAD_REQUEST = 400;
|
||||
|
||||
/**
|
||||
* Determines whether an SSE event should be treated as a "message" event carrying a
|
||||
* JSON-RPC payload.
|
||||
*
|
||||
* <p>
|
||||
* Per the <a href=
|
||||
* "https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation">
|
||||
* SSE specification (WHATWG HTML Living Standard §9.2.6)</a>, an event with no
|
||||
* explicit {@code event:} field MUST be dispatched as a {@code message} event by
|
||||
* default. This method applies that rule by treating {@code null} or empty event
|
||||
* names as equivalent to {@link #MESSAGE_EVENT_TYPE}.
|
||||
*
|
||||
* <p>
|
||||
* This alignment ensures interoperability with MCP servers that emit bare
|
||||
* {@code data:} frames without an accompanying {@code event:} line, which are valid
|
||||
* per the SSE spec.
|
||||
* @param eventName the SSE event name, which may be {@code null} or empty
|
||||
* @return {@code true} if the event should be parsed as a JSON-RPC message
|
||||
*/
|
||||
static boolean isMessageEvent(String eventName) {
|
||||
return eventName == null || eventName.isEmpty() || MESSAGE_EVENT_TYPE.equals(eventName);
|
||||
}
|
||||
|
||||
private final McpJsonMapper jsonMapper;
|
||||
|
||||
private final URI baseUri;
|
||||
|
|
@ -323,7 +346,7 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport {
|
|||
+ statusCode));
|
||||
}
|
||||
else if (statusCode >= 200 && statusCode < 300) {
|
||||
if (MESSAGE_EVENT_TYPE.equals(sseResponseEvent.sseEvent().event())) {
|
||||
if (isMessageEvent(sseResponseEvent.sseEvent().event())) {
|
||||
String data = sseResponseEvent.sseEvent().data();
|
||||
// Per 2025-11-25 spec (SEP-1699), servers may
|
||||
// send SSE events
|
||||
|
|
|
|||
|
|
@ -0,0 +1,51 @@
|
|||
/*
|
||||
* Copyright 2024-2026 the original author or authors.
|
||||
*/
|
||||
|
||||
package io.modelcontextprotocol.client.transport;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.NullAndEmptySource;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link HttpClientStreamableHttpTransport#isMessageEvent(String)}.
|
||||
*
|
||||
* <p>
|
||||
* Verifies that SSE event classification follows the <a href=
|
||||
* "https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation">
|
||||
* WHATWG HTML Living Standard §9.2.6</a>: an event without an explicit {@code event:}
|
||||
* field must be dispatched as a {@code message} event.
|
||||
*
|
||||
* @author jiajingda
|
||||
* @see <a href="https://github.com/modelcontextprotocol/java-sdk/issues/885">#885</a>
|
||||
*/
|
||||
class HttpClientStreamableHttpTransportSseEventTypeTest {
|
||||
|
||||
@ParameterizedTest
|
||||
@NullAndEmptySource
|
||||
void shouldTreatNullOrEmptyEventAsMessage(String eventName) {
|
||||
assertThat(HttpClientStreamableHttpTransport.isMessageEvent(eventName))
|
||||
.as("SSE frame with null/empty event field must be treated as a 'message' event per SSE spec")
|
||||
.isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldTreatExplicitMessageEventAsMessage() {
|
||||
assertThat(HttpClientStreamableHttpTransport.isMessageEvent("message"))
|
||||
.as("Explicit 'message' event must be parsed as a JSON-RPC message")
|
||||
.isTrue();
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(strings = { "ping", "error", "notification", "MESSAGE", "Message", "custom-event" })
|
||||
void shouldNotTreatOtherEventsAsMessage(String eventName) {
|
||||
assertThat(HttpClientStreamableHttpTransport.isMessageEvent(eventName))
|
||||
.as("Non-'message' SSE event '%s' must not be parsed as a JSON-RPC message", eventName)
|
||||
.isFalse();
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in a new issue