|
2 | 2 |
|
3 | 3 | Items raised in reviews that are real but not actionable now. |
4 | 4 |
|
| 5 | +## Deferred from: code review of story-1-4 (2026-05-14) |
| 6 | + |
| 7 | +- **Unbounded error body size** — `StatusError.body` holds the full `resp.content` with no cap; large 5xx pages stay pinned in memory through exception lifetimes (Sentry payloads, logs, retained tracebacks). Revisit with retry/observability middleware. (`src/httpware/transports/httpx2.py:117-124`) |
| 8 | +- **`httpx2.StreamError` family escape** — `StreamError` and its children (`StreamConsumed`, `StreamClosed`, `ResponseNotRead`, `RequestNotRead`) are `RuntimeError` subclasses, not `HTTPError`; not caught by the seam's `except httpx2.HTTPError`. Unreachable via the default httpx2 config (no redirects, no retries) but exploitable through user-supplied clients with retry layers. Revisit when retry middleware lands. (`src/httpware/transports/httpx2.py:103-108`) |
| 9 | +- **Header CRLF / log-injection** — extends the existing URL CRLF deferral. `dict(request.headers)` forwards values verbatim including embedded `\r\n`; full sanitization lands with the `Redactor` middleware (Story 5.3). (`src/httpware/transports/httpx2.py:94`) |
| 10 | +- **Userinfo on `StatusError.request_url` raw field** — `__repr__` and the exception summary strip `user:pass@`, but the field itself retains credentials, leaking through structured-logging serializers. Defense-in-depth strip is the Redactor's job (Story 5.3) per `errors.py` docstring. (`src/httpware/transports/httpx2.py:123`) |
| 11 | +- **Concurrent `aclose()` ↔ `__call__` races** — no synchronization between in-flight `client.send` and a parallel `aclose`. Best case raises `RuntimeError`; worst case completes on a partly-disposed pool. Broader concurrency / lifecycle design; defer to Story 1.7 or retry stories. (`src/httpware/transports/httpx2.py:87-145`) |
| 12 | + |
| 13 | +## Deferred from: code review of story-1-4 (2026-05-13) |
| 14 | + |
| 15 | +- **URL CRLF / log-injection** — relying on httpx2's `InvalidURL` validation; explicit `Redactor`-level sanitization deferred to Story 5.3. |
| 16 | +- **`request.method` validation beyond uppercasing** — httpx2 surfaces `LocalProtocolError` for malformed methods; no further mitigation in `transports/httpx2.py`. |
| 17 | +- **Case-insensitive header type + multi-valued header collapse** — `Mapping[str, str]` with lowercase ASCII keys is the v0 contract. Two limitations bundled: (a) case-insensitive lookup unavailable; (b) `dict(resp.headers)` collapses duplicate-key headers like `Set-Cookie`, `Via`, `Link` to the last value only. Revisit together when real header-handling middleware (Story 2.3 era) demands either capability — the contract widens to `Mapping[str, Sequence[str]]` (or similar) at that point. |
| 18 | + |
5 | 19 | ## Deferred from: code review of story-1-3 (2026-05-13) |
6 | 20 |
|
7 | | -- **`request_method` / `request_url` CRLF / log-injection** — fields stored verbatim; `repr` echoes embedded `\r\n`. Transport seam should validate before crafting the request. (`src/httpware/errors.py:55,58`) — revisit in Story 1.4. |
8 | | -- **Header case-folding contract** — `Mapping[str, str]` doesn't pin case-sensitivity. `exc.headers["Content-Type"]` may `KeyError` if `httpx2.Headers` hands in lowercased keys. (`src/httpware/errors.py:33`) — Story 1.4 transport seam decides. |
9 | | -- **`request_method` casing normalization** — `repr` faithfully echoes `"get"` / `"Get"` if middleware lowercases. Document expected casing or normalize at the seam. (`src/httpware/errors.py:35`) — Story 1.4. |
| 21 | +Resolved by Story 1.4: method uppercased at seam; httpx2 returns lowercased headers (v0 contract); CRLF mitigation via `httpx2.InvalidURL`. |
10 | 22 |
|
11 | 23 | ## Deferred from: code review of story-1-2 (2026-05-13) |
12 | 24 |
|
|
0 commit comments