google / google/quiche

OgHttp2Session strands trailing HEADERS frame when response body exactly exhausts the flow-control window

Open
#149 1 comment 6 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
897
Forks
177
PR merge metrics
No merged PRs in 30d

Description

### Description

When a server response's DATA payload exactly exhausts the flow-control window (e.g. the default 65535-byte window) in the same `WriteForStream()` call that also submits trailers, the trailing HEADERS frame can be left stranded in the internal `frames_` queue instead of being flushed to the wire. This was found while diagnosing envoyproxy/envoy#40821 ("gRPC trailers dropped by oghttp2 when the response is exactly 65535 bytes"), which enables this codec via `envoy.reloadable_features.http2_use_oghttp2`.

### Mechanism

1. `OgHttp2Session::WriteForStream()` (quiche/http2/adapter/oghttp2_session.cc) drains the final chunk of a stream's body via the `DataFrameSource`/visitor callback. When that callback reports end-of-data and trailers are available, `WriteForStream` calls `SubmitTrailer()` (nested, mid-loop) and then `SendTrailers()`, which does `EnqueueFrame(...)` — queuing the HEADERS frame into `frames_`, not writing it immediately.
2. If sending that same final DATA frame also drives `connection_send_window_` to exactly 0 (i.e. body size == window size), `WriteForStream()`'s final return statement:
```cpp
return connection_send_window_ <= 0 ? SendResult::SEND_BLOCKED
: SendResult::SEND_OK;
```
returns `SEND_BLOCKED` purely due to flow-control exhaustion — even though the only thing left to do is flush an already-queued, non-flow-controlled frame.
3. In `OgHttp2Session::Send()`, the trailing flush is gated on `SEND_OK`:
```cpp
if (continue_writing == SendResult::SEND_OK) {
continue_writing = SendQueuedFrames();
}
```
so when `WriteForStream` returned `SEND_BLOCKED`, `SendQueuedFrames()` is skipped, and the queued trailers HEADERS frame is not sent in this `Send()` call.

Additionally, `SubmitTrailer()`'s `trailers_ready_` bypass mechanism (meant to let trailers skip the normal window-gated readiness check) doesn't help here: it's inserted and then immediately erased within the same nested call before `Send()`'s outer loop ever observes it, so it can't trigger a fresh wake-up.

In practice the trailers are usually recovered on the *next* call to `Send()`, since `SendQueuedFrames()` also runs unconditionally at the top of `Send()`. However, in a production proxy (Envoy) chained in front of a real client/server, this can result in the trailers never being sent at all, and an empty DATA frame with END_STREAM being emitted instead — see the pcaps and analysis in the linked Envoy issue.

### Suggested fix

`WriteForStream()` returning `SEND_BLOCKED` should not, by itself, cause a pending flush of already-queued frames to be skipped. Two options:
- Have `OgHttp2Session::Send()` always attempt the trailing `SendQueuedFrames()`, regardless of `WriteForStream`'s return value (it will separately report `SEND_BLOCKED`/`SEND_ERROR` if the socket write itself is blocked).
- Have `WriteForStream()` not report `SEND_BLOCKED` merely because `connection_send_window_ <= 0` while `frames_` still has entries queued for the stream.

### Version

Confirmed present in the current `google/quiche` HEAD (`7b9f65bf5d62e3f6d2dd5da7fd4413be827f2001`, as of 2026-07-23) — `Send()`, `WriteForStream()`, `SubmitTrailer()`, and `SendTrailers()` are unchanged from the version Envoy currently vendors (`c5087ecf2fb9691bbc6de0e7f7506ea4974aaddb`).

### Related

- envoyproxy/envoy#40821

Contributor guide

Open the contributing guide

Research direction

Start in quiche/http2/adapter/oghttp2_session.cc, tracing Send(), WriteForStream(), SendTrailers(), and SendQueuedFrames() for a response whose DATA exactly exhausts the flow-control window while trailers are queued. Reproduce the case and verify that the trailing HEADERS frame is flushed during the same Send() call, with blocked or error socket results still reported correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.