Commit e6e8ce9
authored
fix(client): bound SSE event size to prevent client-side memory exhaustion (#582)
# Description
`parseSseStream` (shared by the JSON-RPC and REST client transports)
accumulates two buffers with **no size bound**:
- the line `buffer` — `buffer += value` on every chunk
- an event's joined `eventData` — consecutive `data:` lines are appended
Because A2A clients stream SSE from remote, potentially untrusted agent
servers, a malicious or broken server can exploit this as a
**denial-of-service
against the client**:
- stream bytes that **never contain a `\n`** → the line `buffer` grows
without bound
- stream endless `data:` lines with **no terminating blank line** →
`eventData` grows without bound
Either way the client keeps allocating without bound. Today that can
result in
a process-level OOM crash that the caller cannot handle as a normal
stream
error.
## Fix
Cap both accumulation points at `maxEventSizeBytes` and throw when
exceeded.
The throw runs the async generator's teardown, whose `finally` now
cancels the
reader (#580), so the offending connection is also closed. Net effect: a
**process-level crash becomes a catchable `Error`** that flows into the
transports' existing SSE-parse error handling.
The same limit applies to the unterminated line buffer because a single
SSE
field line is necessarily part of an event and should not be allowed to
exceed
the maximum event budget on its own.
- Default is **4 MiB** (matching gRPC's default max message size):
realistic
A2A events (Message/Task JSON) are KB-scale, and large files should be
referenced via `FileWithUri` parts rather than inlined.
- It is **configurable** via the new optional `maxEventSizeBytes`
argument for
callers that must inline larger payloads, and the error message points
at
both options.
## Open question — is this the right layer?
I'm **not sure `parseSseStream` is the right place** for this, and would
appreciate maintainer guidance. Alternatives worth considering:
- enforcing it at the transport layer instead of the shared parser,
- deriving/validating against `Content-Length` where present,
- or leaving DoS mitigation entirely to the runtime / a reverse proxy.
Happy to change the default, make it opt-in, or move it elsewhere based
on
what you prefer.
## Tests
Adds regression tests for the unterminated-line,
terminated-oversized-line
(single-chunk), and unterminated-event cases — all would grow unbounded
without the cap — plus a within-limit sanity check.
`test/sse_utils.spec.ts`
is green.
- [x] Follows the `CONTRIBUTING` guide
- [x] PR title uses Conventional Commits (`fix:`)
- [x] Tests and linter pass
- [ ] Docs updated (not necessary)1 parent 83269a5 commit e6e8ce9
2 files changed
Lines changed: 97 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
63 | 84 | | |
64 | 85 | | |
65 | 86 | | |
66 | 87 | | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
67 | 92 | | |
68 | 93 | | |
69 | | - | |
| 94 | + | |
| 95 | + | |
70 | 96 | | |
71 | 97 | | |
72 | 98 | | |
| |||
83 | 109 | | |
84 | 110 | | |
85 | 111 | | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
86 | 115 | | |
87 | 116 | | |
88 | 117 | | |
| |||
108 | 137 | | |
109 | 138 | | |
110 | 139 | | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
111 | 143 | | |
112 | 144 | | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
113 | 151 | | |
114 | 152 | | |
115 | 153 | | |
| |||
118 | 156 | | |
119 | 157 | | |
120 | 158 | | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
121 | 166 | | |
122 | 167 | | |
123 | 168 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
338 | 389 | | |
0 commit comments