VS Code PR #344094: Why SSE Parsing Stripped the Wrong BOMs
Microsoft VS Code PR #344094 fixes an SSE parser that could turn an unrecognized field into event data by removing a character from its name.
The parser used one TextDecoder object, but ended its decoding session at every completed line. The next line started with fresh byte order mark handling. A U+FEFF character at the beginning of that line could disappear even though it was no longer at the beginning of the stream.
For this input, with the invisible character shown as an escape:
data: first\n\uFEFFdata: second\n\nthe old parser emitted first\nsecond. The expected event contained only first.
The fix keeps decoding state across lines and includes the line terminator in decoding before removing it from the resulting string. Those two choices preserve both field names and malformed UTF-8 handling.
The change merged on October 28, 2026, as commit e01c8bd0f8e5d846fcb3a7747d2f0b01741d4b1e.
The extra character changes the field name
Server-sent events carry text fields such as data, event, id, and retry. Multiple data lines contribute to one event, separated by newlines; a blank line completes the event.
The stream is UTF-8. Its decoding rule removes one leading byte order mark, if present. Later occurrences of U+FEFF remain text.
That gives three different cases:
| Position of U+FEFF | Required treatment |
|---|---|
First character of the stream, before data: | Remove the leading marker. |
Beginning of a later field, before data: | Preserve it in the field name; the name is unrecognized. |
Inside a data: value | Preserve it as payload text. |
In the failing example, the second field is named \uFEFFdata, not data. The stream can be syntactically valid while containing a field the receiver must ignore.
VS Code's processField() already uses exact string matching. The fault occurred earlier: decoding changed the field name before the switch statement received it.
One decoder object can still start many decoding sessions
SSEParser.feed() searches incoming bytes for carriage returns and line feeds. Bytes from a line that spans several chunks are retained in a buffer.
When a complete line becomes available, the old implementation decodes the buffered pieces in streaming mode:
str += this.decoder.decode(buf, { stream: true });It then decodes the final piece without that option:
str += this.decoder.decode(chunk.subarray(offset, index));The missing option marks the end of the decoding session. On the subsequent call, TextDecoder creates fresh decoding state, including a fresh BOM decision.
Reusing the same JavaScript object does not preserve that state when its API has been told the stream ended.
For a later line beginning with the UTF-8 bytes EF BB BF, the decoder can therefore suppress U+FEFF again. The exact-match field parser receives the ordinary string data and appends a value that should have been ignored.
This also happens when the original stream contains no leading BOM. The relevant state records whether the beginning of the decoding stream has already been handled, not whether a marker necessarily appeared there.
The replacement expression has three necessary parts
The patch changes the final decode to:
str += this.decoder.decode(
chunk.subarray(offset, index + 1),
{ stream: true }
).slice(0, -1);stream: true carries the decoder's state into the next line.
index + 1 includes the CR or LF byte that ended the current line.
.slice(0, -1) removes that terminator from the decoded text before processLine() sees it.
The parser still recognizes lines at the byte level. Including the terminator gives the UTF-8 decoder the boundary it needs to finish handling any incomplete sequence immediately before it.
The removed character is a decoded ASCII CR or LF, so removing one UTF-16 code unit does not cut a surrogate pair from the payload.
For CRLF, the existing byte scanner advances past both bytes while the decode includes the CR. If CR and LF arrive in separate chunks, endedOnCR skips the following LF. Those framing rules remain unchanged.
Merely adding streaming mode would introduce another bug
Consider an incomplete UTF-8 sequence at the end of a data line:
ASCII bytes for "data: "
E2
LF
ASCII bytes for "data: next"
LF LFE2 begins a multibyte sequence but has no continuation before the newline. The intended data is a replacement character, then a newline, then next:
\uFFFD\nnextThe old non-streaming decode resolves the unfinished sequence at the end of the line.
Changing only the option to stream: true would retain that unfinished sequence. Because the line scanner removes the newline before decoding, the next bytes visible to the decoder would begin the next field. Error handling could then emit U+FFFD before data, changing that field's name as well.
The merged expression avoids this by decoding the actual newline. An ASCII line terminator cannot continue the pending UTF-8 sequence, so the decoder emits the replacement character and then the terminator. Only the terminator is removed.
The newline triggers error handling for an incomplete sequence. It does not flush or restart the decoder.
There is another useful boundary: an initially blank line. Feeding its terminator advances the stream-start decision even when the line has no text. A BOM-prefixed field after that blank line must not receive the special treatment reserved for the start of the stream.
The new tests cross line and chunk boundaries
The first added test builds a stream containing an optional initial BOM, an unrecognized BOM-prefixed field within an event, another after an event boundary, and a BOM preserved inside a data value.
Its combinations are:
2 initial-BOM choices
× 3 line endings: LF, CRLF, CR
× 4 chunk sizes: 1, 2, 3, entire message
= 24 cases inside one testOne-byte chunks split the three-byte BOM across calls. They also separate CR from LF. The whole-message case verifies that the problem is not dependent on network fragmentation.
The expected output contains exactly two events: first\nlast and \uFEFFkept. Neither BOM-prefixed unknown field contributes data.
The second added test puts a lone 0xE2 before a newline and expects \uFFFD\nnext. It protects the behavior that a superficial streaming-only change would lose.
These are complementary checks: one prevents decoder resets from changing field names; the other prevents preserved decoder state from carrying an unfinished character into the next field.
A local negative control verified both requirements
For this review, I copied the connector-retrieved parser into a standalone TypeScript file and compiled it with TypeScript 5.8.3. I ran the resulting JavaScript on Node 22.16.0 under Linux.
The pre-fix variant reverses only the changed executable line. A third, deliberately incomplete variant enables streaming without including the terminator.
The local harness contains 591 cases: the 24-case BOM matrix, all 499 two-chunk split positions across its six input streams, 36 incomplete-UTF-8 cases, and 32 additional controls.
| Source-isolated variant | Passed | Failed |
|---|---|---|
| Pre-fix decode call | 60 | 531 |
| Streaming enabled, terminator omitted | 551 | 40 |
| Merged decode expression | 591 | 0 |
The deliberately incomplete variant passed the BOM matrix but failed every incomplete-UTF-8 case. For the lone E2 example, it emitted an empty data event instead of \uFFFD\nnext.
The control cases also confirmed that an ordinary BOM inside a data value was preserved by both the old and fixed implementations. The demonstrated regression concerns stream-start handling at line boundaries, not indiscriminate deletion of every U+FEFF character.
These are standalone parser checks, not VS Code's Electron/Mocha suite. The cases deliberately stress related boundaries and are not an estimate of failure frequency in real traffic.
What this review establishes
The production change is one executable expression in the shared parser. Field dispatch, caller APIs, and event-buffer reset logic are unchanged. Three public human approvals are recorded on the final PR head; the earlier automated review explicitly said it had not executed the tests.
The source, specification checks, and local comparison support the fix for stream-wide BOM handling while preserving replacement of incomplete UTF-8 at line endings.
I did not build VS Code, execute its native test runner, or reproduce a user-visible failure over a live network. The PR does not supply a linked production incident or a performance benchmark, and this review does not establish a released editor version containing the change.
For a line-oriented streaming parser, decoder state and event state need separate lifetimes. Event buffers may reset after dispatch. The decoder's stream-start decision must survive every line and event within that same byte stream.
Sources
https://github.com/microsoft/vscode/pull/344094
https://github.com/microsoft/vscode/commit/e01c8bd0f8e5d846fcb3a7747d2f0b01741d4b1e
https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation