Skip to content

fix(eventsource): enforce maxEventSize on unterminated lines - #5814

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/eventsource-maxeventsize-unterminated-line
Open

mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/eventsource-maxeventsize-unterminated-line

Conversation

@mcollina

Copy link
Copy Markdown
Member

Summary

EventSourceStream only enforces the eventSource.maxEventSize limit inside parseLine(), which runs only when a line terminator (CR/LF) is reached. A server can stream a field (e.g. data:) that never contains a line terminator; the size check never runs and consumed chunks are never reclaimed, so the client buffers the response body without bound and can be driven toward memory exhaustion.

This change enforces the configured cap during byte accumulation, independent of line framing.

Changes

  • lib/web/eventsource/eventsource-stream.js
    • Track currentLineSize (bytes of the in-progress line), reset whenever a new line starts (syncLineStartToCursor).
    • In _transform(), after each non-terminator byte, throw the existing EventSource message size exceeded error once the line exceeds maxEventSize + DATA_FIELD_PREFIX_SIZE (the longest data: prefix: data: + one space = 6 bytes).
    • The bound is applied to the accumulation loop, so it covers all field types (data, event, id, retry, comments), not just the data: branch.
    • maxEventSize <= 0 still disables the limit entirely.

The DATA_FIELD_PREFIX_SIZE allowance keeps a terminated data: line whose value is exactly maxEventSize accepted (e.g. data: 123456789a\n\n with maxEventSize: 10).

Tests

  • test/eventsource/eventsource-stream.js
    • enforces maxEventSize on an unterminated line
    • accepts a terminated data line with a value equal to maxEventSize
  • test/eventsource/eventsource-constructor.js
    • dispatcher eventSource.maxEventSize closes the connection on an unterminated data line

Verification

  • All 110 EventSource tests pass.
  • eslint clean (pre-commit lint hook passed).

Examples

Before: with maxEventSize: 10, a 20-byte unterminated data: line stays OPEN and buffers every byte without bound.

After: the same line fires EventSource message size exceeded and the connection closes, while data: 123456789a\n\n (10-byte value, exactly maxEventSize) still dispatches normally.

The eventSource.maxEventSize limit is only checked inside parseLine(),
which runs only when a line terminator (CR/LF) is reached. A server can
therefore stream a data: field (or any field) that never contains a line
terminator; the size check never runs and consumed chunks are never
reclaimed, so the client buffers the response body without bound, driving
the process toward memory exhaustion.

Track the in-progress line length (currentLineSize) and reject the stream
once it exceeds maxEventSize plus the longest data: field prefix. This
bounds any unterminated line against the configured cap while still
accepting a terminated data line whose value is exactly maxEventSize.

Adds regression tests for the unterminated-line bypass (stream and E2E)
and for the boundary case of a value equal to maxEventSize.
@mcollina
mcollina requested a review from trivikr September 14, 2026 07:07
@codecov-commenter

codecov-commenter commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.50%. Comparing base (1801a3d) to head (c7408b5).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
lib/web/eventsource/eventsource-stream.js 93.75% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5814   +/-   ##
=======================================
  Coverage   93.50%   93.50%           
=======================================
  Files         110      110           
  Lines       39521    39589   +68     
=======================================
+ Hits        36954    37018   +64     
- Misses       2567     2571    +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants