From ef56e246c855465d54fcd575c5d2dcb8fc65c296 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Sat, 26 Sep 2026 20:59:25 +0700 Subject: [PATCH] fix(core): parse event-stream retry and id fields per the SSE spec The decoder now accepts any all-ASCII-digit retry (so `retry: 010` is 10, matching browsers, undici and eventsource-parser) and ignores an id that contains U+0000 NULL, keeping the previous id. Values that parse to Infinity are ignored so withEventMeta never rejects decoded output. The encoder, withEventMeta and the peer validator now also reject ids containing NULL: every spec-compliant client drops such ids, and a NULL in a Last-Event-ID header makes fetch throw. --- .../core/src/event-stream/decoder.test.ts | 41 +++++++++++++++---- packages/core/src/event-stream/decoder.ts | 17 +++++--- .../core/src/event-stream/encoder.test.ts | 20 +++++---- packages/core/src/event-stream/encoder.ts | 7 +++- packages/core/src/event-stream/meta.test.ts | 5 ++- packages/core/src/event-stream/types.ts | 2 +- packages/peer/src/validators.test.ts | 1 + 7 files changed, 67 insertions(+), 26 deletions(-) diff --git a/packages/core/src/event-stream/decoder.test.ts b/packages/core/src/event-stream/decoder.test.ts index 4f01de7..6369a1e 100644 --- a/packages/core/src/event-stream/decoder.test.ts +++ b/packages/core/src/event-stream/decoder.test.ts @@ -93,17 +93,40 @@ describe('decodeEventStreamMessage', () => { }) }) - it('accepts only canonical non-negative integer retry values', () => { + it('ignores ids containing U+0000 NULL', () => { + expect(decodeEventStreamMessage('id: \0\n\n')).toEqual({}) + expect(decodeEventStreamMessage('id: a\0b\n\n')).toEqual({}) + expect(decodeEventStreamMessage('id: abc\0\n\n')).toEqual({}) + + // the previous id is kept, and a later valid id still applies + expect(decodeEventStreamMessage('id: 123\nid: a\0b\n\n')).toEqual({ id: '123' }) + expect(decodeEventStreamMessage('id: a\0b\nid: 456\n\n')).toEqual({ id: '456' }) + + // NULL is only disallowed in ids + expect(decodeEventStreamMessage(': a\0b\nevent: a\0b\ndata: a\0b\n\n')).toEqual({ + event: 'a\0b', + data: 'a\0b', + comments: ['a\0b'], + }) + }) + + it('accepts retry values made only of ASCII digits', () => { expect(decodeEventStreamMessage('retry: 0\n\n')).toEqual({ retry: 0 }) + expect(decodeEventStreamMessage('retry: 000\n\n')).toEqual({ retry: 0 }) + expect(decodeEventStreamMessage('retry: 010\n\n')).toEqual({ retry: 10 }) // base 10, not octal + expect(decodeEventStreamMessage(`retry: ${'0'.repeat(400)}7\n\n`)).toEqual({ retry: 7 }) + + // ' 10' has an extra space that survives the single-space strip; '\u0661\u0660' is Arabic-Indic digits + for (const value of ['', 'hello', '1.5', '-1', '+10', '1abc', '1e3', '0x10', 'Infinity', '\u0661\u0660', '10 ', ' 10']) { + expect(decodeEventStreamMessage(`retry: ${value}\n\n`), JSON.stringify(value)).toEqual({}) + } + + // an invalid retry keeps the previous one + expect(decodeEventStreamMessage('retry: 10\nretry: 1.5\n\n')).toEqual({ retry: 10 }) + }) - expect(decodeEventStreamMessage('retry: hello\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: 1.5\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: -1\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: 1abc\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: Infinity\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: 010\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: +10\n\n')).toEqual({}) - expect(decodeEventStreamMessage('retry: 10\n\n')).toEqual({}) // extra space survives the single-space strip + it('ignores retry values too large to parse to a finite number', () => { + expect(decodeEventStreamMessage(`retry: ${'9'.repeat(400)}\n\n`)).toEqual({}) }) }) diff --git a/packages/core/src/event-stream/decoder.ts b/packages/core/src/event-stream/decoder.ts index 2aaffff..1ac09d4 100644 --- a/packages/core/src/event-stream/decoder.ts +++ b/packages/core/src/event-stream/decoder.ts @@ -1,4 +1,5 @@ import type { EventStreamMessage } from './types' +import { isEventStreamMessageId, isEventStreamMessageRetry } from './encoder' import { EventStreamDecoderError } from './error' // A line ending is CR, LF or CRLF. @@ -8,6 +9,9 @@ const LINE_ENDING_REGEX = /\r\n|\r(?!\n)|\n/ const MESSAGE_DELIMITER_REGEX = /(?:\r\n|\r(?!\n)|\n){2,}/g const LEADING_LINE_ENDINGS_REGEX = /^[\r\n]+/ +// JS `\d` matches ASCII digits only, as the spec requires for retry. +const ASCII_DIGITS_REGEX = /^\d+$/ + // Pending text never contains a blank line, so it ends in at most one line // ending ('\r\n'). A delimiter crossing a chunk boundary therefore starts // within its last 2 characters. @@ -50,15 +54,18 @@ export function decodeEventStreamMessage(encoded: string): EventStreamMessage { break case 'id': - message.id = value + // Per spec, an id containing U+0000 NULL is ignored and the previous id is kept. + if (isEventStreamMessageId(value)) { + message.id = value + } break case 'retry': { - const maybeInteger = Number.parseInt(value, 10) + const retry = Number.parseInt(value, 10) - // The round-trip check already rejects NaN, signs, padding and floats. - if (maybeInteger >= 0 && maybeInteger.toString() === value) { - message.retry = maybeInteger + // parseInt returns Infinity for 309+ digits, which withEventMeta would reject. + if (ASCII_DIGITS_REGEX.test(value) && isEventStreamMessageRetry(retry)) { + message.retry = retry } break } diff --git a/packages/core/src/event-stream/encoder.test.ts b/packages/core/src/event-stream/encoder.test.ts index 164c3fd..b9f6bdb 100644 --- a/packages/core/src/event-stream/encoder.test.ts +++ b/packages/core/src/event-stream/encoder.test.ts @@ -29,6 +29,12 @@ describe('predicates', () => { } }) + it('reject ids containing NULL', () => { + expect(isEventStreamMessageId('\0')).toBe(false) + expect(isEventStreamMessageId('a\0b')).toBe(false) + expect(isEventStreamMessageComment('a\0b')).toBe(true) // only ids are ignored by clients + }) + it('reject non-integer or negative retry values', () => { for (const retry of [Number.NaN, -1, 1.5, Number.POSITIVE_INFINITY]) { expect(isEventStreamMessageRetry(retry)).toBe(false) @@ -51,10 +57,10 @@ describe('assertions', () => { expect(() => assertEventStreamMessageRetry(10000)).not.toThrow() }) - it('reject ids containing line breaks', () => { - for (const lineBreak of ['\n', '\r', '\r\n']) { - expect(() => assertEventStreamMessageId(`hi${lineBreak}`)) - .toThrow('Event\'s id must not contain a carriage return or newline character') + it('reject ids containing line breaks or NULL', () => { + for (const char of ['\n', '\r', '\r\n', '\0']) { + expect(() => assertEventStreamMessageId(`hi${char}`)) + .toThrow('Event\'s id must not contain a carriage return, newline or NULL character') } }) @@ -157,9 +163,9 @@ describe('encodeEventStreamMessage', () => { }) it('rejects an invalid id', () => { - for (const lineBreak of ['\n', '\r', '\r\n']) { - expect(() => encodeEventStreamMessage({ event: 'message', id: `hi${lineBreak}` })) - .toThrow('Event\'s id must not contain a carriage return or newline character') + for (const char of ['\n', '\r', '\r\n', '\0']) { + expect(() => encodeEventStreamMessage({ event: 'message', id: `hi${char}` })) + .toThrow('Event\'s id must not contain a carriage return, newline or NULL character') } }) diff --git a/packages/core/src/event-stream/encoder.ts b/packages/core/src/event-stream/encoder.ts index ffa7646..3a81813 100644 --- a/packages/core/src/event-stream/encoder.ts +++ b/packages/core/src/event-stream/encoder.ts @@ -4,12 +4,15 @@ import { EventStreamEncoderError } from './error' const EVENT_STREAM_LINE_ENDING_REGEX = /\r\n|[\n\r]/ const EVENT_STREAM_LINE_ENDING_GLOBAL_REGEX = /\r\n|[\n\r]/g +// Per spec, clients ignore an id containing U+0000 NULL, so it would never reach them. +const EVENT_STREAM_INVALID_ID_CHAR_REGEX = /[\r\n\0]/ + function containsEventStreamLineBreak(value: string): boolean { return EVENT_STREAM_LINE_ENDING_REGEX.test(value) } export function isEventStreamMessageId(maybe: unknown): maybe is string { - return typeof maybe === 'string' && !containsEventStreamLineBreak(maybe) + return typeof maybe === 'string' && !EVENT_STREAM_INVALID_ID_CHAR_REGEX.test(maybe) } export function isEventStreamMessageRetry(maybe: unknown): maybe is number { @@ -22,7 +25,7 @@ export function isEventStreamMessageComment(maybe: unknown): maybe is string { export function assertEventStreamMessageId(id: string): void { if (!isEventStreamMessageId(id)) { - throw new EventStreamEncoderError('Event\'s id must not contain a carriage return or newline character') + throw new EventStreamEncoderError('Event\'s id must not contain a carriage return, newline or NULL character') } } diff --git a/packages/core/src/event-stream/meta.test.ts b/packages/core/src/event-stream/meta.test.ts index e1bfaea..d1b9428 100644 --- a/packages/core/src/event-stream/meta.test.ts +++ b/packages/core/src/event-stream/meta.test.ts @@ -9,8 +9,9 @@ it('get/withEventMeta', () => { expect(getEventMeta(data)).toEqual(undefined) expect(getEventMeta(1)).toEqual(undefined) - expect(() => withEventMeta(data, { id: '123\n' })).toThrow('Event\'s id must not contain a carriage return or newline character') - expect(() => withEventMeta(data, { id: '123\r' })).toThrow('Event\'s id must not contain a carriage return or newline character') + expect(() => withEventMeta(data, { id: '123\n' })).toThrow('Event\'s id must not contain a carriage return, newline or NULL character') + expect(() => withEventMeta(data, { id: '123\r' })).toThrow('Event\'s id must not contain a carriage return, newline or NULL character') + expect(() => withEventMeta(data, { id: '123\0' })).toThrow('Event\'s id must not contain a carriage return, newline or NULL character') expect(() => withEventMeta(data, { retry: Number.NaN })).toThrow('Event\'s retry must be a integer and >= 0') expect(() => withEventMeta(data, { retry: 1.1 })).toThrow('Event\'s retry must be a integer and >= 0') expect(() => withEventMeta(data, { retry: -1 })).toThrow('Event\'s retry must be a integer and >= 0') diff --git a/packages/core/src/event-stream/types.ts b/packages/core/src/event-stream/types.ts index 7168dbb..94298c1 100644 --- a/packages/core/src/event-stream/types.ts +++ b/packages/core/src/event-stream/types.ts @@ -2,7 +2,7 @@ export interface EventMeta { /** * Event identifier, sent back by the client as `lastEventId` for reconnection attempts. * - * @warning id cannot contain newline characters (`\n`) + * @warning id cannot contain carriage return (`\r`), newline (`\n`) or NULL (`\0`) characters */ id?: string | undefined diff --git a/packages/peer/src/validators.test.ts b/packages/peer/src/validators.test.ts index e64bfd3..b548ab7 100644 --- a/packages/peer/src/validators.test.ts +++ b/packages/peer/src/validators.test.ts @@ -135,6 +135,7 @@ describe('isPeerEventStreamMessage', () => { ['negative retry', { retry: -1 }], ['fractional retry', { retry: 1.5 }], ['id with line break', { id: 'a\nb' }], + ['id with NULL', { id: 'a\0b' }], ['non-string-array comments', { comments: [1, 2] }], ['comment with line break', { comments: ['ok', 'a\rb'] }], ])('rejects %s', (_, json) => expect(isPeerEventStreamMessage(msg(json))).toBe(false))