diff --git a/psi/serve.py b/psi/serve.py index fecc6b7..0bab46b 100644 --- a/psi/serve.py +++ b/psi/serve.py @@ -308,18 +308,35 @@ def _handle_list(self) -> None: self._respond(200, "\n".join(names).encode()) def _respond(self, code: int, body: bytes) -> None: - self.send_response(code) - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) + self._write_response(code, [], body) def _respond_error(self, code: int, error: str, detail: str) -> None: body = json.dumps({"error": error, "detail": detail}).encode() + self._write_response(code, [("Content-Type", "application/json")], body) + + def _write_response( + self, + code: int, + headers: list[tuple[str, str]], + body: bytes, + ) -> None: + """Write a full HTTP response, swallowing client-hangup errors. + + Podman's shell driver uses ``curl -sf`` which closes the socket + as soon as it sees a non-2xx status, so the server routinely + races the client on error responses. ``BrokenPipeError`` and + ``ConnectionResetError`` from the peer are normal conditions + for any HTTP server and must not escape as tracebacks. + """ self.send_response(code) - self.send_header("Content-Type", "application/json") + for name, value in headers: + self.send_header(name, value) self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) + try: + self.end_headers() + self.wfile.write(body) + except BrokenPipeError, ConnectionResetError: + pass def log_message(self, format: str, *args: object) -> None: # noqa: A002 """Suppress per-request logging.""" diff --git a/tests/test_serve.py b/tests/test_serve.py index 248e485..c4eb42d 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -186,3 +186,35 @@ def test_timing_safe_comparison_used(self, tmp_path: Path) -> None: ) h.do_GET() assert h._status == 401 + + +class _BrokenWriter: + """wfile stand-in that fails on every write, like a disconnected peer.""" + + def __init__(self, exc: type[OSError]) -> None: + self._exc = exc + + def write(self, _data: bytes) -> None: + raise self._exc(32, "peer gone") + + +class TestClientHangup: + def test_respond_error_swallows_broken_pipe(self, tmp_path: Path) -> None: + """A 401 to a disconnected curl must not escape as BrokenPipeError.""" + handler_cls = _make_test_handler(tmp_path, token="abcdefgh1234") + h = handler_cls("/list", headers={}) + h.wfile = _BrokenWriter(BrokenPipeError) # type: ignore[assignment] + + h.do_GET() + + assert h._status == 401 + + def test_respond_success_swallows_connection_reset(self, tmp_path: Path) -> None: + """A 200 response raced by peer reset must also be swallowed.""" + handler_cls = _make_test_handler(tmp_path, token=None) + h = handler_cls("/healthz", headers={}) + h.wfile = _BrokenWriter(ConnectionResetError) # type: ignore[assignment] + + h.do_GET() + + assert h._status == 200