fix(core): parse event-stream retry and id fields per the SSE spec

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({})
   })
 })
 

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
       }

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

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

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

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
 

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