api7 / api7/aisix

BufferFull stream cap counts text bytes, not held wire bytes

Open
#513 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
157
Forks
32
Avg merge
1h 25m
Merged PRs (30d)
145

Description

Summary

build_sse_stream's BufferFull cap (crates/aisix-proxy/src/chat.rs) bounds memory using content_buffer.len() — the accumulated text content — but the memory actually held while the stream is paused is pending: Vec<Event>, the full rendered SSE events (content deltas plus tool-call arguments, reasoning, role, and JSON/SSE framing).

let buffered = content_buffer.as_ref().map_or(0, |b| b.len());
if buffered > *max_buffer_bytes { ... }

A response stream that is light on assistant text but heavy on tool-call / reasoning chunks grows pending without growing content_buffer, so the cap under-counts real held memory and can trip late (or, in a pathological tool-call-only stream, never).

Why it matters now

Previously reachable only by operators who explicitly set stream_processing_mode: buffer_full on a text-moderation guardrail. After #466 made BufferFull the trait default for output-blocking guardrails, every default-configured output guardrail relies on this cap — widening the blast radius. It is a memory-amplification / soft-DoS vector (bounded in practice by the model's max output tokens, but multiplied across concurrent held streams).

Proposed fix

Track held wire bytes, not just text:

  • Add a running pending_bytes: usize counter alongside pending.
  • Thread each rendered event's serialized length (json.len()) out of the chunk match and add it on pending.push.
  • Reset pending_bytes = 0 at each pending.drain(..) site (window release, cap fail-open release, end-of-stream release).
  • Check pending_bytes > max_buffer_bytes instead of content_buffer.len().

pending_bytes >= content_buffer.len() always, so this only makes the cap trip earlier — it can never let through a stream that previously failed closed.

Trade-off to decide (why this is split out, not bundled into #466)

Counting wire bytes changes the meaning of max_buffer_bytes from "text bytes" to "held wire bytes," and the current default (262144 = 256 KiB) would then fail-closed legitimate large-text responses earlier (~64-128 KiB of text once JSON/SSE framing overhead is included). The default value likely needs re-tuning at the same time. That is a deliberate design decision, tracked here rather than silently changing a config field's semantics inside the security-default PR (#512).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in crates/aisix-proxy/src/chat.rs at build_sse_stream, then trace where rendered events are pushed into pending and where pending.drain(..) releases them. Determine how to count serialized event bytes and resolve whether max_buffer_bytes needs re-tuning under the new held-wire-bytes meaning. Done means the cap reflects pending event data and the default behavior is intentionally decided.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.