Browse Source

Docs: mark R01-N24 fixed in 821122d

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 2 ngày trước cách đây
mục cha
commit
abe9595f77
1 tập tin đã thay đổi với 41 bổ sung5 xóa
  1. 41 5
      doc/REVIEW_01.md

+ 41 - 5
doc/REVIEW_01.md

@@ -860,11 +860,47 @@ note. Do not delete entries — they're history.
 
 ### R01-N24 — No request-body size limit on JSON endpoints
 - **Severity**: MEDIUM.
-- **Status**: open.
-- **Where**: `src/Http/Request.php` lines 52-62, 84-101.
-- **What**: `file_get_contents('php://input')` reads whatever PHP's
-  `post_max_size` allows (default 8 MB). `json_decode` with depth=64 is
-  reasonable, but a 7 MB JSON list with 1M entries will be parsed before
+- **Status**: fixed-in-`821122d` — went with both bullet points
+  from the audit's Suggested-fix list. `Request::MAX_BODY_BYTES =
+  1 MiB`; `Request::fromGlobals()` now checks `Content-Length`
+  before reading `php://input`, so an oversized body is never
+  loaded into PHP memory in the first place (with a post-read
+  length check as defence-in-depth for clients that under-report
+  or omit the header). The decision rides through to the new
+  `bodyTooLarge` constructor flag, which `public/index.php`
+  consults BEFORE the HTTPS redirect — wasting a TLS round trip
+  on something we'd reject anyway is a worse experience than a
+  fast 413. The 413 ships as the standard
+  `{ok: false, error: { code: 'request_too_large', … }}`
+  envelope; HTML form clients (which would already be in
+  pathological territory at >1 MiB) see the JSON body in the
+  browser, an acceptable trade-off vs. plumbing a separate
+  HTML branch. Per-batch cap: `MAX_BATCH_ITEMS = 5000` on both
+  `SprintController` and `TaskController`. The four list-bodied
+  endpoints — `updateWeekCells`, `updateAssignments`,
+  `updateAssignmentsStatus`, and tasks `reorder` — reject past
+  the cap with `too_many_items` 413. Real workloads sit
+  comfortably under: a sprint has ≤26 weeks (schema cap) and a
+  realistic worker count, so the wall is well above any genuine
+  UI flow but dodges the "1 MiB body packed with tens of
+  thousands of tiny cells" path that would still keep one
+  transaction long-running. New `RequestTest` cases (4) pin the
+  constant, the constructor flag default + wiring, and the
+  `json()`-returns-null contract when `bodyTooLarge` is set.
+  `SprintControllerTest` + new `TaskControllerTest` pin
+  `MAX_BATCH_ITEMS = 5000` and the cross-controller drift fence
+  (the two caps must stay in lockstep). Tests: 331 / 860 (was
+  325 / 853).
+- **Where**:
+  - `src/Http/Request.php` — `MAX_BODY_BYTES`, `bodyTooLarge`
+    flag, `fromGlobals()` short-circuit.
+  - `public/index.php` — pre-dispatch 413.
+  - `src/Controllers/SprintController.php`,
+    `src/Controllers/TaskController.php` — `MAX_BATCH_ITEMS` +
+    per-endpoint guards.
+- **What**: `file_get_contents('php://input')` read whatever PHP's
+  `post_max_size` allowed (default 8 MB). `json_decode` with depth=64 is
+  reasonable, but a 7 MB JSON list with 1M entries was parsed before
   any controller-level cap.
 - **Why it matters**: Any admin can DoS the server by repeatedly POST'ing
   large bodies. Insider-only, but easy to do accidentally with a misbehaving