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))