Pull requests / #594

#594 serve: read the request body an answer left unread, so the close is not a reset

closed · @gputier · 0 commentaires · Sur GitHub

Server & APISecurityWindowsLinux

Description

## The defect

The server speaks HTTP/1.0 and answers some requests without reading their body: a 401 for a wrong key, a 403 (cross-site or unknown Host), and a method with no handler (501). On v0.1.38 `/load` and `/unload` did the same; v0.1.39 reads their body first (#630). It then closes the connection with the body still unread.

On Windows that close sends a reset (RST). A client that sends its body after the headers, which is what `http.client`, `urllib` and `requests` do, gets `WinError 10054` instead of the answer. The user sees a connection error where the server said "401 Unauthorized".

To reproduce: start the server with `--api-key`, then POST to `/v1/chat/completions` from Python with a wrong key and a JSON body. On Windows the call raises `ConnectionResetError: [WinError 10054]`. On Linux the answer is still readable, but the socket carries a pending error: `SO_ERROR` reads 32 (EPIPE) after the answer.

## The fix

After an answer, `handle_one_request` reads and drops the body that no handler took. The limit is time only: 5 s for the whole body, with no byte limit, so a conversation of many MiB still gets its answer. The body is read one socket read at a time (`read1`): `read(n)` waits for all n bytes and each byte that arrives restarts the socket timeout, so a client that sends a byte every few seconds would never be let go.

Every handler that reads a body goes through one `_body()` that records the read, so a body is never read twice: the POST dispatcher, `/settings`, and on v0.1.39 `/config` and the `/load` / `/unload` reader of #630. Without that, the drain waited 5 s for bytes those handlers had already taken. `_body()` marks the body as read only when the read got all of it. When #630's 2 s read on `/load` times out, the 400 goes out and the drain takes the rest. It reads from the socket, because after a timeout Python's `rfile` refuses every read ("cannot read from timed out object").

The change touches `serve/server.py` (the handler class) and adds a test class to `serve/test_server.py`.

## Proof

`serve.test_server.AnswerBeforeTheBody`, 10 tests. Each sends the headers, waits 0.3 s, then sends the body, and fails on a reset or a non-zero `SO_ERROR`. They cover a wrong key, a wrong key with an 80 MiB body (sergqwer's case), a body sent one byte every 0.2 s, `/load` and `/unload`, a cross-site 403, an unknown Host 403, a method with no handler (501), a body that `/load`, `/unload` or `/config` read themselves, a `/load` body over 64 KiB (413), and a `/load` body sent half at once and half 3 s later (400). The 5 original ones (wrong key, `/load` and `/unload`, both 403s, the 501) fail on a bare v0.1.38 and pass with the fix. The `/load` / `/unload` / `/config` test fails on all three paths without the `_body()` change. The 413 test fails 3 of 3 on a bare v0.1.39 and passes 3 of 3 with the fix. The late-body test fails without the socket fallback and passes 3 of 3 with it.

- On v0.1.39, Linux container (Python 3.13): `serve.test_server` and `serve.test_security` pass 171 tests. On my fork's main, which carries this branch, the full `serve` suite passes all but one test, `serve.test_responses.OverHttp.test_json_schema_text_format`, which errors the same way on a bare v0.1.39.
- On v0.1.38 (commit `1b2f230`): `serve.test_server` 125 tests, OK, on Linux (Python 3.13) and on Windows 11 (build 26200, Python 3.13.2). Not yet run on Windows on v0.1.39.

## Limits

A chunked body (no `Content-Length`) is not drained, as before the fix: the close can still reset the connection. A body that has not fully arrived after 5 s is left unread, with the same result. After a timed-out read, the drain cannot know how many bytes that read had taken, so it can hold the connection up to the full 5 s.

Sur le site

Liens install, modèles, releases.