Cap request body size (HTTP 413)
LimitRequestBodyMiddleware (pure ASGI, registered outermost) rejects request bodies larger than PIRATS_MAX_BODY_BYTES (default 1 MiB) before they're buffered into memory: it checks the declared Content-Length first, then counts the bytes actually streamed so a chunked/length-omitting client can't bypass the header check. Exposed as services.pirats.maxBodyBytes and documented in the README. Tested in isolation (Content-Length fast path + streamed path) and through the real app. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -73,6 +73,7 @@ Once running, access the web UI at [http://localhost:8000](http://localhost:8000
|
|||||||
| `PIRATS_PURGE_INTERVAL_HOURS` | `24` | How often the purge task runs. |
|
| `PIRATS_PURGE_INTERVAL_HOURS` | `24` | How often the purge task runs. |
|
||||||
| `PIRATS_PURGE_FINISHED_DAYS` | `14` | Purge games that finished more than this many days ago. |
|
| `PIRATS_PURGE_FINISHED_DAYS` | `14` | Purge games that finished more than this many days ago. |
|
||||||
| `PIRATS_PURGE_INACTIVE_DAYS` | `30` | Purge games with no activity for this many days. |
|
| `PIRATS_PURGE_INACTIVE_DAYS` | `30` | Purge games with no activity for this many days. |
|
||||||
|
| `PIRATS_MAX_BODY_BYTES` | `1048576` | Reject request bodies larger than this (HTTP 413). |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -128,6 +128,11 @@
|
|||||||
default = "info";
|
default = "info";
|
||||||
description = "Minimum severity of log messages to record.";
|
description = "Minimum severity of log messages to record.";
|
||||||
};
|
};
|
||||||
|
maxBodyBytes = lib.mkOption {
|
||||||
|
type = lib.types.ints.positive;
|
||||||
|
default = 1048576;
|
||||||
|
description = "Reject request bodies larger than this many bytes (HTTP 413).";
|
||||||
|
};
|
||||||
openFirewall = lib.mkOption {
|
openFirewall = lib.mkOption {
|
||||||
type = lib.types.bool;
|
type = lib.types.bool;
|
||||||
default = false;
|
default = false;
|
||||||
@@ -181,6 +186,7 @@
|
|||||||
PIRATS_PURGE_INTERVAL_HOURS = toString cfg.purge.intervalHours;
|
PIRATS_PURGE_INTERVAL_HOURS = toString cfg.purge.intervalHours;
|
||||||
PIRATS_PURGE_FINISHED_DAYS = toString cfg.purge.finishedDays;
|
PIRATS_PURGE_FINISHED_DAYS = toString cfg.purge.finishedDays;
|
||||||
PIRATS_PURGE_INACTIVE_DAYS = toString cfg.purge.inactiveDays;
|
PIRATS_PURGE_INACTIVE_DAYS = toString cfg.purge.inactiveDays;
|
||||||
|
PIRATS_MAX_BODY_BYTES = toString cfg.maxBodyBytes;
|
||||||
};
|
};
|
||||||
|
|
||||||
serviceConfig = {
|
serviceConfig = {
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
// - Add a CHANGELOG entry only when a commit changes something players can
|
// - Add a CHANGELOG entry only when a commit changes something players can
|
||||||
// see. Skip refactors, tests, and tooling. Keep wording player-facing.
|
// see. Skip refactors, tests, and tooling. Keep wording player-facing.
|
||||||
|
|
||||||
export const VERSION = 11;
|
export const VERSION = 12;
|
||||||
|
|
||||||
// Newest first. Each entry: { version, date: 'YYYY-MM-DD', changes: [string, ...] }.
|
// Newest first. Each entry: { version, date: 'YYYY-MM-DD', changes: [string, ...] }.
|
||||||
export const CHANGELOG = [
|
export const CHANGELOG = [
|
||||||
|
|||||||
@@ -67,6 +67,90 @@ def configure_logging() -> None:
|
|||||||
# A bad/unwritable path mustn't take the server down — keep stderr.
|
# A bad/unwritable path mustn't take the server down — keep stderr.
|
||||||
logger.exception("Could not open log file %s; logging to stderr only", log_file)
|
logger.exception("Could not open log file %s; logging to stderr only", log_file)
|
||||||
|
|
||||||
|
DEFAULT_MAX_BODY_BYTES = 1024 * 1024 # 1 MiB — far above any legitimate form post
|
||||||
|
|
||||||
|
def max_body_bytes() -> int:
|
||||||
|
"""Largest request body to accept, from PIRATS_MAX_BODY_BYTES (default 1 MiB)."""
|
||||||
|
val = os.environ.get("PIRATS_MAX_BODY_BYTES")
|
||||||
|
if val is None:
|
||||||
|
return DEFAULT_MAX_BODY_BYTES
|
||||||
|
try:
|
||||||
|
return max(int(val), 0)
|
||||||
|
except ValueError:
|
||||||
|
logger.warning("Invalid PIRATS_MAX_BODY_BYTES=%r; using default", val)
|
||||||
|
return DEFAULT_MAX_BODY_BYTES
|
||||||
|
|
||||||
|
|
||||||
|
class _BodyTooLarge(Exception):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class LimitRequestBodyMiddleware:
|
||||||
|
"""Reject request bodies larger than `max_bytes` with HTTP 413 before they are
|
||||||
|
buffered into memory. The app is on the open internet, so this caps the damage
|
||||||
|
a single oversized POST can do. The declared Content-Length is checked up front
|
||||||
|
(the common case); the bytes actually streamed are then counted too, so a
|
||||||
|
chunked or length-omitting client can't slip past the header check."""
|
||||||
|
|
||||||
|
def __init__(self, app, max_bytes: int):
|
||||||
|
self.app = app
|
||||||
|
self.max_bytes = max_bytes
|
||||||
|
|
||||||
|
async def __call__(self, scope, receive, send):
|
||||||
|
if scope["type"] != "http":
|
||||||
|
await self.app(scope, receive, send)
|
||||||
|
return
|
||||||
|
|
||||||
|
for name, value in scope.get("headers", []):
|
||||||
|
if name == b"content-length":
|
||||||
|
try:
|
||||||
|
declared = int(value)
|
||||||
|
except ValueError:
|
||||||
|
break
|
||||||
|
if declared > self.max_bytes:
|
||||||
|
await self._send_413(send)
|
||||||
|
return
|
||||||
|
break
|
||||||
|
|
||||||
|
received = 0
|
||||||
|
response_started = False
|
||||||
|
|
||||||
|
async def capped_receive():
|
||||||
|
nonlocal received
|
||||||
|
message = await receive()
|
||||||
|
if message["type"] == "http.request":
|
||||||
|
received += len(message.get("body", b""))
|
||||||
|
if received > self.max_bytes:
|
||||||
|
raise _BodyTooLarge()
|
||||||
|
return message
|
||||||
|
|
||||||
|
async def watched_send(message):
|
||||||
|
nonlocal response_started
|
||||||
|
if message["type"] == "http.response.start":
|
||||||
|
response_started = True
|
||||||
|
await send(message)
|
||||||
|
|
||||||
|
try:
|
||||||
|
await self.app(scope, capped_receive, watched_send)
|
||||||
|
except _BodyTooLarge:
|
||||||
|
# The handler reads the whole body before responding, so nothing has
|
||||||
|
# been sent yet; if a response somehow already started, we can only stop.
|
||||||
|
if not response_started:
|
||||||
|
await self._send_413(send)
|
||||||
|
|
||||||
|
async def _send_413(self, send):
|
||||||
|
body = b'{"error":"Request body too large."}'
|
||||||
|
await send({
|
||||||
|
"type": "http.response.start",
|
||||||
|
"status": 413,
|
||||||
|
"headers": [
|
||||||
|
(b"content-type", b"application/json"),
|
||||||
|
(b"content-length", str(len(body)).encode()),
|
||||||
|
],
|
||||||
|
})
|
||||||
|
await send({"type": "http.response.body", "body": body})
|
||||||
|
|
||||||
|
|
||||||
@asynccontextmanager
|
@asynccontextmanager
|
||||||
async def lifespan(app: FastAPI):
|
async def lifespan(app: FastAPI):
|
||||||
configure_logging()
|
configure_logging()
|
||||||
@@ -131,6 +215,10 @@ async def broadcast_state_changes(request, call_next):
|
|||||||
await manager.broadcast(game_id, {"type": "state_changed"})
|
await manager.broadcast(game_id, {"type": "state_changed"})
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
# Added last so it wraps outermost: oversized bodies are rejected before any
|
||||||
|
# other middleware or the route handler buffers them.
|
||||||
|
app.add_middleware(LimitRequestBodyMiddleware, max_bytes=max_body_bytes())
|
||||||
|
|
||||||
@app.websocket("/api/game/{game_id}/ws")
|
@app.websocket("/api/game/{game_id}/ws")
|
||||||
async def game_websocket(websocket: WebSocket, game_id: str):
|
async def game_websocket(websocket: WebSocket, game_id: str):
|
||||||
await manager.connect(game_id, websocket)
|
await manager.connect(game_id, websocket)
|
||||||
|
|||||||
@@ -2525,3 +2525,47 @@ def test_create_and_join_sanitize_names():
|
|||||||
assert "\x01" not in player.name
|
assert "\x01" not in player.name
|
||||||
finally:
|
finally:
|
||||||
app.dependency_overrides.clear()
|
app.dependency_overrides.clear()
|
||||||
|
|
||||||
|
|
||||||
|
def test_request_body_size_limit_middleware():
|
||||||
|
from fastapi import FastAPI, Request
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from pirats.main import LimitRequestBodyMiddleware
|
||||||
|
|
||||||
|
app = FastAPI()
|
||||||
|
app.add_middleware(LimitRequestBodyMiddleware, max_bytes=1000)
|
||||||
|
|
||||||
|
@app.post("/echo")
|
||||||
|
async def echo(request: Request):
|
||||||
|
body = await request.body()
|
||||||
|
return {"len": len(body)}
|
||||||
|
|
||||||
|
client = TestClient(app)
|
||||||
|
|
||||||
|
# Under the limit: passes through and the handler sees the full body.
|
||||||
|
r = client.post("/echo", content=b"x" * 500)
|
||||||
|
assert r.status_code == 200
|
||||||
|
assert r.json()["len"] == 500
|
||||||
|
|
||||||
|
# Over the limit with a declared Content-Length: rejected up front.
|
||||||
|
r = client.post("/echo", content=b"x" * 5000)
|
||||||
|
assert r.status_code == 413
|
||||||
|
|
||||||
|
# Over the limit via a streamed (length-omitting) body: rejected once the
|
||||||
|
# counted bytes exceed the cap.
|
||||||
|
def oversized_chunks():
|
||||||
|
for _ in range(10):
|
||||||
|
yield b"x" * 500 # 5000 bytes total
|
||||||
|
|
||||||
|
r = client.post("/echo", content=oversized_chunks())
|
||||||
|
assert r.status_code == 413
|
||||||
|
|
||||||
|
|
||||||
|
def test_request_body_size_limit_on_real_app():
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from pirats.main import app, DEFAULT_MAX_BODY_BYTES
|
||||||
|
|
||||||
|
# No DB override needed: the middleware rejects before routing/dependencies.
|
||||||
|
client = TestClient(app)
|
||||||
|
r = client.post("/api/game", content=b"x" * (DEFAULT_MAX_BODY_BYTES + 1))
|
||||||
|
assert r.status_code == 413
|
||||||
|
|||||||
Reference in New Issue
Block a user