fix(tradein/security): photo-upload — чанковое чтение с капом 10MB + Caddy request_body 12MB (#2233) #2258
3 changed files with 81 additions and 3 deletions
|
|
@ -518,11 +518,19 @@ async def upload_photo(
|
|||
status_code=409, detail=f"photo limit reached ({_MAX_PHOTOS_PER_ESTIMATE})"
|
||||
)
|
||||
|
||||
content = await file.read()
|
||||
# #2233: читаем тело чанками с жёстким капом, а не await file.read() целиком —
|
||||
# иначе multi-GB аплоад буферизуется в RAM и OOM-killed backend (mem_limit 768m, #2214).
|
||||
# 413 бросается СРАЗУ при превышении, остаток тела запроса НЕ читается.
|
||||
chunks: list[bytes] = []
|
||||
total = 0
|
||||
while chunk := await file.read(64 * 1024):
|
||||
total += len(chunk)
|
||||
if total > _MAX_PHOTO_BYTES:
|
||||
raise HTTPException(status_code=413, detail="file too large (max 10 MB)")
|
||||
chunks.append(chunk)
|
||||
content = b"".join(chunks)
|
||||
if not content:
|
||||
raise HTTPException(status_code=400, detail="empty file")
|
||||
if len(content) > _MAX_PHOTO_BYTES:
|
||||
raise HTTPException(status_code=413, detail="file too large (max 10 MB)")
|
||||
|
||||
# Sanitize: re-encode through Pillow to drop EXIF, kill polyglot payloads,
|
||||
# cap dimensions. Closes finding #6 from 2026-05-24 audit.
|
||||
|
|
|
|||
|
|
@ -491,6 +491,69 @@ def test_upload_photo_owner_can_upload(trade_in_app: FastAPI, monkeypatch) -> No
|
|||
assert resp.status_code == 200
|
||||
|
||||
|
||||
class _InfiniteUploadFile:
|
||||
"""UploadFile stub whose async read(size) yields endless 64KB chunks (#2233).
|
||||
|
||||
Simulates a multi-GB stream. Counts read() calls so the test can assert the
|
||||
handler stops reading right after the cap is exceeded instead of draining the
|
||||
whole body into RAM.
|
||||
"""
|
||||
|
||||
content_type = "image/png"
|
||||
filename = "huge.png"
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.read_calls = 0
|
||||
|
||||
async def read(self, size: int = -1) -> bytes:
|
||||
self.read_calls += 1
|
||||
return b"\x00" * (64 * 1024)
|
||||
|
||||
|
||||
async def test_upload_photo_streams_over_cap_returns_413_without_full_read() -> None:
|
||||
"""#2233: a stream larger than 10 MB → 413 raised early, body NOT fully read.
|
||||
|
||||
Acceptance: read() is called only ~(10MB/64KB)+1 times (161), proving the loop
|
||||
bails on the first chunk that pushes total past _MAX_PHOTO_BYTES rather than
|
||||
buffering an unbounded upload (which would OOM the 768m-capped container, #2214).
|
||||
"""
|
||||
from uuid import UUID
|
||||
|
||||
from fastapi import HTTPException
|
||||
|
||||
import app.core.auth as auth_mod
|
||||
from app.api.v1.trade_in import _MAX_PHOTO_BYTES, upload_photo
|
||||
|
||||
auth_mod.get_role = lambda _u: "pilot" # type: ignore[assignment]
|
||||
|
||||
# guard SELECT → owner; count SELECT → 0. INSERT must never be reached.
|
||||
db = MagicMock()
|
||||
guard_result = MagicMock()
|
||||
guard_result.fetchone.return_value = SimpleNamespace(created_by="kopylov")
|
||||
count_result = MagicMock()
|
||||
count_result.scalar_one.return_value = 0
|
||||
db.execute.side_effect = [guard_result, count_result]
|
||||
|
||||
upload = _InfiniteUploadFile()
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await upload_photo(
|
||||
estimate_id=UUID(_ESTIMATE_ID),
|
||||
db=db,
|
||||
file=upload, # type: ignore[arg-type]
|
||||
x_authenticated_user="kopylov",
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 413
|
||||
expected_reads = _MAX_PHOTO_BYTES // (64 * 1024) + 1 # 161
|
||||
assert upload.read_calls == expected_reads
|
||||
# sanity: bounded, nowhere near infinite
|
||||
assert upload.read_calls < 200
|
||||
# INSERT (3rd execute) never fired — nothing persisted
|
||||
assert db.execute.call_count == 2
|
||||
db.commit.assert_not_called()
|
||||
|
||||
|
||||
# ── Derived analytics routes (representative: /houses, /imv-benchmark) ────────
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,13 @@
|
|||
handle_path /trade-in/api/* {
|
||||
# handle_path вырезает префикс /trade-in перед прокси →
|
||||
# FastAPI получает /api/v1/...
|
||||
#
|
||||
# #2233: первый рубеж против OOM на фото-аплоаде — режем тело >12MB на
|
||||
# уровне Caddy (backend капит на 10MB чанково, см. upload_photo). Scoped
|
||||
# ТОЛЬКО на /trade-in/api/* — не трогает фронт/остальной сайт.
|
||||
request_body {
|
||||
max_size 12MB
|
||||
}
|
||||
reverse_proxy tradein-backend:8000
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue