Pull requests / #2910
#2910 serve: a request body that stops arriving is answered, not parked
open · @ctr-137 · 0 Kommentare · Auf GitHub
Beschreibung
`_body()` is the one body read in the server with no deadline. A peer that announces a body and then says nothing — a
silently dead client, no RST and no FIN — parks the handler thread and its fd for good, and the disconnect watcher
that would have noticed starts only *after* the body has been read.
### The three body readers, and which one was the odd one out
The two others already bound their reads, in two different ways:
```python
def _control_body(self) -> bool:
timeout = self.connection.gettimeout()
try:
self.connection.settimeout(2.0)
complete = len(self.rfile.read(length)) == length
except OSError:
complete = False
finally:
self.connection.settimeout(timeout)
```
and `_drain_body()`, which passes `deadline=time.monotonic() + DRAIN_SECONDS` to `_read_chunked()` — a parameter
`_read_chunked()` already had and `_body()` did not pass:
```python
def _body(self) -> bytes:
...
if self._chunked():
return self._read_chunked(limit) # no deadline
...
return self.rfile.read(length) # no deadline, one blocking read
```
### What changed
- `BODY_STALL_S` (`STRATA_BODY_STALL_S`, default 120 s, `0` = wait for ever as before), beside `body_limit()`, shaped
like the engine's own read timeouts (#1317). Generous on purpose: this is a stopped client's deadline, not a
slow-client detector.
- `_body()` sets it on the connection for the read and restores the previous timeout in a `finally`, exactly as
`_control_body()` does, and answers a read that gets nothing with **408** — the status a body problem already
carries (`BadBody` → `self._json(e.status, ...)` in `do_POST`).
It is the wait for one read that gets nothing, not a bound on the whole body: a client that is sending, however
slowly, is never cut off, only one that has stopped. A half-read body leaves the stream unusable, so
`close_connection` is set and the connection ends with the answer.
### Why the existing test file missed it
`test_request_hardening.py` covers the two neighbours and stops just short of this one: the oversized case announces
10 TiB, which the 413 refuses *unread*, and a half-close ends the read with a 400. A **within-limit** body that
announces a length and sends nothing is the case nothing constructed.
### How this was checked
Three cases added to `serve/test_request_hardening.py`: a within-limit `Content-Length` and a chunked body, each
announcing a body and sending none, both must answer 408 — and the elapsed time is asserted too, or a client-side
timeout would pass the test as well. A complete body still answers 200, with the deadline set and with it off.
That file's 13 tests pass and `test_chunked_body.py`'s 12 pass unchanged, with no GPU and no engine (they run
against `MockEngine`).
A three-arm probe against the real server — announce a body, send nothing, wait 8 s:
```
deadline 1 s -> HTTP/1.0 408 Request Timeout after 1.0 s
deadline off -> no answer within 8 s: the handler thread is parked and its fd is held
the code before this -> no answer within 8 s: the same
```
The two arms that reproduce the bug are the two with no deadline.
### What I left alone
The response path is the same class and wants its own change: a streamed answer to a client that stops reading blocks
`self.wfile.write()` while the generator is suspended inside `with turn:`, so the engine's single turn is held and
every later request waits (`serve/server.py:5976`). Its `except OSError` already does the right thing —
`cancel.set()`, `chunks.close()` — and `socket.timeout` is an `OSError`, so only a *send* deadline is missing there.
That is a separate pull request.
There is no issue for this; it came out of a static audit of the tree rather than a report. Happy to file one if you
would rather triage it there first.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Mehr auf der Site
Links zu Install, Modellen, Releases.