serve: fix HTTP/1.1 keep-alive desync and SSE framing (#597 item 3)

[ZacharyZcR]

Closes the last open item of #597. @mkelcb reported Bad request syntax ('{...json body...}POST /v1/chat/completions HTTP/1.1') — a previous request's body being parsed as the next request line — and suggested either dropping to HTTP/1.0 or doing HTTP/1.1 persistence properly with tests. This is the latter.

I reproduced it against dev before writing anything. There are two independent causes, and the diagnosis explains why the symptom seemed to come and go.

Cause 1 — early rejections leak the request body

do_POST runs _check_host() (403) and require_auth() (401) before read_json(). Both raise, the response goes out with a Content-Length, close_connection stays false, and the unread body is still sitting in the socket when the next readline() looks for a request line. Reproduced verbatim:

[api] "POST /v1/chat/completions HTTP/1.1" 403 -
[api] code 400, message Bad request syntax ('{"model": "test-model", ...}POST /v1/chat/completions HTTP/1.1')

This is why it showed up through Tailscale Serve: every request was a 403 until the item 2 allowlist landed in #606, so every request leaked a body. Fixing the allowlist removed the trigger, not the bug — hence "item 3 (retest)". A bad or over-MAX_BODY Content-Length leaks the same way and still does today.

Cause 2 — the streaming 200 is close-framed but says otherwise

start_stream sends no Content-Length and no Connection: close, so the only possible message boundary is the close — yet close_connection = True sat at the end of the stream (openai_server.py:1845, and :2110 for Anthropic), outside any try/finally. Two consequences:

• A conforming client is told the connection is reusable for a response it can only terminate by reading to EOF.
• If generation raises after the commit, that line never runs. do_POST then calls send_json, writing a complete HTTP/1.1 500 response inside the event stream, and leaves the socket advertised as reusable. My probe against dev:

-> second HTTP response injected into the stream? True
-> connection left open (no EOF)?             True

#609's deferred commit narrowed the window before ACCEPT; everything after it was still exposed.

The fix

Rather than teaching each early return to remember its body, drain once at the request boundary in handle_one_request() — one place, covering every early exit that exists now or later. Where the body can't be swallowed safely (chunked, unparseable or over-limit length, short read) it closes instead: an unreusable connection is correct, a desynchronised one is not, and swallowing an arbitrary declared length just to keep a socket warm is not a trade worth making.

Both SSE paths now send Connection: close and set close_connection when the 200 is committed, not when the stream ends — so a mid-stream failure cannot leave a reusable socket. send_response() is overridden as the single choke point recording that the status line is out, and _fail() consults it to stay silent rather than splice a second response into a committed stream.

Plain JSON keeps HTTP/1.1 persistence — explicitly tested, so this can't quietly degrade into close-everything (which is what the HTTP/1.0 workaround would have cost).

Tests

KeepAliveFramingTest, 11 cases covering the six points from the issue's regression list plus the paths above. 8 fail on dev, 3 are no-regression guards that pass either way:

• 403 / 401 / 404 rejection followed by a valid request on the same connection
• 4 sequential requests on one connection, asserting the engine sees each prompt exactly once with no bleed
• rejections and successes interleaved 3× on one connection
• over-limit and unparseable Content-Length close rather than desync
• both SSE paths (OpenAI + Anthropic) announce connection: close and reach EOF
• engine failure after commit splices no second response (raw.count("HTTP/1.") == 1)
• non-streaming JSON still reuses the connection

make check: 197 passed, 13 skipped. python3 -m pytest tests/test_openai_server.py: 94 passed.

Verified on Linux only — the change is pure http.server behaviour with no platform-specific paths.

@mkelcb if you can retest through Tailscale Serve, the interesting case is now a deliberately wrong --allowed-host followed by a real request on the same connection: that used to poison the connection and should now just be a clean 403.