Skip to content

Commit 100e402

Browse files
lzcGeeklzcGeek
authored andcommitted
fix: stop truncating SSE field values containing U+2028/U+2029/U+0085
1 parent c7fef64 commit 100e402

2 files changed

Lines changed: 192 additions & 42 deletions

File tree

‎mcp-core/src/main/java/io/modelcontextprotocol/client/transport/ResponseSubscribers.java‎

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import java.util.concurrent.CompletionStage;
1414
import java.util.concurrent.Flow;
1515
import java.util.concurrent.atomic.AtomicReference;
16-
import java.util.regex.Pattern;
1716

1817
import org.reactivestreams.FlowAdapters;
1918
import org.reactivestreams.Subscription;
@@ -147,21 +146,6 @@ static BodyHandler<String> boundedStringBodyHandler(int maxSize) {
147146

148147
static class SseLineSubscriber extends BaseSubscriber<String> {
149148

150-
/**
151-
* Pattern to extract data content from SSE "data:" lines.
152-
*/
153-
private static final Pattern EVENT_DATA_PATTERN = Pattern.compile("^data:(.+)$", Pattern.MULTILINE);
154-
155-
/**
156-
* Pattern to extract event ID from SSE "id:" lines.
157-
*/
158-
private static final Pattern EVENT_ID_PATTERN = Pattern.compile("^id:(.+)$", Pattern.MULTILINE);
159-
160-
/**
161-
* Pattern to extract event type from SSE "event:" lines.
162-
*/
163-
private static final Pattern EVENT_TYPE_PATTERN = Pattern.compile("^event:(.+)$", Pattern.MULTILINE);
164-
165149
/**
166150
* The sink for emitting parsed response events.
167151
*/
@@ -227,49 +211,75 @@ protected void hookOnSubscribe(Subscription subscription) {
227211
});
228212
}
229213

214+
/**
215+
* Extracts the value of an SSE field from a line, per the <a href=
216+
* "https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation">
217+
* SSE specification</a>: the characters after the colon with a single leading
218+
* space removed.
219+
*
220+
* <p>
221+
* A value may legally contain U+2028 (LINE SEPARATOR), U+2029 (PARAGRAPH
222+
* SEPARATOR) and U+0085 (NEXT LINE). Those are not SSE line terminators, so
223+
* extracting the value with a {@code MULTILINE} regex instead of this method
224+
* silently truncates it there.
225+
* @param line the SSE line, already stripped of its terminator by the line
226+
* subscriber
227+
* @param field the field prefix, e.g. {@code "data:"}
228+
* @return the field value with a single leading space removed, never truncated
229+
* @see #hookOnNext(String)
230+
*/
231+
private static String fieldValue(String line, String field) {
232+
String value = line.substring(field.length());
233+
if (value.startsWith(" ")) {
234+
value = value.substring(1);
235+
}
236+
return value;
237+
}
238+
239+
/**
240+
* Returns the buffered data lines joined by the separators the data: handler
241+
* appended, with only the final separator removed. Trimming the whole buffer
242+
* instead would also strip significant leading/trailing whitespace from the first
243+
* and last data lines, which the SSE field rules explicitly preserve.
244+
*/
245+
private String concatenatedDataLines() {
246+
String buffered = this.eventBuilder.toString();
247+
return buffered.substring(0, buffered.length() - 1);
248+
}
249+
230250
@Override
231251
protected void hookOnNext(String line) {
232252
if (line.isEmpty()) {
233253
// Empty line means end of event
234254
if (this.eventBuilder.length() > 0) {
235-
String eventData = this.eventBuilder.toString();
236-
SseEvent sseEvent = new SseEvent(currentEventId.get(), currentEventType.get(), eventData.trim());
255+
String eventData = concatenatedDataLines();
256+
SseEvent sseEvent = new SseEvent(currentEventId.get(), currentEventType.get(), eventData);
237257

238258
this.sink.next(new SseResponseEvent(responseInfo, sseEvent));
239259
this.eventBuilder.setLength(0);
240260
}
241261
}
242262
else {
243263
if (line.startsWith("data:")) {
244-
var matcher = EVENT_DATA_PATTERN.matcher(line);
245-
if (matcher.find()) {
246-
String data = matcher.group(1).trim();
247-
// Measured before appending, so that an event carrying exactly
248-
// maxSize of data is accepted: the trailing separator below is
249-
// stripped again before the event is emitted.
250-
if (this.eventBuilder.length() + data.length() > this.maxSize) {
251-
upstream().cancel();
252-
this.sink.error(
253-
new McpTransportException("Inbound SSE event exceeds the maximum allowed size of "
254-
+ this.maxSize + " bytes"));
255-
return;
256-
}
257-
this.eventBuilder.append(data).append("\n");
264+
String data = fieldValue(line, "data:");
265+
// Measured before appending, so that an event carrying exactly
266+
// maxSize of data is accepted: the trailing separator below is
267+
// stripped again before the event is emitted.
268+
if (this.eventBuilder.length() + data.length() > this.maxSize) {
269+
upstream().cancel();
270+
this.sink.error(new McpTransportException(
271+
"Inbound SSE event exceeds the maximum allowed size of " + this.maxSize + " bytes"));
272+
return;
258273
}
274+
this.eventBuilder.append(data).append("\n");
259275
upstream().request(1);
260276
}
261277
else if (line.startsWith("id:")) {
262-
var matcher = EVENT_ID_PATTERN.matcher(line);
263-
if (matcher.find()) {
264-
this.currentEventId.set(matcher.group(1).trim());
265-
}
278+
this.currentEventId.set(fieldValue(line, "id:"));
266279
upstream().request(1);
267280
}
268281
else if (line.startsWith("event:")) {
269-
var matcher = EVENT_TYPE_PATTERN.matcher(line);
270-
if (matcher.find()) {
271-
this.currentEventType.set(matcher.group(1).trim());
272-
}
282+
this.currentEventType.set(fieldValue(line, "event:"));
273283
upstream().request(1);
274284
}
275285
else if (line.startsWith(":")) {
@@ -290,8 +300,8 @@ else if (line.startsWith(":")) {
290300
@Override
291301
protected void hookOnComplete() {
292302
if (this.eventBuilder.length() > 0) {
293-
String eventData = this.eventBuilder.toString();
294-
SseEvent sseEvent = new SseEvent(currentEventId.get(), currentEventType.get(), eventData.trim());
303+
String eventData = concatenatedDataLines();
304+
SseEvent sseEvent = new SseEvent(currentEventId.get(), currentEventType.get(), eventData);
295305
this.sink.next(new SseResponseEvent(responseInfo, sseEvent));
296306
}
297307
this.sink.complete();
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
* Copyright 2024-2026 the original author or authors.
3+
*/
4+
5+
package io.modelcontextprotocol.client.transport;
6+
7+
import java.net.http.HttpClient;
8+
import java.net.http.HttpHeaders;
9+
import java.net.http.HttpResponse;
10+
import java.util.List;
11+
import java.util.Map;
12+
13+
import org.junit.jupiter.api.Test;
14+
15+
import reactor.core.publisher.Flux;
16+
import reactor.test.StepVerifier;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
/**
21+
* Unit tests for {@link ResponseSubscribers.SseLineSubscriber}.
22+
*
23+
* <p>
24+
* Verifies that SSE field values are extracted per the <a href=
25+
* "https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation">
26+
* WHATWG HTML Living Standard §9.2.6</a>: the field value is everything after the colon
27+
* minus a single leading space. In particular, U+2028 (LINE SEPARATOR), U+2029 (PARAGRAPH
28+
* SEPARATOR) and U+0085 (NEXT LINE) are legal inside a field value and must not truncate
29+
* it — they are not SSE line terminators.
30+
*
31+
* @see <a href="https://github.com/modelcontextprotocol/java-sdk/issues/1136">#1136</a>
32+
*/
33+
class ResponseSubscribersTest {
34+
35+
private static final HttpResponse.ResponseInfo RESPONSE_INFO = new HttpResponse.ResponseInfo() {
36+
37+
@Override
38+
public int statusCode() {
39+
return 200;
40+
}
41+
42+
@Override
43+
public HttpHeaders headers() {
44+
return HttpHeaders.of(Map.of(), (name, value) -> true);
45+
}
46+
47+
@Override
48+
public HttpClient.Version version() {
49+
return HttpClient.Version.HTTP_1_1;
50+
}
51+
52+
};
53+
54+
private static List<ResponseSubscribers.SseEvent> parse(List<String> lines) {
55+
return Flux.<ResponseSubscribers.ResponseEvent>create(sink -> Flux.fromIterable(lines)
56+
.subscribe(new ResponseSubscribers.SseLineSubscriber(RESPONSE_INFO, sink, Integer.MAX_VALUE)))
57+
.map(event -> ((ResponseSubscribers.SseResponseEvent) event).sseEvent())
58+
.collectList()
59+
.block();
60+
}
61+
62+
/**
63+
* A {@code data:} payload containing U+2028, U+2029 or U+0085 must survive parsing
64+
* intact. A MULTILINE regex used to truncate the value at those characters, because
65+
* the Java regex engine treats them as line terminators.
66+
*/
67+
@Test
68+
void shouldNotTruncateDataAtUnicodeLineSeparators() {
69+
List<String> separators = List.of("\u2028", "\u2029", "\u0085");
70+
71+
for (String separator : separators) {
72+
String payload = "{\"text\":\"a" + separator + "b\"}";
73+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: " + payload, ""));
74+
75+
assertThat(events).as("payload with U+%04X", (int) separator.charAt(0)).hasSize(1);
76+
assertThat(events.get(0).data()).isEqualTo(payload);
77+
}
78+
}
79+
80+
@Test
81+
void shouldPreserveVerticalTabInData() {
82+
String payload = "{\"text\":\"a\u000Bb\"}";
83+
84+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: " + payload, ""));
85+
86+
assertThat(events).hasSize(1);
87+
assertThat(events.get(0).data()).isEqualTo(payload);
88+
}
89+
90+
@Test
91+
void shouldStripOnlySingleLeadingSpacePerDataLine() {
92+
// The leading space after the colon is stripped per line; any further
93+
// whitespace is part of the value.
94+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: first", "data: second", ""));
95+
96+
assertThat(events).hasSize(1);
97+
assertThat(events.get(0).data()).isEqualTo("first\n second");
98+
}
99+
100+
/**
101+
* Only the separator the {@code data:} handler appended after the last line may be
102+
* removed when the event is dispatched; trimming the whole buffer would also strip
103+
* significant whitespace from the first and last data lines.
104+
*/
105+
@Test
106+
void shouldNotTrimSignificantWhitespaceOfSingleDataLine() {
107+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: padded ", ""));
108+
109+
assertThat(events).hasSize(1);
110+
assertThat(events.get(0).data()).isEqualTo(" padded ");
111+
}
112+
113+
@Test
114+
void shouldPreserveLeadingAndTrailingWhitespaceOfFirstAndLastDataLines() {
115+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: first", "data: last ", ""));
116+
117+
assertThat(events).hasSize(1);
118+
assertThat(events.get(0).data()).isEqualTo(" first\nlast ");
119+
}
120+
121+
@Test
122+
void shouldJoinMultipleDataLinesWithNewline() {
123+
List<ResponseSubscribers.SseEvent> events = parse(List.of("data: first", "data: second", ""));
124+
125+
assertThat(events).hasSize(1);
126+
assertThat(events.get(0).data()).isEqualTo("first\nsecond");
127+
}
128+
129+
@Test
130+
void shouldCaptureEventIdAndTypeWithUnicodeValue() {
131+
List<ResponseSubscribers.SseEvent> events = parse(
132+
List.of("event: message\u2028tail", "id: 42\u2028tail", "data: body", ""));
133+
134+
assertThat(events).hasSize(1);
135+
assertThat(events.get(0).event()).isEqualTo("message\u2028tail");
136+
assertThat(events.get(0).id()).isEqualTo("42\u2028tail");
137+
assertThat(events.get(0).data()).isEqualTo("body");
138+
}
139+
140+
}

0 commit comments

Comments
 (0)