Add max_event_size to cap SSE event buffering#1071
Conversation
Iterating an EventSource buffers a server-sent event until its terminating blank line, so a stream that never completes an event grows the decoder buffer without bound. Add an opt-in max_event_size to client.sse() (and EventSource) that raises SSEError once a single event's buffered bytes exceed the limit, checked both for an unterminated line and for accumulated data: fields. The counter resets per event.
|
Docs preview: |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74895ce2c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Bound SSE event buffering by default rather than only when the caller opts in, mirroring the WebSocket max_message_size_bytes default. Callers can raise the cap or pass None to disable it.
Comment lines (SSE keepalive heartbeats) are not buffered, so counting them let a long idle stream trip max_event_size despite holding no event data. Skip them before accounting, and reset the counter on every blank line so comment bytes never carry into the next event.
- AsyncClient.sse() now defaults max_event_size to 1 MiB like the sync client; it was left at None, so async callers had no cap by default. - Move the size check into EventSource so it counts the completed lines and the pending unterminated line against one limit, instead of each decoder checking its own buffer (which allowed nearly 2x the cap to buffer before a newline arrived). - Attach the request to the size-limit SSEError, matching the content-type SSEError and RequestError behaviour.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Summing the accumulated event bytes with the pending line buffer let a partial next event, carried in the same chunk as a completed event, count against the previous event and wrongly trip the limit. Check each against the cap on its own instead.
| `max_event_size` caps the number of bytes buffered for a single event; | ||
| iterating raises `SSEError` if an event exceeds it. Set it to `None` to | ||
| buffer events without a size limit. |
There was a problem hiding this comment.
Dropped in 6f31fb0 (both sync and async). The parameter stays documented in docs/sse.md.
| `max_event_size` caps the number of bytes buffered for a single event; | ||
| iterating raises `SSEError` if an event exceeds it. Set it to `None` to | ||
| buffer events without a size limit. |
Move the size check into the decoders, raising where each buffer grows, instead of calling a helper after every decoded line, chunk, and flush. The line and event buffers stay independent, and wrapping iteration in request_context attaches the request to every SSEError.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/httpx2/httpx2/_sse.py">
<violation number="1" location="src/httpx2/httpx2/_sse.py:112">
P2: An event can exceed the cap while its final line is still unterminated: completed fields and `_buffer` are checked independently rather than as parts of the same event. Account for their combined size after processing each chunk, while resetting/partitioning at blank lines so an adjacent event's partial line is not charged to its predecessor.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| text = self._buffer + text.replace("\r\n", "\n").replace("\r", "\n") | ||
| lines = text.split("\n") | ||
| self._buffer = lines.pop() | ||
| if self._max_event_size is not None and len(self._buffer.encode("utf-8")) > self._max_event_size: |
There was a problem hiding this comment.
P2: An event can exceed the cap while its final line is still unterminated: completed fields and _buffer are checked independently rather than as parts of the same event. Account for their combined size after processing each chunk, while resetting/partitioning at blank lines so an adjacent event's partial line is not charged to its predecessor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_sse.py, line 112:
<comment>An event can exceed the cap while its final line is still unterminated: completed fields and `_buffer` are checked independently rather than as parts of the same event. Account for their combined size after processing each chunk, while resetting/partitioning at blank lines so an adjacent event's partial line is not charged to its predecessor.</comment>
<file context>
@@ -105,6 +109,8 @@ def decode(self, text: str) -> list[str]:
text = self._buffer + text.replace("\r\n", "\n").replace("\r", "\n")
lines = text.split("\n")
self._buffer = lines.pop()
+ if self._max_event_size is not None and len(self._buffer.encode("utf-8")) > self._max_event_size:
+ raise SSEError(f"Server-sent event exceeded the {self._max_event_size} byte limit.")
return lines
</file context>
EventSourcebuffers a server-sent event until its terminating blank line arrives, so a stream that keeps sending data without ever completing an event grows the decoder buffer proportionally to what the server sends. There's currently no way to bound that.This adds an opt-in
max_event_sizetoclient.sse()/AsyncClient.sse()(threaded intoEventSource). When set, iterating raisesSSEErroronce a single event's buffered bytes exceed the limit. The check covers both shapes:\n), anddata:fields across many lines before the blank-line terminator.The counter resets after each event, so the limit is per event rather than per stream. Default is
None(unbounded), so existing behaviour is unchanged.This mirrors the bound Go SSE readers get for free from
bufio.Scanner(MaxScanTokenSize, raised viaScanner.Buffer); the Rusteventsource-stream/reqwest-eventsourcecrates have no equivalent.Docs: added a "Limiting event size" section to
docs/sse.md. Coverage on_sse.pystays at 100%.