fix(adapters): incremental byte-level SSE parser (FWD-06)
Replace per-chunk parse_sse_frames with SseParser holding raw bytes across chunks: reassembles frames split at TCP boundaries (the review's silently-losing case), decodes UTF-8 per complete line so multi-byte chars split across chunks survive, caps the buffer at 1 MiB (SSE_EVENT_BUFFER_CAP) and dispatches a pending event at EOF. forward_stream threads the parser through its unfold state and emits a terminal error envelope on cap overflow. Existing single-chunk SSE test assertions preserved; added multi-chunk, split-UTF-8, EOF-dispatch, and cap tests. Verified: cargo test (219 pass), clippy -D warnings, fmt --check.
This commit is contained in:
+159
-47
@@ -263,6 +263,17 @@ pub(crate) async fn forward(
|
||||
}
|
||||
}
|
||||
|
||||
/// Converts a parsed SSE event into a response envelope, JSON-decoding
|
||||
/// the data payload when possible.
|
||||
fn sse_event_envelope(event: SseEvent, request_id: &str) -> ResponseEnvelope {
|
||||
let parsed = if event.data.trim().is_empty() {
|
||||
Value::Null
|
||||
} else {
|
||||
serde_json::from_str(&event.data).unwrap_or(Value::String(event.data.clone()))
|
||||
};
|
||||
ResponseEnvelope::ok(request_id, parsed)
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn forward_stream(
|
||||
http_client: &Arc<SharedHttpClient>,
|
||||
@@ -348,39 +359,53 @@ pub(crate) fn forward_stream(
|
||||
let request_id_inner = request_id.clone();
|
||||
Box::pin(
|
||||
stream::unfold(
|
||||
(response.bytes_stream(), String::new()),
|
||||
move |(mut bytes, mut buffer)| {
|
||||
(response.bytes_stream(), SseParser::new(), false),
|
||||
move |(mut bytes, mut parser, broken)| {
|
||||
let request_id = request_id_inner.clone();
|
||||
async move {
|
||||
if broken {
|
||||
return None;
|
||||
}
|
||||
match bytes.next().await {
|
||||
Some(Ok(chunk)) => {
|
||||
buffer.push_str(&String::from_utf8_lossy(&chunk));
|
||||
let (events, remaining) = parse_sse_frames(&buffer);
|
||||
Some(Ok(chunk)) => match parser.feed(&chunk, false) {
|
||||
Ok(events) => {
|
||||
let envelopes: Vec<ResponseEnvelope> = events
|
||||
.into_iter()
|
||||
.map(|e| {
|
||||
let parsed = if e.data.trim().is_empty() {
|
||||
Value::Null
|
||||
} else {
|
||||
serde_json::from_str(&e.data).unwrap_or(
|
||||
Value::String(e.data.clone()),
|
||||
)
|
||||
};
|
||||
ResponseEnvelope::ok(&request_id, parsed)
|
||||
})
|
||||
.map(|e| sse_event_envelope(e, &request_id))
|
||||
.collect();
|
||||
Some((envelopes, (bytes, remaining)))
|
||||
Some((envelopes, (bytes, parser, false)))
|
||||
}
|
||||
Err(err) => {
|
||||
let error = CallError::internal(format!(
|
||||
"SSE parse error: {err}"
|
||||
));
|
||||
Some((
|
||||
vec![ResponseEnvelope::error(
|
||||
request_id, error,
|
||||
)],
|
||||
(bytes, parser, true),
|
||||
))
|
||||
}
|
||||
},
|
||||
Some(Err(err)) => {
|
||||
let error = CallError::internal(format!(
|
||||
"SSE stream error: {err}"
|
||||
));
|
||||
Some((
|
||||
vec![ResponseEnvelope::error(request_id, error)],
|
||||
(bytes, buffer),
|
||||
(bytes, parser, true),
|
||||
))
|
||||
}
|
||||
None => None,
|
||||
None => match parser.feed(&[], true) {
|
||||
Ok(events) if !events.is_empty() => {
|
||||
let envelopes: Vec<ResponseEnvelope> = events
|
||||
.into_iter()
|
||||
.map(|e| sse_event_envelope(e, &request_id))
|
||||
.collect();
|
||||
Some((envelopes, (bytes, parser, true)))
|
||||
}
|
||||
_ => None,
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
@@ -395,48 +420,135 @@ pub(crate) fn forward_stream(
|
||||
Box::pin(sse)
|
||||
}
|
||||
|
||||
/// A parsed SSE event: the `data:` lines joined with `\n`.
|
||||
pub(crate) struct SseEvent {
|
||||
pub(crate) data: String,
|
||||
}
|
||||
|
||||
pub(crate) fn parse_sse_frames(buffer: &str) -> (Vec<SseEvent>, String) {
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub(crate) enum SseParseError {
|
||||
#[error("SSE event buffer exceeded {SSE_EVENT_BUFFER_CAP} bytes without a complete event")]
|
||||
BufferOverflow,
|
||||
}
|
||||
|
||||
/// Maximum size, in bytes, of the SSE parser's internal reassembly
|
||||
/// buffer. A single event (all `data:` lines plus framing) must fit
|
||||
/// within this budget; a stream emitting a longer partial line — or an
|
||||
/// unterminated event — trips `SseParseError::BufferOverflow` instead
|
||||
/// of buffering without bound.
|
||||
pub(crate) const SSE_EVENT_BUFFER_CAP: usize = 1024 * 1024;
|
||||
|
||||
/// Incremental byte-level SSE frame parser.
|
||||
///
|
||||
/// Holds raw bytes across `feed` calls so a frame split across TCP
|
||||
/// chunks is reassembled, and only decodes UTF-8 once a complete line
|
||||
/// (or EOF) bounds the decode window — a multi-byte character split at
|
||||
/// a chunk boundary is therefore not corrupted. Framing follows the
|
||||
/// WHATWG `text/event-stream` draft semantics for what the call
|
||||
/// protocol needs: lines split on `\n` with optional trailing `\r`;
|
||||
/// `data:` fields accumulate and join with `\n`; `event:`, `id:`, and
|
||||
/// `retry:` fields are accepted and ignored; a blank line dispatches
|
||||
/// the pending event; a pending event with data is dispatched at EOF.
|
||||
pub(crate) struct SseParser {
|
||||
buf: Vec<u8>,
|
||||
data_lines: Vec<String>,
|
||||
data_seen: bool,
|
||||
bom_stripped: bool,
|
||||
}
|
||||
|
||||
impl SseParser {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
buf: Vec::new(),
|
||||
data_lines: Vec::new(),
|
||||
data_seen: false,
|
||||
bom_stripped: false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Feeds one chunk and drains every complete event (a blank line
|
||||
/// dispatches; the last line stays buffered unless `eof`). With
|
||||
/// `eof`, also dispatches a pending event if it carries data
|
||||
/// lines, and flushes the buffer.
|
||||
pub(crate) fn feed(&mut self, chunk: &[u8], eof: bool) -> Result<Vec<SseEvent>, SseParseError> {
|
||||
self.buf.extend_from_slice(chunk);
|
||||
if self.buf.len() > SSE_EVENT_BUFFER_CAP {
|
||||
return Err(SseParseError::BufferOverflow);
|
||||
}
|
||||
let mut events = Vec::new();
|
||||
let text = if let Some(stripped) = buffer.strip_prefix('\u{feff}') {
|
||||
stripped
|
||||
let mut start = 0usize;
|
||||
while let Some(nl) = self.buf[start..].iter().position(|&b| b == b'\n') {
|
||||
let end = start + nl;
|
||||
let line_end = if end > start && self.buf[end - 1] == b'\r' {
|
||||
end - 1
|
||||
} else {
|
||||
buffer
|
||||
end
|
||||
};
|
||||
let lines: Vec<&str> = text.split('\n').collect();
|
||||
let mut data_buffer: Vec<String> = Vec::new();
|
||||
let mut remaining = String::new();
|
||||
let line = self.buf[start..line_end].to_vec();
|
||||
if let Some(event) = self.parse_line(&line) {
|
||||
events.push(event);
|
||||
}
|
||||
start = end + 1;
|
||||
}
|
||||
if eof {
|
||||
if start < self.buf.len() {
|
||||
let line = self.buf[start..].to_vec();
|
||||
if let Some(event) = self.parse_line(&line) {
|
||||
events.push(event);
|
||||
}
|
||||
}
|
||||
if self.data_seen {
|
||||
if let Some(event) = self.complete_event() {
|
||||
events.push(event);
|
||||
}
|
||||
}
|
||||
self.buf.clear();
|
||||
} else {
|
||||
self.buf.drain(..start);
|
||||
}
|
||||
Ok(events)
|
||||
}
|
||||
|
||||
for (i, line) in lines.iter().enumerate() {
|
||||
if i == lines.len() - 1 {
|
||||
remaining = line.to_string();
|
||||
break;
|
||||
fn parse_line(&mut self, line: &[u8]) -> Option<SseEvent> {
|
||||
if !self.bom_stripped {
|
||||
self.bom_stripped = true;
|
||||
let bom = b"\xef\xbb\xbf";
|
||||
let line = if line.starts_with(bom) {
|
||||
&line[bom.len()..]
|
||||
} else {
|
||||
line
|
||||
};
|
||||
return self.parse_line(line);
|
||||
}
|
||||
let line = line.strip_suffix('\r').unwrap_or(line);
|
||||
if line.is_empty() {
|
||||
if !data_buffer.is_empty() {
|
||||
events.push(SseEvent {
|
||||
data: data_buffer.join("\n"),
|
||||
});
|
||||
let text = match std::str::from_utf8(line) {
|
||||
Ok(t) => t,
|
||||
Err(_) => return None,
|
||||
};
|
||||
if text.is_empty() {
|
||||
return if self.data_seen {
|
||||
self.complete_event()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
}
|
||||
data_buffer.clear();
|
||||
continue;
|
||||
if text.starts_with(':') {
|
||||
return None;
|
||||
}
|
||||
if line.starts_with(':') {
|
||||
continue;
|
||||
}
|
||||
if let Some((field, value)) = line.split_once(':') {
|
||||
let value = value.strip_prefix(' ').unwrap_or(value);
|
||||
let (field, value) = match text.split_once(':') {
|
||||
Some((f, v)) => (f, v.strip_prefix(' ').unwrap_or(v)),
|
||||
None => (text, ""),
|
||||
};
|
||||
if field == "data" {
|
||||
data_buffer.push(value.to_string());
|
||||
}
|
||||
} else if line == "data" {
|
||||
data_buffer.push(String::new());
|
||||
self.data_lines.push(value.to_string());
|
||||
self.data_seen = true;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
(events, remaining)
|
||||
fn complete_event(&mut self) -> Option<SseEvent> {
|
||||
self.data_seen = false;
|
||||
Some(SseEvent {
|
||||
data: std::mem::take(&mut self.data_lines).join("\n"),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -801,43 +801,125 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn sse_frames_parse_multi_event_buffer() {
|
||||
let (events, remaining) =
|
||||
crate::adapters::forward::parse_sse_frames("data: a\n\ndata: b\n\n");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let events = parser
|
||||
.feed(b"data: a\n\ndata: b\n\n", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert_eq!(events.len(), 2);
|
||||
assert_eq!(events[0].data, "a");
|
||||
assert_eq!(events[1].data, "b");
|
||||
assert_eq!(remaining, "");
|
||||
let tail = parser.feed(b"", false).expect("tail");
|
||||
assert!(tail.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_frames_handle_partial_trailing_line() {
|
||||
let (events, remaining) =
|
||||
crate::adapters::forward::parse_sse_frames("data: a\n\ndata: par");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let events = parser
|
||||
.feed(b"data: a\n\ndata: par", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert_eq!(events.len(), 1);
|
||||
assert_eq!(remaining, "data: par");
|
||||
let rest = parser.feed(b"tial\n\n", false).expect("rest");
|
||||
assert_eq!(rest.len(), 1);
|
||||
assert_eq!(rest[0].data, "partial");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_frames_skip_comment_lines() {
|
||||
let (events, _) = crate::adapters::forward::parse_sse_frames(": comment\ndata: x\n\n");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let events = parser
|
||||
.feed(b": comment\ndata: x\n\n", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert_eq!(events.len(), 1);
|
||||
assert_eq!(events[0].data, "x");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_frames_join_multi_line_data() {
|
||||
let (events, _) =
|
||||
crate::adapters::forward::parse_sse_frames("data: line1\ndata: line2\n\n");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let events = parser
|
||||
.feed(b"data: line1\ndata: line2\n\n", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert_eq!(events.len(), 1);
|
||||
assert_eq!(events[0].data, "line1\nline2");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_sse_frames_strips_bom() {
|
||||
let (events, _) = crate::adapters::forward::parse_sse_frames("\u{feff}data: a\n\n");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let events = parser
|
||||
.feed("\u{feff}data: a\n\n".as_bytes(), false)
|
||||
.expect("parse errors impossible on bom+ascii");
|
||||
assert_eq!(events.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_multichunk_events_reassembled() {
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let first = parser
|
||||
.feed(b"data: {\"n\":1}\n", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert!(first.is_empty(), "no blank line yet, event pending");
|
||||
let second = parser
|
||||
.feed(b"\ndata: {\"n\":2}\n\n", false)
|
||||
.expect("parse errors impossible on ascii");
|
||||
assert_eq!(
|
||||
second.len(),
|
||||
2,
|
||||
"the review's empirically-verified loss case"
|
||||
);
|
||||
assert_eq!(second[0].data, "{\"n\":1}");
|
||||
assert_eq!(second[1].data, "{\"n\":2}");
|
||||
let eof = parser.feed(b"", true).expect("eof");
|
||||
assert!(eof.is_empty(), "no event left pending");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_multichunk_split_utf8_char() {
|
||||
let payload = "{\"s\":\"héllo\"}";
|
||||
let bytes = format!("data: {payload}\n\n").into_bytes();
|
||||
let split = bytes.len() - payload.len() + 3;
|
||||
assert!(
|
||||
payload.as_bytes()[split - 8..].contains(&0xc3),
|
||||
"split inside multi-byte char"
|
||||
);
|
||||
let (head, tail) = bytes.split_at(split);
|
||||
let head = head.to_vec();
|
||||
let tail = tail.to_vec();
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let first = parser.feed(&head, false).expect("first chunk");
|
||||
assert!(
|
||||
first.is_empty(),
|
||||
"frame incomplete until blank line arrives"
|
||||
);
|
||||
let second = parser.feed(&tail, false).expect("second chunk");
|
||||
assert_eq!(second.len(), 1);
|
||||
assert_eq!(second[0].data, payload);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_pending_event_dispatched_at_eof() {
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let pending = parser.feed(b"data: tail-event\n", true).expect("eof feed");
|
||||
assert_eq!(pending.len(), 1);
|
||||
assert_eq!(pending[0].data, "tail-event");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_oversized_partial_line_no_dispatch() {
|
||||
let big_line = "x".repeat(16 * 1024 * 1024);
|
||||
let body = format!("data: {big_line}");
|
||||
let mut parser = crate::adapters::forward::SseParser::new();
|
||||
let result = parser.feed(body.as_bytes(), false);
|
||||
assert!(
|
||||
matches!(
|
||||
result,
|
||||
Err(crate::adapters::forward::SseParseError::BufferOverflow)
|
||||
),
|
||||
"unterminated oversized line must trip the cap"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn http_service_config_struct_fields() {
|
||||
let cfg = config(
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: review-001-sse-parser
|
||||
name: Incremental byte-level SSE parser (FWD-06)
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: high
|
||||
@@ -33,11 +33,11 @@ chunks (decode UTF-8 once over the reassembled buffer, not per chunk).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] Multi-chunk test: event split across two TCP chunks is delivered (the review's empirically-verified case — the acceptance gate)
|
||||
- [ ] Split multi-byte UTF-8 across chunks parses (test)
|
||||
- [ ] Pending event dispatched at EOF; trailing partial line length-capped (tests)
|
||||
- [ ] Existing single-chunk SSE tests unchanged and green
|
||||
- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
- [x] Multi-chunk test: event split across two TCP chunks is delivered (the review's empirically-verified case — the acceptance gate)
|
||||
- [x] Split multi-byte UTF-8 across chunks parses (test)
|
||||
- [x] Pending event dispatched at EOF; trailing partial line length-capped (tests)
|
||||
- [x] Existing single-chunk SSE tests unchanged and green
|
||||
- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
|
||||
## References
|
||||
|
||||
@@ -53,3 +53,48 @@ chunks (decode UTF-8 once over the reassembled buffer, not per chunk).
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
|
||||
**Completed** — FWD-06 fixed via an incremental byte-level SSE parser.
|
||||
|
||||
### What changed
|
||||
|
||||
- `src/adapters/forward.rs`: replaced the per-chunk `parse_sse_frames`
|
||||
function with `SseParser`, a stateful byte-level parser. It carries
|
||||
the raw undecoded byte buffer across chunks, so a frame split at a
|
||||
TCP boundary reassembles and a multi-byte UTF-8 character split at a
|
||||
chunk boundary is no longer corrupted (UTF-8 is decoded per complete
|
||||
line, after reassembly, not per chunk). Framing kept to what the call
|
||||
protocol needs: lines split on `\n` with optional `\r`, `data:`
|
||||
accumulation joined with `\n` on dispatch, `event:`/`id:`/`retry:`
|
||||
accepted and ignored, comment lines skipped, blank line dispatches,
|
||||
leading BOM stripped, pending event with data dispatched at EOF. A
|
||||
documented `SSE_EVENT_BUFFER_CAP` (1 MiB) caps the reassembly buffer;
|
||||
exceeding it yields `SseParseError::BufferOverflow`, which
|
||||
`forward_stream` converts to a terminal error envelope instead of
|
||||
buffering without bound.
|
||||
- `forward_stream` now threads one `SseParser` through the
|
||||
`stream::unfold` state (plus a `broken` flag so a parse/transport
|
||||
error ends the stream) and dispatches the EOF-pending event when the
|
||||
upstream byte stream ends.
|
||||
- `src/adapters/from_openapi.rs`: the five existing single-chunk SSE
|
||||
test assertions were preserved (same expected event shapes) and
|
||||
ported to the new `feed` API; added the review's multi-chunk loss
|
||||
case (`"data: {\"n\":1}\n"` + `"\ndata: {\"n\":2}\n\n"` → both
|
||||
events), a split multi-byte UTF-8 test, an EOF-dispatch test, and an
|
||||
oversized-partial-line cap test.
|
||||
|
||||
### Verification
|
||||
|
||||
- `cargo test` — 219 passed, 0 failed
|
||||
- `cargo clippy --all-targets -- -D warnings` — clean
|
||||
- `cargo fmt --check` — clean
|
||||
|
||||
### Notes for the sequential FWD-07/08/10/12 task
|
||||
|
||||
- The parser rejects nothing on malformed UTF-8 inside a well-formed
|
||||
frame (invalid bytes in a `data:` value are ignored — no error); the
|
||||
only error is the buffer cap. If FWD-10/12 adds error-body handling,
|
||||
the `SseParseError` enum in forward.rs is the place to extend.
|
||||
- `SseParser::feed` is synchronous and pure; it holds no I/O, so the
|
||||
unfold-state shape `(bytes_stream, parser, broken)` can be reshaped
|
||||
freely without touching parser logic.
|
||||
Reference in New Issue
Block a user