cloudflare / cloudflare/pingora

Add an overall upstream response-header deadline distinct from read_timeout

Open
#992 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

## What problem does this solve?

Pingora currently exposes `PeerOptions::read_timeout` as a per-read inactivity timeout. That is useful for a silent upstream, but it does not bound the total time spent receiving an incomplete response header when the upstream keeps making small amounts of progress before each individual read timeout expires.

At current `main` commit `09696b51bc59315353d96686355861604d0bb48c`, the HTTP/1 client `HttpSession::read_response()` keeps partial bytes in `response_header_read_buf`, then wraps each `underlying_stream.read_buf(...)` in a fresh `read_timeout`. Any successful read loops back to parsing and receives a new timeout. The public peer documentation and both H1/H2 client fields also describe `read_timeout` as resetting on every read and explicitly not being an overall response-duration timeout.

That means an upstream can send an incomplete response header one byte/fragment at a time, each fragment arriving inside `read_timeout`, and keep a request occupied without ever completing `\r\n\r\n` or violating the inactivity timeout.

This cannot be implemented reliably by a `ProxyHttp` application today: `upstream_response_filter` is invoked only after the response header has arrived, while body filters are later still. `total_connection_timeout` is not a substitute because it bounds connection establishment (including TLS), not the response-header phase.

## Desired behavior

Please consider an additive, explicit **overall upstream response-header deadline** (name/API shape up to maintainers) that is distinct from `read_timeout`.

The important semantics would be:

- the deadline spans repeated successful reads while one response header is incomplete;
- per-read `read_timeout` remains independently useful as an inactivity timeout;
- HTTP/1 and HTTP/2 behavior is explicit;
- informational (1xx) responses have documented semantics: e.g. whether the budget applies independently to each header block or to the entire pre-final-response header phase;
- timeout cancellation is safe and does not leave response-header parsing state or pooled connection reuse in an inconsistent state;
- an unset value preserves current behavior.

A peer option is one possible surface, but an equivalent cancellation-safe hook would also solve the application-level gap if it can actually terminate a pending header read at the overall deadline.

## Reproduction shape

A deterministic regression can use an origin that:

1. accepts the upstream request;
2. starts sending a syntactically valid-but-incomplete status/header block;
3. sends one byte or small fragment every `N` ms where `N < read_timeout`;
4. deliberately withholds the terminating `\r\n\r\n` past the configured overall header deadline.

Expected with a new overall deadline: the request fails at approximately that total header budget even though every individual read made progress inside `read_timeout`.

This is specifically about bounding the **response-header phase**. It is separate from whole-response/body lifetime policy and from application-level streaming semantics.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.