Skip to content

fix(streams): frame multiline SSE payloads as consecutive data: lines - #393

Merged
mhenrixon merged 2 commits into
mainfrom
issue-392-sse-multiline-data-framing
Aug 4, 2026
Merged

fix(streams): frame multiline SSE payloads as consecutive data: lines#393
mhenrixon merged 2 commits into
mainfrom
issue-392-sse-multiline-data-framing

Conversation

@mhenrixon

@mhenrixon mhenrixon commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Streams::Envelope.message (lib/pgbus/streams/envelope.rb) no longer strips \r/\n from payloads — a multiline payload is split on SSE line terminators (\r\n | \r | \n) and framed as consecutive data: lines, which EventSource clients rejoin with \n. Delivery is lossless, including a trailing newline (empty final data: line, via split(re, -1)).
  • The <pgbus-stream-source> fetch-path parser (app/assets/javascripts/pgbus/stream_source_element.js) had the matching client-side bug: data += line.slice(5).trim() joined multi-line frames without \n and trimmed significant whitespace. It now follows EventSource semantics — join with \n, strip only the single leading space after the colon. (The native-EventSource reconnect path and spec/support/sse_test_client.rb were already spec-compliant.)
  • Injection defense preserved: every payload line carries the data: prefix, so a crafted payload cannot forge id:/event: fields (spec proves it). Single-line fields — event names, comments — still strip newlines, where a newline is pure injection.

Both the ephemeral and durable delivery paths go through Connection#enqueueEnvelope.message, so the durable path flagged in the issue is fixed by the same change.

Closes #392

Test plan

  • bundle exec rspec spec/pgbus/streams/envelope_spec.rb — framing, round-trip, \r normalization, trailing newline, injection, empty payload (25 examples)
  • bundle exec rspec spec/pgbus/ spec/generators/ — CI unit scope, 3826 examples, 0 failures, coverage 90.57% line / 78.13% branch
  • Manual: broadcast "<div>\n <b>multi</b>\n <i>line</i>\n</div>" on an ephemeral stream, observe data: line per payload line on the wire and byte-identical payload client-side

Deviations & judgment calls

  • Discovery: the newline strip was deliberate (documented) — flat Turbo Stream HTML + SSE field-injection defense. Multi-line data: framing satisfies both; an explicit injection spec proves every payload line stays data:-prefixed.
  • Discovery: the corruption had a second, client-side leg in the fetch-path JS parser (join without \n + trim()); fixed to EventSource semantics. Without this the server fix alone would leave first-connect (XHR) delivery corrupted.
  • Judgment call: \r\n and lone \r normalize to \n on rejoin — SSE has no representation for a raw \r (it is a line terminator in the spec); this is the maximum fidelity SSE permits. Documented in the module docs.
  • Judgment call: empty payload emits a single data: line — Ruby's "".split(re, -1) returns [], which would have dropped the data field entirely; guarded and spec'd.
  • Judgment call: event names and comments still strip newlines — single-line SSE fields where a newline is never data.
  • Discovery: no JS unit-test harness exists (no npm); the parser fix is covered only indirectly by the known-flaky system suite. Verified with node --check and conformance reasoning against the WHATWG SSE parsing algorithm.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed multiline event payloads losing formatting during streaming.
    • Preserved newlines, trailing blank lines, and empty payloads when sending and receiving events.
    • Normalized line endings consistently during event processing.
    • Maintained protections against malformed event data and field injection.
  • Tests

    • Added coverage for multiline payloads, line-ending normalization, empty content, trailing blank lines, and payload reconstruction.

… instead of stripping newlines

## Summary
Envelope.message collapsed \r/\n in broadcast payloads to nothing before
writing a single data: line — silent corruption of whitespace-significant
content on both ephemeral and durable delivery. Payloads are now split on
SSE line terminators into consecutive data: lines, which EventSource
rejoins with \n (lossless, trailing newline included). Injection defense
preserved: every payload line is data:-prefixed; event names and comments
still strip newlines. The stream-source element's fetch-path parser had
the matching client bug (joined without \n, trim()ed payload) and now
follows EventSource semantics.

## Test Coverage
- exact multi-line framing of a multiline payload
- EventSource-style rejoin round-trip (incl. blank lines, trailing \n)
- \r\n / lone \r normalization; no raw \r in frames
- SSE field-injection impossible via payload newlines
- empty payload still emits its data: line

## Verification
- [x] bundle exec rubocop passes
- [x] bundle exec rspec spec/pgbus/ spec/generators/ — 3826 examples, 0 failures

Refs #392
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f35951c9-cba5-4ae9-b1f5-e75b3d973530

📥 Commits

Reviewing files that changed from the base of the PR and between 1e99a09 and 746167c.

📒 Files selected for processing (1)
  • CHANGELOG.md

📝 Walkthrough

Walkthrough

The change preserves multiline SSE payloads. The server emits one data: field per payload line, and the client rejoins those fields with newlines. Tests cover empty payloads, trailing newlines, line-ending normalization, and injection protection.

Changes

SSE multiline payload handling

Layer / File(s) Summary
Server SSE framing and validation
lib/pgbus/streams/envelope.rb, spec/pgbus/streams/envelope_spec.rb, CHANGELOG.md
SSE envelopes preserve payload line boundaries, trailing newlines, and empty payloads. Tests cover line-ending normalization, payload reconstruction, and field-injection protection.
Client SSE payload reconstruction
app/assets/javascripts/pgbus/stream_source_element.js
The parser removes only one optional space after data: and joins consecutive data fields with newline characters.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit sees lines in a bright SSE stream,
Each data: carries one part of the dream.
Newlines stay safe, empty lines too,
The client joins them as EventSource will do.
“No payload shall vanish!” the rabbit declares,
Then hops through the tests with impeccable care.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: framing multiline SSE payloads as consecutive data: lines.
Linked Issues check ✅ Passed The changes implement multiline SSE data: framing, lossless parser reconstruction, injection protection, and matching durable delivery behavior required by issue #392.
Out of Scope Changes check ✅ Passed All code, parser, changelog, and test changes directly support the multiline SSE payload framing objectives in issue #392.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-392-sse-multiline-data-framing

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 5: Update the CHANGELOG entry’s inline code formatting around the SSE
data prefix: remove the trailing space from the `data:` code span and describe
the separator space outside the span, preserving the existing explanation.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8f5a17c6-35cd-46a2-8f09-9afa6723be19

📥 Commits

Reviewing files that changed from the base of the PR and between b1f5950 and 1e99a09.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • app/assets/javascripts/pgbus/stream_source_element.js
  • lib/pgbus/streams/envelope.rb
  • spec/pgbus/streams/envelope_spec.rb

Comment thread CHANGELOG.md Outdated
@mhenrixon mhenrixon self-assigned this Aug 4, 2026
@mhenrixon mhenrixon added the bug Something isn't working label Aug 4, 2026
@mhenrixon
mhenrixon merged commit 94366ff into main Aug 4, 2026
13 checks passed
@mhenrixon
mhenrixon deleted the issue-392-sse-multiline-data-framing branch August 4, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ephemeral SSE delivery strips newlines from payloads instead of SSE multi-line data framing

1 participant