Skip to content

Commit 6928451

Browse files
Preserve whitespace in SSE field values
1 parent 98ac45e commit 6928451

2 files changed

Lines changed: 73 additions & 11 deletions

File tree

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ Optional<SseEvent> feed(String line) {
498498
// valueless `data:` line still marks the event as carrying data and gets
499499
// dispatched with empty data. Servers send such an event to prime a
500500
// stream, and dropping it leaves the request it answers hanging.
501-
String value = line.substring(5).trim();
501+
String value = fieldValue(line, 5);
502502
// Measured before appending, so that an event carrying exactly
503503
// maxSize of data is accepted: the trailing separator below is
504504
// stripped again before the event is emitted.
@@ -509,15 +509,15 @@ Optional<SseEvent> feed(String line) {
509509
data.append(value).append('\n');
510510
}
511511
else if (line.startsWith("id:")) {
512-
String value = line.substring(3).trim();
512+
String value = fieldValue(line, 3);
513513
// The spec ignores an id carrying a NULL, and an empty id resets the last
514514
// event ID, which leaves nothing to resume from.
515515
if (value.indexOf('\0') == -1) {
516516
id = value.isEmpty() ? null : value;
517517
}
518518
}
519519
else if (line.startsWith("event:")) {
520-
String value = line.substring(6).trim();
520+
String value = fieldValue(line, 6);
521521
event = value.isEmpty() ? null : value;
522522
}
523523
else if (line.startsWith(":")) {
@@ -531,6 +531,15 @@ else if (line.startsWith(":")) {
531531
return Optional.empty();
532532
}
533533

534+
private static String fieldValue(String line, int offset) {
535+
// SSE removes only one optional U+0020 after the colon; all other
536+
// leading and trailing whitespace belongs to the field value.
537+
if (line.length() > offset && line.charAt(offset) == ' ') {
538+
offset++;
539+
}
540+
return line.substring(offset);
541+
}
542+
534543
/**
535544
* Emits the pending event, if a {@code data:} field was seen, and resets the
536545
* per-event state. The event type is reset even when nothing is dispatched, as
@@ -543,7 +552,9 @@ Optional<SseEvent> flush() {
543552
if (data.isEmpty()) {
544553
return Optional.empty();
545554
}
546-
SseEvent result = new SseEvent(id, type != null ? type : DEFAULT_EVENT_TYPE, data.toString().trim());
555+
// Remove only the final LF appended by the parser, preserving the payload.
556+
SseEvent result = new SseEvent(id, type != null ? type : DEFAULT_EVENT_TYPE,
557+
data.substring(0, data.length() - 1));
547558
data.setLength(0);
548559
return Optional.of(result);
549560
}

‎mcp-core/src/test/java/io/modelcontextprotocol/client/transport/SseEventParserTests.java‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,20 @@
55
package io.modelcontextprotocol.client.transport;
66

77
import java.util.Optional;
8+
import java.util.stream.Stream;
89

910
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.params.ParameterizedTest;
12+
import org.junit.jupiter.params.provider.Arguments;
13+
import org.junit.jupiter.params.provider.MethodSource;
14+
import org.junit.jupiter.params.provider.ValueSource;
1015

1116
import io.modelcontextprotocol.client.transport.ResponseBodyHandlers.SseEvent;
1217
import io.modelcontextprotocol.client.transport.ResponseBodyHandlers.SseEventParser;
18+
import io.modelcontextprotocol.spec.McpTransportException;
1319

1420
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1522

1623
class SseEventParserTests {
1724

@@ -27,13 +34,56 @@ void simpleDataEvent() {
2734
}
2835

2936
@Test
30-
void multiLineDataAccumulatesWithNewlineSeparatorAndTrims() {
37+
void multiLineDataAccumulatesWithNewlineSeparatorAndPreservesWhitespace() {
3138
SseEventParser p = new SseEventParser(Integer.MAX_VALUE);
32-
assertThat(p.feed("data: first")).isEmpty();
33-
assertThat(p.feed("data: second")).isEmpty();
39+
assertThat(p.feed("data: first ")).isEmpty();
40+
assertThat(p.feed("data: \tsecond\t")).isEmpty();
3441
Optional<SseEvent> event = p.feed("");
3542
assertThat(event).isPresent();
36-
assertThat(event.get().data()).isEqualTo("first\nsecond");
43+
assertThat(event.get().data()).isEqualTo(" first \n\tsecond\t");
44+
}
45+
46+
@ParameterizedTest
47+
@MethodSource("fieldValues")
48+
void fieldValuesRemoveAtMostOneLeadingSpace(String input, String expected) {
49+
SseEventParser p = new SseEventParser(Integer.MAX_VALUE);
50+
p.feed("id:" + input);
51+
p.feed("event:" + input);
52+
p.feed("data:" + input);
53+
SseEvent first = p.feed("").orElseThrow();
54+
assertThat(first.id()).isEqualTo(expected);
55+
assertThat(first.event()).isEqualTo(expected);
56+
assertThat(first.data()).isEqualTo(expected);
57+
58+
p.feed("data: next");
59+
SseEvent second = p.feed("").orElseThrow();
60+
assertThat(second.id()).isEqualTo(expected);
61+
assertThat(second.event()).isEqualTo("message");
62+
assertThat(second.data()).isEqualTo("next");
63+
}
64+
65+
static Stream<Arguments> fieldValues() {
66+
return Stream.of(Arguments.of("token", "token"), Arguments.of(" token", "token"),
67+
Arguments.of(" token ", "token "), Arguments.of(" token ", " token "),
68+
Arguments.of("\ttoken\t", "\ttoken\t"), Arguments.of(" \ttoken\t", "\ttoken\t"),
69+
Arguments.of(" ", " "), Arguments.of("\t", "\t"));
70+
}
71+
72+
@Test
73+
void emptyDataLinesPreserveLeadingAndTrailingNewlines() {
74+
SseEventParser p = new SseEventParser(Integer.MAX_VALUE);
75+
p.feed("data:");
76+
p.feed("data: payload");
77+
p.feed("data:");
78+
assertThat(p.feed("").orElseThrow().data()).isEqualTo("\npayload\n");
79+
}
80+
81+
@Test
82+
void preservedWhitespaceCountsTowardsSizeLimit() {
83+
SseEventParser p = new SseEventParser(3);
84+
p.feed("data: a ");
85+
assertThat(p.feed("").orElseThrow().data()).isEqualTo(" a ");
86+
assertThatThrownBy(() -> p.feed("data: a ")).isInstanceOf(McpTransportException.class);
3787
}
3888

3989
@Test
@@ -90,11 +140,12 @@ void emptyIdClearsLastEventId() {
90140
assertThat(p.feed("").orElseThrow().id()).isNull();
91141
}
92142

93-
@Test
94-
void idContainingNullIsIgnored() {
143+
@ParameterizedTest
144+
@ValueSource(strings = { "\0token", "token\0", "to\0ken" })
145+
void idContainingNullIsIgnored(String id) {
95146
SseEventParser p = new SseEventParser(Integer.MAX_VALUE);
96147
p.feed("id: 1");
97-
p.feed("id: 2\u0000" + "3");
148+
p.feed("id: " + id);
98149
p.feed("data: payload");
99150
assertThat(p.feed("").orElseThrow().id()).isEqualTo("1");
100151
}

0 commit comments

Comments
 (0)