modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

fix(core): carry the in-progress SSE frame across chunk boundaries (#2134)

Browse source 
parseSSE declared currentEvent/currentData inside the read loop, so an SSE
frame whose data: line arrived in one chunk and whose terminating blank
line arrived in the next was discarded. buffer already carried a partial
LINE across reads; nothing carried a partial FRAME.

Because this is the web client's transport, the dropped frame is a
JSON-RPC response that never settles: the request hangs forever with no
rejection, no console message, and nothing in the Network tab. The
payload does not arrive corrupted, it does not arrive at all, which is
why it presents as a hang rather than an error.

It has not bitten us because Chromium and Firefox happen to deliver whole
frames at the payload sizes this transport currently sees. That is luck,
not a guarantee: chunk boundaries depend on payload size, TCP
segmentation, and any intermediary. The exposure grows with payload size,
so a large resources/read is the most likely first victim.

The fix hoists both out of the loop and adds an end-of-stream flush — a
server that closes without a final blank line has still delivered a
complete frame, and dropping it would lose the last message of every such
stream.

parseSSE is now exported, solely so the test can reach it. That is the
only way to hit this deterministically: driving a real transport cannot
steer where a chunk boundary lands, but a test that supplies the reader
can put it exactly on the terminator.

The test keeps the two PASSING cases (whole frame, split mid-payload)
alongside the three that failed, because they are what make the diagnosis
precise. This is not "SSE parsing is broken" — mid-payload splits are
handled correctly today, and only the frame-terminator boundary loses
data. Verified against the unfixed code: 3 of 5 fail, all 5 pass after.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-25 23:59:37 -04:00
commit 6cacfa3ec5
2 changed files with 144 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

89
clients/web/src/test/core/mcp/remote/parseSSE.test.ts Normal file
View file

@ -0,0 +1,89 @@
/**
* `parseSSE` cross-chunk frame loss.
*
* The reproduction needs no browser and no network: `parseSSE` takes a reader,
* so a test supplies the chunk boundary directly instead of hoping a real
* transport happens to produce one. That is what makes this defect testable at
* all — on the wire the split is a function of payload size, TCP segmentation
* and any proxy in between, none of which a test can steer.
*
* The bug: `currentEvent` / `currentData` are declared INSIDE the read loop, so
* a frame whose `data:` line arrives in one chunk and whose terminating blank
* line arrives in the next is discarded. `buffer` already carries a partial
* *line* across reads; nothing carries a partial *frame*.
*/
import { describe, expect, it } from "vitest";
import { parseSSE } from "@inspector/core/mcp/remote/remoteClientTransport";
/** A reader that yields exactly the chunks given, in order. */
function readerFrom(chunks: string[]): ReadableStreamDefaultReader<Uint8Array> {
const encoder = new TextEncoder();
let i = 0;
return {
read: async () =>
i < chunks.length
? { done: false as const, value: encoder.encode(chunks[i++]!) }
: { done: true as const, value: undefined },
} as ReadableStreamDefaultReader<Uint8Array>;
}
async function collect(chunks: string[]) {
const out: { event: string; data: string }[] = [];
for await (const frame of parseSSE(readerFrom(chunks))) out.push(frame);
return out;
}
const FRAME = 'event: message\ndata: {"id":1,"result":"payload"}\n\n';
describe("parseSSE chunk boundaries", () => {
it("delivers a frame that arrives whole (the case that hides the bug)", async () => {
// Chromium and Firefox happen to produce this shape at our payload sizes,
// which is why the defect below has never been observed in practice.
expect(await collect([FRAME])).toEqual([
{ event: "message", data: '{"id":1,"result":"payload"}' },
]);
});
it("delivers a frame split between its data line and its terminator", async () => {
// THE BUG. Chunk 1 ends after the data line's newline; chunk 2 carries only
// the blank line that closes the frame. Nothing is malformed — this is one
// valid frame, delivered in two reads.
const split = [
'event: message\ndata: {"id":1,"result":"payload"}\n',
"\n",
];
expect(split.join("")).toBe(FRAME); // same bytes as above, different reads
expect(await collect(split)).toEqual([
{ event: "message", data: '{"id":1,"result":"payload"}' },
]);
});
it("delivers a frame split mid-payload", async () => {
const split = ['event: message\ndata: {"id":1,"res', 'ult":"payload"}\n\n'];
expect(split.join("")).toBe(FRAME);
expect(await collect(split)).toEqual([
{ event: "message", data: '{"id":1,"result":"payload"}' },
]);
});
it("delivers every frame when a burst is split at each boundary", async () => {
// Bytes for three frames, re-cut so each split lands between a data line
// and its terminator — the shape a large response most plausibly produces.
const chunks: string[] = [];
for (const id of [1, 2, 3]) {
chunks.push(`event: message\ndata: {"id":${id}}\n`, "\n");
}
expect(await collect(chunks)).toEqual([
{ event: "message", data: '{"id":1}' },
{ event: "message", data: '{"id":2}' },
{ event: "message", data: '{"id":3}' },
]);
});
it("flushes a final frame when the stream ends without a blank line", async () => {
expect(await collect(['event: message\ndata: {"id":9}\n'])).toEqual([
{ event: "message", data: '{"id":9}' },
]);
});
});

84
core/mcp/remote/remoteClientTransport.ts
View file

@ -165,14 +165,55 @@ function legacySessionId(json: RemoteConnectResponse): string | undefined {
}
/**
* Parse SSE stream from a ReadableStream.
* Yields { event, data } for each SSE message.
* Parse an SSE stream from a ReadableStream.
* Yields `{ event, data }` for each complete SSE message.
*
* ⚠️ **The in-progress event must live OUTSIDE the read loop.** A `read()`
* returns an arbitrary slice of bytes, not a whole frame, so a frame's `data:`
* line can arrive in one chunk and the blank line terminating it in the next.
* Declaring `currentEvent` / `currentData` inside the loop — which this did
* until #2134 — discards the accumulated payload at that boundary: the frame is
* never yielded, the JSON-RPC response it carried never settles, and the caller
* hangs forever with no error on any channel. `buffer` already carries a partial
* *line* across reads; these carry the partial *frame*.
*
* It survived because Chromium and Firefox happen to deliver whole frames at the
* payload sizes this transport sees. That is luck rather than a guarantee —
* chunk boundaries are a function of payload size, TCP segmentation and any
* intermediary — and the exposure grows with payload size, so a large
* `resources/read` is the most likely first victim.
*
* Exported for `clients/web/src/test/core/mcp/remote/parseSSE.test.ts`, which is
* the only way to reach the defect deterministically: driving a real transport
* cannot steer where the chunk boundary lands, but a test that supplies the
* reader can put it exactly on the terminator.
*/
async function* parseSSE(
export async function* parseSSE(
reader: ReadableStreamDefaultReader<Uint8Array>,
): AsyncGenerator<{ event: string; data: string }> {
const decoder = new TextDecoder();
let buffer = "";
// Carried ACROSS reads, not per chunk — see the header.
let currentEvent = "message";
let currentData: string[] = [];
const consume = (line: string): { event: string; data: string } | null => {
if (line.startsWith("event:")) {
currentEvent = line.slice(6).trim();
} else if (line.startsWith("data:")) {
currentData.push(line.slice(5).trimStart());
} else if (line === "") {
const complete =
currentData.length > 0
? { event: currentEvent, data: currentData.join("\n") }
: null;
currentEvent = "message";
currentData = [];
return complete;
}
// Anything else (a `:` keep-alive comment, `id:`, `retry:`) is ignored.
return null;
};
while (true) {
const { done, value } = await reader.read();
@ -183,36 +224,21 @@ async function* parseSSE(
/* v8 ignore next -- String.split always returns a non-empty array, so pop() is never undefined; the ?? "" fallback is unreachable. */
buffer = lines.pop() ?? "";
let currentEvent = "message";
let currentData: string[] = [];
for (const line of lines) {
if (line.startsWith("event:")) {
currentEvent = line.slice(6).trim();
} else if (line.startsWith("data:")) {
currentData.push(line.slice(5).trimStart());
} else if (line === "") {
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
currentEvent = "message";
currentData = [];
}
const complete = consume(line);
if (complete) yield complete;
}
}
if (buffer.trim()) {
const lines = buffer.split("\n");
let currentEvent = "message";
const currentData: string[] = [];
for (const line of lines) {
if (line.startsWith("event:")) currentEvent = line.slice(6).trim();
else if (line.startsWith("data:"))
currentData.push(line.slice(5).trimStart());
}
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
// Stream ended: flush the trailing partial line, then anything still held. A
// server that closes without a final blank line has still delivered a complete
// frame, and dropping it would lose the last message of every such stream.
if (buffer) {
const complete = consume(buffer);
if (complete) yield complete;
}
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
}