Merge pull request 'fix(tradein/security): photo-upload — чанковое чтение с капом 10MB + Caddy request_body 12MB (#2233)' (#2258) from fix/tradein-photo-upload-stream-cap into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 12s
Deploy Trade-In / build-browser (push) Successful in 38s
Deploy Trade-In / build-frontend (push) Successful in 40s
Deploy Trade-In / test (push) Successful in 1m48s
Deploy Trade-In / build-backend (push) Successful in 58s
Deploy Trade-In / deploy (push) Successful in 56s
All checks were successful
Deploy Trade-In / changes (push) Successful in 12s
Deploy Trade-In / build-browser (push) Successful in 38s
Deploy Trade-In / build-frontend (push) Successful in 40s
Deploy Trade-In / test (push) Successful in 1m48s
Deploy Trade-In / build-backend (push) Successful in 58s
Deploy Trade-In / deploy (push) Successful in 56s
This commit is contained in:
commit
910bfed5f1
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})"
|
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:
|
if not content:
|
||||||
raise HTTPException(status_code=400, detail="empty file")
|
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,
|
# Sanitize: re-encode through Pillow to drop EXIF, kill polyglot payloads,
|
||||||
# cap dimensions. Closes finding #6 from 2026-05-24 audit.
|
# 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
|
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) ────────
|
# ── Derived analytics routes (representative: /houses, /imv-benchmark) ────────
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -15,6 +15,13 @@
|
||||||
handle_path /trade-in/api/* {
|
handle_path /trade-in/api/* {
|
||||||
# handle_path вырезает префикс /trade-in перед прокси →
|
# handle_path вырезает префикс /trade-in перед прокси →
|
||||||
# FastAPI получает /api/v1/...
|
# 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
|
reverse_proxy tradein-backend:8000
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue