diff --git a/README.md b/README.md index 4d3bd14..275260a 100644 --- a/README.md +++ b/README.md @@ -260,6 +260,46 @@ that got the request this far has already completed, never a forced handshake. `WebSocketSession` mirrors this exactly (`isSecure()`, `sslSession()`) by delegating to the upgrading `Request` — no separate TLS state is tracked for WS. +## Object lifetime + +`Request` and `Response` are **pooled per connection**, not allocated per request: one instance is +created per connection and repositioned (`reset()`) over each new request/response in turn — the +same idiom Java NIO buffers use, applied to the whole request/response model +(`flash/docs/http2/MESSAGE-MODEL.md` has the full design record). This is what makes a warm h1 +request/response cycle 0 B/op. + +**Do not retain a `Request` or `Response` past the handler that received it.** A reference kept in +a field, a captured closure, a `CompletableFuture` continuation, or a background thread and read +*after* the handler returns will observe whatever the *next* request on that connection +repositioned the same instance to — not the request you thought you had: + +```java +// WRONG — captures `req`, reads it after the handler has returned +app.get("/slow", (req, res) -> { + CompletableFuture.runAsync(() -> log(req.header("X-Trace-Id"))); // may log the NEXT request's header + return "ok"; +}); +``` + +Copy out whatever you need before returning or handing work off asynchronously — every accessor +that returns a `String` (`header`, `param`, `query`, `path`, …) gives you an independent heap copy +that's safe to keep as long as you like: + +```java +app.get("/slow", (req, res) -> { + String traceId = req.header("X-Trace-Id"); // copy now, safe to retain + CompletableFuture.runAsync(() -> log(traceId)); + return "ok"; +}); +``` + +Run with `-Dflash.env=dev` and a use-after-return access throws `IllegalStateException` immediately +at the offending call site instead of silently reading the wrong request's data — turn this on in +tests and local development. It's a no-op in production beyond a single `boolean` field read. + +`req.body()`/`RequestBody` follows the same rule — materialise (`.bytes()`) or fully consume +(`.stream()`) it inside the handler; don't stash the `RequestBody` itself for later. + ## Architecture ``` diff --git a/flash/docs/http2/DECISIONS.md b/flash/docs/http2/DECISIONS.md index d547e3a..fd37880 100644 --- a/flash/docs/http2/DECISIONS.md +++ b/flash/docs/http2/DECISIONS.md @@ -765,3 +765,103 @@ or `FrameWriteBuffer` are ever modified in a way that could plausibly affect the profile. --- + +## DEC-22 — `HeaderMap` splits into `HeaderView` (interface) + `Http1HeaderMap` (impl, staying in `models`, not moving to `http1`) + +**Context.** Phase 6 task 1 requires splitting the concrete `HeaderMap` class into a +protocol-neutral read contract (so a future `Http2HeaderMap` can implement it) plus the existing +h1 byte-buffer-backed implementation, and explicitly asks for two decisions to be recorded: +whether the public-facing name stays `HeaderMap` or moves to the interface, and (implicitly, via +the plan's own Files list) whether the concrete class moves to `dev.relism.flash.http1`. + +**Decision 1 — naming.** Checked whether `HeaderMap` is actually part of `Request`'s public +surface first, since the task's hard constraint is "the public API of `Request` must not +change": `Request`'s own methods (`header`, `headers`, `param`, `query`) return `String`/ +`List`, never a `HeaderMap`/`HeaderView` — the only exposure is the transitive, +Javadoc'd-as-"Internal" `Request.getRequestLine().getHeaders()` path. Concluded the type name +itself is not public API in the sense the constraint cares about, so took the plan's Files list +literally: new interface named `HeaderView` (the read contract), concrete implementation renamed +`Http1HeaderMap`. `RequestLine.headers` (and its Lombok-generated `getHeaders()`) is now typed +`HeaderView`. + +**Decision 2 — package placement.** The plan's Files list suggests `http1/Http1HeaderMap.java`. +Verified first (as `DEC-19` did for the same class of question): `RequestParser`, which owns and +resets the one `Http1HeaderMap` instance per connection, lives in the root `dev.relism.flash` +package, not `http1`. `http1` already depends on root (`Http1Connection` imports +`RequestParser`); moving the header-map implementation into `http1` would require root to import +back from `http1` for `RequestParser` to construct one — the same reverse-edge problem `DEC-19` +found and avoided for `routing`/`transport`. Kept `Http1HeaderMap` in `models` instead, alongside +`HeaderView` — deviating from the plan's literal suggested path, not from its intent. + +**Consequence.** `HeaderView` is the new protocol-neutral interface (`first`, `all`, `view`, +`valueEqualsIgnoreCase`, `contains`, `count`, `forEach`); `contains`/`count` did not exist on the +old `HeaderMap` and were added to satisfy the interface's stated method list. `Http1HeaderMap` +carries the full `EX-09`/`EX-05` implementation unchanged, just renamed and re-typed against the +interface. Every call site across `main` and `test` sources updated (`RequestParser`, test files +constructing header maps directly); `HeaderMapTest`/`HeaderMapIndexTest` renamed to +`Http1HeaderMapTest`/`Http1HeaderMapIndexTest` to match. 449/449 tests green, unchanged count — +this was a pure rename/re-type, no behavior change. + +**Revisit when.** Phase 10, when `Http2HeaderMap` is built — confirms whether `HeaderView`'s +method list is actually sufficient for an HPACK-backed implementation, or needs extending. + +--- + +## DEC-23 — Phase 6 closes `DEC-20`'s revisit loop: the h1 zero-alloc contract, re-measured after `Request`/`RequestBody`/`RequestLine`/`Response` pooling, plus one more allocation found and fixed (`EX-42`) + +**Context.** `DEC-20` (Phase 4) measured `RequestPipelineBenchmark.parseAndRoute` at 120.008 B/op +and attributed it entirely to `Request`/`RequestBody`/`RequestLine` construction, explicitly +deferring the fix to Phase 6 and asking for a re-run once that pooling landed. Phase 6 tasks 2–7 +(`EX-20`–`EX-24`) did that pooling; this entry is the promised re-run (same JDK 21.0.11, JMH 1.37, +`avgt` mode, `-prof gc`, `flash/src/jmh/java`, same fixture: `GET /users/12345 HTTP/1.1` with +`Host`/`Accept`/`Authorization`). + +**First re-run, after `EX-20`–`EX-24` alone:** + +| | ns/op | B/op | +|---|---|---| +| `parseAndRoute` | 1194.105 ± 944.469 | 48.008 | +| `parseRouteAndExtractThreeFields` | 1324.679 ± 296.883 | 232.009 | + +Down from 120.008 to 48.008 B/op — real progress, but not the 0 B/op the phase's own DoD text +requires for `parseAndRoute` (no header/param access). Investigated rather than accepted: reading +`RequestParser.parse` line by line turned up three `new FastPathViews.RequestByteView(...)` +allocations (path, query when present, protocol) on every call — pre-existing since at least Phase +4, just smaller than the `Request`/`RequestBody`/`RequestLine` cost `DEC-20` measured and therefore +invisible until this phase's pooling removed the larger cost sitting on top of it. Registered as +`EX-42` and fixed the same way every other per-connection object in this codebase already is: +`RequestByteView` gained a `reset(byte[], int, int)`, `RequestParser` now owns one pooled instance +per role instead of allocating fresh ones. + +**Second re-run, after `EX-42`:** + +| | ns/op | B/op | +|---|---|---| +| `parseAndRoute` | 1111.260 ± 104.692 | 0.008 | +| `parseRouteAndExtractThreeFields` | 1301.840 ± 228.068 | 184.009 | + +`parseAndRoute` — 0.008 B/op is JMH's noise floor (a `-prof gc` sampling artifact, not a real +allocation); this is the 0 B/op the contract asks for. `parseRouteAndExtractThreeFields` dropped +from 232.009 to 184.009 B/op — the exact 48 bytes `EX-42` removed, confirming the fix's accounting +and leaving only the "user-facing `String`s the handler explicitly asks for" the contract's own +text carves out (one path param, two headers — three `String` allocations plus their backing +`byte[]`s). + +**Decision.** The h1 zero-alloc contract is met: `parseAndRoute` (parse + route with a parametric +match) is 0 B/op; the residual cost in `parseRouteAndExtractThreeFields` is entirely the explicit +`String` reads the DoD text itself exempts. `DEC-20`'s revisit item is closed. + +**Consequence.** `RequestByteView`'s public 3-arg constructor is unchanged (still used for +one-shot views by tests, `AbstractWsRouter`, `ErrorPagesTest`, etc.) — only `RequestParser`'s three +call sites moved to the pooled `reset()` path. `queryView` is only reset and wired into +`RequestLine` when a query string is actually present, preserving +`RequestLine.getQuery()`'s existing `null`-means-absent contract — verified by +`RequestParserTest.samePooledParser_secondRequestWithoutQuery_doesNotLeakFirstRequestsQuery`, the +pooling-leak class of test this codebase writes for every pooled object (`RequestPoolingTest`, +`ResponsePoolingTest`, `RequestBodyTest`'s new pooling tests). 500/500 tests green. + +**Revisit when.** Never expected to — this closes the loop `DEC-20` opened. If a future phase adds +a fourth per-request view (e.g. an h2 equivalent), extend this same pooled-`reset()` pattern rather +than reintroducing a fresh allocation. + +--- diff --git a/flash/docs/http2/IMPLEMENTATION-PLAN.md b/flash/docs/http2/IMPLEMENTATION-PLAN.md index d74d1c7..1cb2422 100644 --- a/flash/docs/http2/IMPLEMENTATION-PLAN.md +++ b/flash/docs/http2/IMPLEMENTATION-PLAN.md @@ -67,7 +67,7 @@ Status values: `not started` / `in progress` / `blocked` / `done`. | 3 — Serialized frame writer (GO/NO-GO gate) | done | `feature/core/http2` | `Http2FrameWriter`/`WriteIntent`/`IntrusiveMpscQueue` + `Http2FrameWriterTest`/`Http2FrameWriterStressTest` + `FrameWriterBenchmark` (JMH, `-Pjmh`, `src/jmh/java` — moved there from `src/test/java` after it broke default `mvn test`; see `DEC-17`). All 4 gate criteria met: N=1 0 B/op & 42.6 ns overhead (≤50 ns budget); N=64 65.5% throughput retention (≥60%) & 11.8–14.2 µs p999 (<1 ms); no carrier pinning; stress test 10 000/10 000 green (1000 iters × 5 N values × 2 scheduler configs). Full numbers in `WRITER.md`, `DEC-09`. 321/321 non-JMH tests green. | | 4 — Byte-layer foundations | done | `feature/core/http2` | `dev.relism.flash.bytes` package (`ByteScan`+SWAR, `ArrayBackedByteView`, `SegmentedByteView`, `PooledSlice`/`SlicePool`, `ByteWriter`, `Pairs`) built. `EX-04`/`EX-05`/`EX-09`/`EX-19`/`EX-25`/`EX-26`/`EX-33` done, plus `EX-06`'s router half (plan correction, `DEC-19`) removing `FastPathRouterImpl`/`FastPathWsRouterImpl`'s `ThreadLocal`s via an opaque per-connection scratch (`AbstractRouter#newScratch`) instead of extending `ConnectionScratch` (would have created a `routing`→`transport` package cycle). `AbstractRouter`/`AbstractWsRouter.route()` gained a `scratch` param — all call sites updated. Measured (`DEC-20`): SWAR scan 35.4% faster (kept), `EX-04`'s word-path 32.1% faster at the mechanism level (kept; today's router doesn't route through it — `MethodPathByteView` stays non-array-backed by design). Router matching itself is ≈0 B/op including parametric routes. Full h1 pipeline is 120.008 B/op, 100% attributable to `Request`/`RequestBody`/`RequestLine` construction — explicitly Phase 6 scope, not a Phase 4 regression. Two documented (non-hot-path) anonymous-`ByteView` fallbacks remain in `QueryParams`/`PathParams.view`. `BYTES.md` written. 395/395 tests green (both with and without `-Pjmh`). | | 5 — Frame layer | done | `feature/core/http2` | `FrameType`/`FrameFlags`/`FrameHeader`/`Http2FrameReader`/`FrameValidator`/`Padding`/`FrameWriteBuffer` built. All 10 frame types read/validated/written; per-type RFC error codes verified individually (`FrameValidatorTest`); fuzz-tested 10M random inputs (~14s, green). Zero-alloc contract measured, not asserted: read+validate+consume 0.002 B/op, write ≈10⁻⁴ B/op (`DEC-21`). Found+fixed `EX-37` (`BufferedByteSource`'s deadline mechanism NPE'd against a `null` socket — zero prior test coverage of `EX-07`'s own fix; added `BufferedByteSourceTest`). `FRAMES.md` written. 449/449 tests green. | -| 6 — Request/Response model refactor | not started | — | — | +| 6 — Request/Response model refactor | done | `feature/core/http2` | `Request`/`RequestBody`/`RequestLine`/`Response` all pooled per connection (`EX-20`–`EX-24`), same `reset()`/dev-mode-guard idiom as `Http1HeaderMap`. `HeaderMap` split into `HeaderView` (interface) + `Http1HeaderMap` (impl, stays in `models` — `DEC-22`). `Response` gained byte-level structured headers + `PreEncodedHeader`; `ResponseSerializer` is the one source of truth for a response's header sequence, consumed by `Http1ResponseWriter`'s single-bulk-write rewrite (`EX-27`). `ByteTemplate` fixed to O(1) slot lookup + a buffer-writing overload (`EX-28`). `Multipart` audited: found and fixed 3 resource-exhaustion gaps (unbounded buffered part size/part count/per-part header parsing — `EX-38`–`EX-40`), confirmed boundary length already bounded (`EX-41`, non-finding). Re-measuring `RequestPipelineBenchmark` after the pooling work found one more allocation underneath it — `RequestParser` was still building fresh `RequestByteView`s per request — fixed (`EX-42`). Zero-alloc contract closed: `parseAndRoute` 120.008 → 0.008 B/op (`DEC-20`/`DEC-23`). Verifying the DoD's own "Response header region bounded" checkbox found it unimplemented — fixed (`EX-43`). `MESSAGE-MODEL.md` written; README gained an "Object lifetime" section. 503/503 tests green. | | 7 — HPACK decoder | not started | — | — | | 8 — Connection state machine | not started | — | — | | 9 — HPACK encoder + h2 response path | not started | — | — | @@ -654,6 +654,92 @@ working. Production always supplies a real socket, so no production behavior cha mechanism against a `null` socket, closing the actual test gap this bug lived in. **Phase**: 5 (found and fixed while building `Http2FrameReaderTest`). +### EX-38 — `Multipart` buffered a part body with no size bound +Found during the `EX-29` audit (Phase 6). `Multipart.scanNext` buffered text fields — and, during +a full `parts()`/`parts(String)` scan, file bodies too — via the JDK's default +`InputStream.readAllBytes()`, which has no size limit and grows its internal buffer by doubling +for as long as bytes keep arriving. `Http1Limits.MAX_CONTENT_LENGTH` bounds the *whole* request +body at 4 GiB (and does essentially nothing for a chunked body — `MAX_CHUNKS_PER_BODY` × +`MAX_CHUNK_SIZE` allows up to ~1.6 TB), but nothing stopped a single part inside that body from +being eagerly materialized into one heap allocation of whatever size a hostile peer chose to send. +**Fix**: `readBoundedBody` replaces the `readAllBytes()` call, throwing `IOException` once the +part exceeds `Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE` (10 MiB). Deliberately does **not** +apply to `Part.materialize()` on a streaming file part returned by `Multipart.file()` — that call +is documented as an explicit, opt-in heap allocation the caller chooses to pay for. +**Phase**: 6. + +### EX-39 — `Multipart` accepted an unbounded number of parts +Found during the `EX-29` audit. `scanNext` is called in an unbounded loop by `field()`, `file()`, +and `scanAll()`; nothing capped how many parts (`scanned` entries, each backed by a `HashMap` of +its own headers) a single body could contain — the multipart analogue of the chunked-body +`MAX_CHUNKS_PER_BODY` bound. +**Fix**: a `partCount` counter checked against the new `Http1Limits.MAX_MULTIPART_PARTS` (1,000) +at the top of every `scanNext` call. +**Phase**: 6. + +### EX-40 — `Multipart`'s per-part header parsing had no count or line-length bound +Found during the `EX-29` audit. `readPartHeaders` looped until a blank line with no cap on the +number of header lines read, and its `readLine` helper appended to a `StringBuilder` with no cap +on a single line's length — unlike the top-level HTTP headers, which `RequestParser` already +bounds via `Http1Limits.MAX_HEADER_COUNT`/`MAX_HEADER_VALUE_LENGTH`, these per-part header lines +live inside the body and were entirely unguarded. A peer that never sent `\r\n` could grow a +single line's buffer for as long as it kept streaming bytes; a peer sending header lines +indefinitely could grow the per-part `HashMap` without bound. +**Fix**: `readPartHeaders` now rejects a part once it exceeds +`Http1Limits.MAX_MULTIPART_PART_HEADER_COUNT` (20); `readLine` now rejects a line once it exceeds +`Http1Limits.MAX_MULTIPART_HEADER_LINE_LENGTH` (8,192 bytes) — both throw `IOException`. +**Phase**: 6. + +### EX-41 — (non-finding) `Multipart`'s boundary length is already bounded +Checked during the `EX-29` audit, as required by Part I's rules — recorded here because absence +of a bug is easy to mistake for "wasn't checked". The `boundary` parameter comes from the +request's `Content-Type` header value, which `RequestParser` already caps at +`Http1Limits.MAX_HEADER_VALUE_LENGTH` (8,192 bytes) before `Multipart.of` ever sees it — no +separate bound needed in `Multipart` itself. +**Phase**: 6. + +### EX-42 — `RequestParser.parse` still allocated three `RequestByteView`s per request +Found while re-measuring `RequestPipelineBenchmark` at the end of Phase 6, after `EX-20`..`EX-24` +pooled `Request`/`RequestBody`/`RequestLine`/`Response`: `parseAndRoute` (parse + route, no +header/param access — the isolation benchmark `DEC-20` introduced) was still 48.008 B/op, not the +0 B/op Phase 6's own zero-alloc contract requires. `RequestParser.parse` built a fresh +`FastPathViews.RequestByteView` for the path, the query (when present), and the protocol on every +call — `Request`/`RequestBody`/`RequestLine` were the *only* per-request allocations `DEC-20` +measured at Phase 4, but that measurement predates this phase's own pooling work exposing what was +underneath: these three view objects were always there, just masked by the larger R/RB/RL cost. +**Fix**: `RequestByteView` gained a `reset(byte[], int, int)` (mirroring `Http1HeaderMap`/ +`RequestLine`/`RequestBody`'s own `reset` methods) without touching its existing public +constructor (still used for one-shot views elsewhere — tests, `AbstractWsRouter`). `RequestParser` +now owns one pooled instance per role (`pathView`/`queryView`/`protocolView`), repositioned per +request; `queryView` is only reset and wired into `RequestLine` when a query string is actually +present, preserving `RequestLine.getQuery()`'s existing "`null` means no query" contract. +**Result**: `parseAndRoute` measured 0.008 B/op after the fix (noise-floor, effectively 0); +`parseRouteAndExtractThreeFields` (which explicitly reads one path param and two headers — the +DoD text's own "user-facing `String`s the handler explicitly asks for" carve-out) dropped from +232.009 to 184.009 B/op, the same 48 bytes accounted for exactly. +**Phase**: 6. + +### EX-43 — `Response.header(...)` had no bound, unlike every request-side header limit +Found while verifying Phase 6's own DoD checklist, which names this bound explicitly ("Response +header region bounded (`Http1Limits.MAX_RESPONSE_HEADER_BYTES`) — a handler in a loop calling +`header(...)` must not grow the scratch without limit") — a checkbox item, not yet implemented +when checked. `Response.header(String,String)`/`header(PreEncodedHeader)` wrote into `headerRegion` +(a growable `ByteWriter`) and `header(byte[])` appended to `rawHeaderLines`, all three via +`recordHeaderEntry` growing `headerTags`/`headerRefs`, with no upper bound on either the region's +total bytes or the number of `header(...)` calls — unlike every *request*-side header limit +(`MAX_HEADER_COUNT`, `MAX_HEADER_NAME_LENGTH`, `MAX_HEADER_VALUE_LENGTH`), which bound a hostile +peer's input. This is the response-side, application-bug analogue: a handler that calls +`header(...)` in an unbounded loop (e.g. echoing an unbounded collection into headers) would grow +this connection's pooled scratch region without limit for the rest of the connection's lifetime, +since Phase 6's pooling means it is never reallocated back down between requests. +**Fix**: two new limits, `Http1Limits.MAX_RESPONSE_HEADER_BYTES` (64 KiB) and +`MAX_RESPONSE_HEADER_COUNT` (1,000); all three `header(...)` overloads now check the count via a +shared `checkHeaderBudget()`, and the two name/value overloads additionally check the region's +total bytes via `checkHeaderRegionBudget()` after writing. Both throw `IllegalStateException` +(an application-code misuse, not a wire-input rejection, so this deliberately does not go through +`MalformedRequestException`'s HTTP-status-carrying path). +**Phase**: 6. + --- # PART III — The phases @@ -1694,7 +1780,7 @@ layer avoids building the h2 side twice. Nothing here may make the h1 path slower or the user-facing API uglier. ### EX items -`EX-20`, `EX-21`, `EX-22`, `EX-23`, `EX-24`, `EX-27`, `EX-28`, `EX-29`. +`EX-20`, `EX-21`, `EX-22`, `EX-23`, `EX-24`, `EX-27`, `EX-28`, `EX-29`, `EX-38`, `EX-39`, `EX-40`, `EX-41`, `EX-42`, `EX-43`. ### Files @@ -1779,22 +1865,39 @@ path params, read three headers, set two response headers, write a 200 with a by must be **0 B/op**. ### Safety checks -- [ ] Recycled `Request`/`Response`/`RequestBody` fully cleared; no cross-request data leak - (explicit security test: connection A's `Authorization` header must never be visible on - connection B through a recycled object) -- [ ] Dev-mode use-after-recycle detection works and has a test -- [ ] Response header region bounded (`Http1Limits.MAX_RESPONSE_HEADER_BYTES`) — a handler in a - loop calling `header(...)` must not grow the scratch without limit -- [ ] `Multipart` limits enforced +- [x] Recycled `Request`/`Response`/`RequestBody` fully cleared; no cross-request data leak + (explicit security test: `RequestPoolingTest.secondRequest_onSameConnection_doesNotSeeFirstRequestsAuthorizationHeader` + — reframed from "cross-connection" to "cross-request, same connection" since this codebase's + pooling is per-connection, not a shared cross-connection pool; see that test's own class + Javadoc and `RequestParserTest`'s `samePooledParser_*` tests for the `EX-42` view-pooling + leak checks) +- [x] Dev-mode use-after-recycle detection works and has a test + (`RequestRecycleGuardTest`, `ResponseRecycleGuardTest`) +- [x] Response header region bounded (`Http1Limits.MAX_RESPONSE_HEADER_BYTES`/ + `MAX_RESPONSE_HEADER_COUNT`) — a handler in a loop calling `header(...)` must not grow the + scratch without limit (`EX-43`, found while checking this exact box; `ResponseTest`'s + `header_exceeding*` tests) +- [x] `Multipart` limits enforced (`EX-38`–`EX-41`; `MultipartTest`'s "EX-29: resource-exhaustion + bounds" section — kept in the existing test class rather than a separate + `MultipartSecurityTest` file, matching how `RequestParserSecurityTest` is the one exception + elsewhere in this codebase that *does* get its own file, because its request-line-level + concerns don't share fixtures with `RequestParserTest`; `Multipart`'s bounds tests share the + same `body()`/`textPart()`/`filePart()` helpers as its correctness tests) ### Tests - Every existing test in `models/`, `routing/`, `template/`, `api/multipart/` passes. -- `RequestPoolingTest`, `ResponsePoolingTest` — including the cross-connection leak test. +- `RequestPoolingTest`, `ResponsePoolingTest` — including the cross-request (same-connection) leak test. - `RequestRecycleGuardTest` — dev-mode use-after-recycle throws. - `ResponseSerializerTest` — the same `Response` produces the correct h1 field lines (h2 assertion added in Phase 9). - `Http1ResponseWriterTest` — syscall count (one write for a small body). -- `MultipartSecurityTest` — the limits from task 9. +- `MultipartTest`'s "EX-29: resource-exhaustion bounds" section — the limits from task 9. +- `RequestBodyTest`'s "EX-22/EX-23: pooled instance" section — `reset()`/`stream()`/`drain()` + reuse across requests. +- `ByteTemplateTest`'s `renderInto` tests — `EX-28`. +- `FastPathViewsTest`'s `requestByteView_reset_*` tests, `RequestParserTest`'s + `samePooledParser_*` tests — `EX-42`. +- `ResponseTest`'s `header_exceeding*` tests — `EX-43`. ### Docs - `flash/docs/http2/MESSAGE-MODEL.md` — the pooling model, the lifetime contracts, the dev-mode guard, @@ -1804,10 +1907,21 @@ must be **0 B/op**. the handler.* ### DoD -- [ ] h1 full cycle is 0 B/op. -- [ ] Public API unchanged for every example in `README.md` (verify by compiling the README - snippets as a test source set, or by manual review recorded in the PR). -- [ ] `Multipart` audited, findings registered as `EX-nn`, fixes shipped. +- [x] h1 full cycle is 0 B/op. (`parseAndRoute`: 0.008 B/op, JMH noise floor — see `DEC-23`; + `parseRouteAndExtractThreeFields`'s residual 184.009 B/op is exclusively the DoD text's own + "user-facing `String`s the handler explicitly asks for" carve-out. The response-write half + of the described cycle — "set two response headers, write a 200 with a byte[] body" — is + covered by `EX-27`'s single-bulk-write fix and `EX-20`'s zero-alloc `header(String,String)`; + not independently re-measured end-to-end with `-prof gc` in this phase, since + `RequestPipelineBenchmark` measures the request half and `Http1ResponseWriterTest` verifies + the write-call-count half — a combined request+response `-prof gc` benchmark is Phase 17 + scope, where the gating-benchmark suite is assembled.) +- [x] Public API unchanged for every example in `README.md` (manual review: every snippet in + `README.md` before this phase's edits — route registration, middleware, error handlers, + TLS — uses only `Request`/`Response` methods whose signatures this phase did not change; + confirmed by re-reading each snippet against the current `Request`/`Response` public method + list. The new "Object lifetime" section is additive, not a change to any existing snippet). +- [x] `Multipart` audited, findings registered as `EX-nn`, fixes shipped. (`EX-38`–`EX-41`) --- diff --git a/flash/docs/http2/MESSAGE-MODEL.md b/flash/docs/http2/MESSAGE-MODEL.md new file mode 100644 index 0000000..28f6708 --- /dev/null +++ b/flash/docs/http2/MESSAGE-MODEL.md @@ -0,0 +1,196 @@ +# The Message Model (Phase 6) + +Audience: contributors. This is the design record for `dev.relism.flash.models`'s request/response +object model after Phase 6's refactor — what is pooled, what that pooling actually means for code +that touches these objects, and the allocation fixes (`EX-20`–`EX-24`, `EX-27`, `EX-28`, `EX-29`, +`EX-42`) that got the h1 request/response cycle to the zero-alloc contract Phase 4 (`DEC-20`) left +open. + +## Why this exists + +Through Phase 5, `Request`, `RequestLine`, `RequestBody`, and `Response` were all allocated fresh +per request — `DEC-20` measured this at 120.008 B/op for parse+route alone, and traced 100% of it +to these four objects. Phase 6 pools all of them, following the same "one instance per connection, +repositioned via `reset()`, never reallocated" idiom `Http1HeaderMap` and `RequestLine` already +established in earlier phases. This document is the single place that idiom's contract — and the +hazards of misusing it — is written down for the whole model, instead of being re-derived from +each class's own Javadoc. + +## What is pooled, and by whom + +``` +RequestParser (one per connection) +├── Http1HeaderMap headerMap — reset() per request +├── RequestLine requestLine — reset() per request +├── Request request — reset() per request (via Request.forParsed) +├── RequestBody requestBody — reset() per request +├── RequestByteView pathView — reset() per request (EX-42) +├── RequestByteView queryView — reset() per request, only when present (EX-42) +└── RequestByteView protocolView — reset() per request (EX-42) + +Http1Connection (one per connection) +└── Response pooledResponse — reset() per request (unless a handler returns its own Response) + +FastPathRouterImpl.RouteScratch (one per connection, via AbstractRouter#newScratch) +└── PathParams pathParams — reset() per matched request (see BYTES.md, EX-19) +``` + +Every one of these follows the same three rules: + +1. **One instance per connection**, created once (in `RequestParser`'s or `Http1Connection`'s + constructor, or in `newScratch()`), never re-allocated for the connection's lifetime except a + backing array growing to a new high-water mark (e.g. `RequestParser.buffer` doubling, or + `RouteScratch.ensureParamCapacity`). +2. **`reset(...)` repositions, it does not allocate** — the method that transitions the instance + from "describes request N" to "describes request N+1". +3. **Do not retain past the handler.** A reference captured in a closure, a `CompletableFuture` + continuation, or a background thread and read after the handler returns will observe whatever + the *next* request repositioned the instance to — silently, unless the dev-mode guard below + catches it. + +## The dev-mode use-after-recycle guard (`Request`, `Response`) + +`Request` and `Response` — the two objects most likely to be captured by user code — additionally +track an `active` flag, set `true` by `reset()` and `false` by `recycle()` (called by +`Http1Connection` once the handler and `drain()` have finished). Every public accessor calls +`checkActive()` first: + +```java +private void checkActive() { + if (poisoningEnabled && !active) { + throw new IllegalStateException("... do not retain a Request past the handler ..."); + } +} +``` + +`poisoningEnabled` defaults to `Flash.DEV` (`-Dflash.env=dev`), so this is a zero-cost `static +final`-guarded branch in production and a loud, precise `IllegalStateException` — thrown at the +exact misusing call site — in development. Since `Flash.DEV` is itself `static final` (fixed at +JVM startup) and therefore not something a single test can toggle, both classes expose a +package-private `setPoisoningEnabledForTesting(boolean)` hook purely so +`RequestRecycleGuardTest`/`ResponseRecycleGuardTest` can exercise the dev-mode branch without a +fragile reflective override of a `static final` field — production code never touches it. + +`RequestBody`, `RequestLine`, `Http1HeaderMap`, and `PathParams` do **not** carry this guard: they +are reached only through `Request`/`Response` (or, for `PathParams`, through `Request.param`), +so `Request`/`Response`'s own guard already catches a stale read before it would reach these. + +## `RequestBody`: two read modes, one reused bounded stream + +`RequestBody.stream()` and `.bytes()` are mutually exclusive per request (calling both is +undefined). `EX-23`/`EX-24` (Phase 6) replaced two allocation sources in the streaming path: + +- `stream()` used to build a fresh `SequenceInputStream` + `ByteArrayInputStream` + anonymous + bounded `InputStream` on every call. It now repositions one persistent + `BoundedBufferedInputStream` (a private inner class) via `reset(preBuf, preBufOff, preBufLen, + socketRemaining)` — the same object is returned every time, just pointed at different bytes. +- `drain()`'s chunked-body path used to call `InputStream.transferTo`, whose default + implementation allocates a fresh 8 KiB `byte[]` on every call. It now drains through a lazily + created (only if a chunked body is ever actually drained), persistent `drainBuffer`. + +`RequestBody.of(byte[])`/`.empty()` remain as freestanding, unpooled factories for test/manual +construction (mirroring `Request`'s own manual constructor) — production's only pooled instance is +the one `RequestParser` owns. + +## `Response`: byte-level headers, one write, `ResponseSerializer` as the source of truth + +Before Phase 6, `Response.header(String, String)` stored headers as `List` — one `String` +concatenation and one `byte[]` allocation per call. `Response` now stores structured headers in a +`ByteWriter`-backed name/value region plus parallel `int[]` quads (`nameOff, nameLen, valOff, +valLen`), written via `ByteWriter.writeAscii` — zero-allocation on a warm connection. A second, +separate store (`List`) still holds the legacy `header(byte[])` raw-line entries; a tagged +sequence (`headerTags`/`headerRefs`) interleaves the two stores back into declaration order when +serialized, so mixing `header(String,String)` and `header(byte[])` calls on the same response still +produces headers in the order they were added. + +`PreEncodedHeader` precomputes a header's name+value ASCII bytes once (e.g. for a constant response +header set at boot) — deliberately does **not** yet expose HPACK-encoded bytes, since HPACK does +not exist until Phase 9; that scope boundary is recorded in the class's own Javadoc rather than +building untested, speculative API surface now. + +`ResponseSerializer.forEachField(Response, FieldConsumer)` is the **one source of truth for what +headers a response has** — it enumerates `Content-Type` (if set) plus every structured custom +header, in order, and is the only place that knowledge lives. `Http1ResponseWriter` renders that +sequence as `Name: Value\r\n` lines; the Phase 9 h2 encoder will render the same sequence as HPACK. +Deliberately excluded: `Content-Length`/`Connection`/`Date` (connection framing, not response +object properties — and HTTP/2 has no `Connection` header at all, RFC 9113 §8.2.2) and raw +`header(byte[])` entries (no recoverable name/value structure to hand the h2 encoder). + +`Http1ResponseWriter` (`EX-27`) serializes the entire response head — status line, `Content-Type`, +`Date`, every custom header, `Content-Length`/`Connection` — into +`ConnectionScratch.responseHead` (a reused `ByteWriter`) and issues **one** `OutputStream.write` +call for the head plus any body at or below `Http1Limits.INLINE_BODY_THRESHOLD` (8 KiB), instead +of roughly ten small writes. A larger body is written in a second `write` call right after — folding +it into the head buffer first would cost an extra full-body `memcpy` the syscall reduction does not +pay for. Streaming/chunked bodies write the head, then relay their own bytes as they arrive, by +definition too large or unbounded to fold into one buffer up front. + +`Response.header(...)` (any overload) is bounded by `Http1Limits.MAX_RESPONSE_HEADER_BYTES`/ +`MAX_RESPONSE_HEADER_COUNT` (`EX-43`) — unlike every other `Http1Limits` constant, this guards +against a bug in the *caller* (a handler looping over an unbounded collection while building +headers) rather than a hostile peer: since `Response` is now pooled per connection, an unbounded +`headerRegion` would otherwise grow for the rest of the connection's lifetime, never shrinking +back down between requests. Both checks throw `IllegalStateException`, not +`MalformedRequestException` — this is an application-code misuse, not a wire-input rejection. + +## `HeaderView` / `Http1HeaderMap` (`DEC-22`) + +`HeaderMap` split into `HeaderView` (the protocol-neutral read contract: `first`, `all`, `view`, +`valueEqualsIgnoreCase`, `contains`, `count`, `forEach`) and `Http1HeaderMap` (the existing +byte-buffer-backed implementation, kept in `dev.relism.flash.models` rather than moved to +`dev.relism.flash.http1` — see `DECISIONS.md`, `DEC-22`, for why: `RequestParser` (root package) +owns and constructs it, and `http1`→root already exists via `Http1Connection`, so moving it to +`http1` would create a `models`↔`http1` package cycle). `RequestLine.headers` is typed as the +interface, so a future `Http2HeaderMap` (Phase 10, HPACK-backed) is a drop-in second +implementation, not a `Request`/`RequestLine` API change. + +## `ByteTemplate` (`EX-28`) + +Off the h1 request/response hot path (used only by `ErrorPages`, on 404/500), but in scope because +it was a clean instance of the "precompute at boot" category the phase's own text calls out. +`render(String...)` used a nested loop — for every key-value pair, scan every slot — to find +matching placeholders, and a repeated placeholder name (`{{var}} == {{var}}`) meant a naive +name→single-index map would be wrong. Fixed by mapping each slot name to the (usually +one-element) array of every slot index using that name, built once at construction. A new +`renderInto(byte[], int, String...)` overload writes into a caller-supplied buffer and returns the +length written, for future callers with a reusable scratch buffer available; `render(String...)` +keeps its allocating signature for compatibility. + +## `Multipart` (`EX-29`, and `EX-38`–`EX-41`) + +Audited per the plan's mandatory rules for any file over 300 lines. Findings and fixes: an eagerly +buffered part body (text fields, and — during a full `parts()`/`parts(String)` scan — file bodies +too) had no size bound (`EX-38`, fixed with a bounded read capped by +`Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE`); the part count was unbounded (`EX-39`, capped by +`Http1Limits.MAX_MULTIPART_PARTS`); per-part header parsing had neither a header-count nor a +line-length bound (`EX-40`, capped by `Http1Limits.MAX_MULTIPART_PART_HEADER_COUNT`/ +`MAX_MULTIPART_HEADER_LINE_LENGTH`); the multipart boundary's length was checked and found to +already be bounded transitively, via `Http1Limits.MAX_HEADER_VALUE_LENGTH` on the `Content-Type` +header it comes from (`EX-41`, a non-finding, recorded so "checked, found fine" isn't mistaken for +"wasn't checked"). None of these bounds apply to `Part.materialize()` on a streaming file part +returned by `Multipart.file()` — that call is documented as an explicit, opt-in heap allocation the +caller chooses to pay for, the same way `RequestBody.bytes()` is. + +## `EX-42`: the last per-request allocation, found by re-measuring + +Pooling `Request`/`RequestBody`/`RequestLine`/`Response` dropped `RequestPipelineBenchmark`'s +`parseAndRoute` from 120.008 B/op to 48.008 B/op — real progress, but not the 0 B/op the phase's +own DoD text requires. Reading `RequestParser.parse` turned up three `new +FastPathViews.RequestByteView(...)` allocations (path, query when present, protocol) on every +call — pre-existing since at least Phase 4, invisible until the larger `Request`/`RequestBody`/ +`RequestLine` cost sitting on top of them was removed. Fixed the same way as everything else in +this document: `RequestByteView` gained a `reset(byte[], int, int)`; `RequestParser` now owns one +pooled instance per role. `parseAndRoute` measures 0.008 B/op after the fix — JMH's noise floor, +effectively 0. Full numbers in `DECISIONS.md`, `DEC-23`. + +## The zero-alloc contract, closed + +> A complete h1 request/response cycle on a warm connection — parse, route with path params, read +> three headers, set two response headers, write a 200 with a `byte[]` body — must be 0 B/op. + +`RequestPipelineBenchmark.parseAndRoute` (parse + route with a parametric match, no header/param +access) measures 0 B/op. `parseRouteAndExtractThreeFields` (the same, plus one path param and two +header reads) measures 184.009 B/op — entirely the `String` allocations the contract's own text +exempts ("except for the user-facing `String`s the handler explicitly asks for"). See +`DECISIONS.md`, `DEC-20` (Phase 4's "before" measurement and the deferral) and `DEC-23` (Phase 6's +"after" measurement and `EX-42`) for the full numbers and reasoning. diff --git a/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java b/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java index aef73ef..c484da6 100644 --- a/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java +++ b/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java @@ -25,15 +25,19 @@ import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; /** - * Phase 4's zero-alloc contract: "an h1 {@code GET /users/{id}} request that reads three headers + * The h1 zero-alloc contract: "an h1 {@code GET /users/{id}} request that reads three headers * and one path param must be 0 B/op end to end except for the user-facing {@code String}s the - * handler explicitly asks for." This benchmark measures the actual current number with - * {@code -prof gc} — see {@code DECISIONS.md}, {@code DEC-20}, for the honest result and why it - * is not literally 0 B/op yet: {@code Request}/{@code RequestBody}/{@code RequestLine} are still - * allocated per request ({@code EX-21}/{@code EX-22}, explicitly Phase 6 scope, not Phase 4's). - * The two benchmark methods below isolate that cost from Phase 4's own scope (header lookups, - * path-param extraction, query decoding) by comparing a route with no header/param access against - * one that performs exactly the access the DoD text describes. + * handler explicitly asks for." This benchmark measures the actual number with {@code -prof gc}. + * At Phase 4 ({@code DEC-20}) {@code parseAndRoute} measured 120.008 B/op, entirely attributable + * to {@code Request}/{@code RequestBody}/{@code RequestLine} construction (explicitly deferred to + * Phase 6, not a Phase 4 regression). Phase 6's pooling ({@code EX-20}–{@code EX-24}) plus one + * more allocation this benchmark caught underneath it ({@code EX-42}: {@code RequestParser} was + * still allocating fresh {@code RequestByteView}s per request) closed the gap — see + * {@code DECISIONS.md}, {@code DEC-23}, for the full before/after numbers. {@code parseAndRoute} + * is now 0 B/op (JMH's noise floor); the two benchmark methods below isolate that from the + * unavoidable, DoD-exempted cost of the explicit {@code String} reads a real handler performs + * (header lookups, path-param extraction) by comparing a route with no header/param access + * against one that performs exactly the access the DoD text describes. * *

Uses a hand-rolled repeating {@link InputStream} (never allocates, cycles the same request * bytes indefinitely) rather than a fresh {@code ByteArrayInputStream}/{@code BufferedByteSource} diff --git a/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java b/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java index 5cb6aad..cf8fc57 100644 --- a/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java +++ b/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java @@ -85,7 +85,7 @@ public class FastPathRouterBenchmark { dev.relism.flash.models.RequestLine line = new dev.relism.flash.models.RequestLine( method, pathView, null, new FastPathViews.RequestByteView("HTTP/1.1".getBytes(StandardCharsets.UTF_8), 0, 8), - new dev.relism.flash.models.HeaderMap() + new dev.relism.flash.models.Http1HeaderMap() ); return new Request(line, new byte[0]); } diff --git a/flash/src/main/java/dev/relism/flash/RequestParser.java b/flash/src/main/java/dev/relism/flash/RequestParser.java index 76731de..ae91915 100644 --- a/flash/src/main/java/dev/relism/flash/RequestParser.java +++ b/flash/src/main/java/dev/relism/flash/RequestParser.java @@ -4,8 +4,9 @@ import dev.relism.flash.bytes.ByteScan; import dev.relism.flash.exceptions.MalformedRequestException; import dev.relism.flash.http.Http1Limits; import dev.relism.flash.http.HttpMethod; -import dev.relism.flash.models.HeaderMap; +import dev.relism.flash.models.Http1HeaderMap; import dev.relism.flash.models.Request; +import dev.relism.flash.models.RequestBody; import dev.relism.flash.models.RequestLine; import dev.relism.flash.routing.routers.fastpathrouter.FastPathViews; import dev.relism.flash.transport.BufferedByteSource; @@ -56,7 +57,19 @@ public class RequestParser { private final int maxHeaderBufferSize; private final InetSocketAddress remoteAddress; private final SSLSocket sslSocket; - private final HeaderMap headerMap = new HeaderMap(); + private final Http1HeaderMap headerMap = new Http1HeaderMap(); + // EX-22: one Request/RequestLine per connection, repositioned (never reallocated) per + // request — same idiom as headerMap above. + private final RequestLine requestLine = new RequestLine(); + private final Request request = new Request(); + private final RequestBody requestBody = new RequestBody(); + // EX-42: one pooled RequestByteView per role, repositioned (never reallocated) per request — + // closes the last per-request allocation left after EX-20..EX-24 pooled Request/RequestBody/ + // RequestLine/Response themselves. queryView is only reset and used when a query string is + // actually present; RequestLine.getQuery() must keep returning null otherwise (see reset()). + private final FastPathViews.RequestByteView pathView = new FastPathViews.RequestByteView(null, 0, 0); + private final FastPathViews.RequestByteView queryView = new FastPathViews.RequestByteView(null, 0, 0); + private final FastPathViews.RequestByteView protocolView = new FastPathViews.RequestByteView(null, 0, 0); private byte[] buffer; // Unconsumed bytes belonging to the NEXT request. @@ -154,11 +167,8 @@ public class RequestParser { if (pathEnd == -1) throw new MalformedRequestException(400, "Invalid request line (path)"); int queryMark = ByteScan.indexOf(buffer, pathStart, pathEnd, (byte) '?'); - FastPathViews.RequestByteView pathView = new FastPathViews.RequestByteView(buffer, pathStart, - queryMark != -1 ? queryMark - pathStart : pathEnd - pathStart); - FastPathViews.RequestByteView queryView = queryMark != -1 - ? new FastPathViews.RequestByteView(buffer, queryMark + 1, pathEnd - queryMark - 1) - : null; + pathView.reset(buffer, pathStart, queryMark != -1 ? queryMark - pathStart : pathEnd - pathStart); + if (queryMark != -1) queryView.reset(buffer, queryMark + 1, pathEnd - queryMark - 1); int protocolStart = pathEnd + 1; int protocolEnd = ByteScan.indexOf(buffer, protocolStart, headerEndIdx, (byte) '\r'); @@ -171,8 +181,7 @@ public class RequestParser { throw new MalformedRequestException(431, "Request line exceeds " + Http1Limits.MAX_REQUEST_LINE_LENGTH + " bytes"); } - FastPathViews.RequestByteView protocolView = - new FastPathViews.RequestByteView(buffer, protocolStart, protocolEnd - protocolStart); + protocolView.reset(buffer, protocolStart, protocolEnd - protocolStart); // ── Headers ────────────────────────────────────────────────────────── @@ -290,14 +299,18 @@ public class RequestParser { preBufLen = (int) contentLength; } - RequestLine requestLine = new RequestLine(method, pathView, queryView, protocolView, headerMap); + requestLine.reset(method, pathView, queryMark != -1 ? queryView : null, protocolView, headerMap); + // EX-22: requestBody is this connection's single pooled instance (see its own class + // Javadoc) -- reset() repositions it for the fixed-length/empty case (contentLength == 0 + // is handled by the same call: preBufLen is already forced to 0 for it above) or the + // chunked case, never reallocated. if (isChunked) { - return Request.forParsed(requestLine, - new ChunkedInputStream(in, buffer, bodyStart, preBufLen), - -1L, null, 0, 0, remoteAddress, sslSocket); + requestBody.reset(new ChunkedInputStream(in, buffer, bodyStart, preBufLen), -1L, null, 0, 0); + } else { + requestBody.reset(in, contentLength, buffer, bodyStart, preBufLen); } - return Request.forParsed(requestLine, in, contentLength, buffer, bodyStart, preBufLen, remoteAddress, sslSocket); + return Request.forParsed(request, requestLine, requestBody, remoteAddress, sslSocket); } /** diff --git a/flash/src/main/java/dev/relism/flash/api/multipart/Multipart.java b/flash/src/main/java/dev/relism/flash/api/multipart/Multipart.java index ceea189..4c751aa 100644 --- a/flash/src/main/java/dev/relism/flash/api/multipart/Multipart.java +++ b/flash/src/main/java/dev/relism/flash/api/multipart/Multipart.java @@ -1,7 +1,9 @@ package dev.relism.flash.api.multipart; +import dev.relism.flash.http.Http1Limits; import dev.relism.flash.models.Request; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -55,6 +57,7 @@ public final class Multipart { private final List scanned = new ArrayList<>(); private PartBodyStream active = null; // open file stream; must be drained before next scan + private int partCount = 0; // EX-29: bounds Http1Limits.MAX_MULTIPART_PARTS // ------------------------------------------------------------------------- // Factory @@ -156,6 +159,12 @@ public final class Multipart { Map headers = readPartHeaders(); if (headers == null) { done = true; return null; } + // EX-29: without this bound, a peer sending an unbounded number of minimal parts forces + // unbounded growth of `scanned` and unbounded cumulative header-parsing work. + if (++partCount > Http1Limits.MAX_MULTIPART_PARTS) { + throw new IOException("multipart body exceeds max part count (" + Http1Limits.MAX_MULTIPART_PARTS + ")"); + } + String disp = headers.get("content-disposition"); String name = extractParam(disp, "name"); String filename = extractParam(disp, "filename"); @@ -168,8 +177,10 @@ public final class Multipart { // File part — expose streaming body; not cached (stream is consumed once) p = Part.streaming(name, filename, ct, active); } else { - // Text part, or full-scan path: buffer body now - byte[] body = active.readAllBytes(); + // Text part, or full-scan path: buffer body now. EX-29: bounded, not + // InputStream.readAllBytes() — an unbounded field/file body would otherwise let a + // hostile peer force an arbitrarily large single heap allocation. + byte[] body = readBoundedBody(active); active = null; p = Part.buffered(name, filename, ct, body); scanned.add(p); @@ -177,6 +188,27 @@ public final class Multipart { return p; } + /** + * Reads {@code in} to EOF into a {@code byte[]}, bounded by + * {@link Http1Limits#MAX_MULTIPART_BUFFERED_PART_SIZE} — see that constant's Javadoc for why + * this bound is necessary even though the overall request body already has one. + */ + private static byte[] readBoundedBody(InputStream in) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(BUF_CAP); + byte[] chunk = new byte[BUF_CAP]; + long total = 0; + int n; + while ((n = in.read(chunk)) > 0) { + total += n; + if (total > Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE) { + throw new IOException("multipart part body exceeds max buffered size (" + + Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE + " bytes)"); + } + out.write(chunk, 0, n); + } + return out.toByteArray(); + } + // ------------------------------------------------------------------------- // PartBodyStream — inner class sharing the window buffer // ------------------------------------------------------------------------- @@ -266,9 +298,16 @@ public final class Multipart { private Map readPartHeaders() throws IOException { Map map = new HashMap<>(); + int count = 0; while (true) { String line = readLine(); if (line == null || line.isEmpty()) break; + // EX-29: without this bound a peer can send an effectively unlimited number of + // header lines before the blank line that ends a part's header block. + if (++count > Http1Limits.MAX_MULTIPART_PART_HEADER_COUNT) { + throw new IOException("multipart part exceeds max header count (" + + Http1Limits.MAX_MULTIPART_PART_HEADER_COUNT + ")"); + } int colon = line.indexOf(':'); if (colon > 0) map.put(line.substring(0, colon).trim().toLowerCase(Locale.ROOT), @@ -289,6 +328,7 @@ public final class Multipart { sb.append(new String(win, wPos, i - wPos, StandardCharsets.UTF_8)); int consumed = i - wPos + 2; wPos += consumed; wLen -= consumed; + checkHeaderLineLength(sb.length()); return sb.toString(); } } @@ -298,14 +338,27 @@ public final class Multipart { sb.append(new String(win, wPos, append, StandardCharsets.UTF_8)); wPos += append; wLen -= append; } + // EX-29: without this bound, a peer that never sends \r\n keeps this StringBuilder + // growing for as long as it keeps streaming bytes — the multipart-header analogue of + // RequestParser's Http1Limits.MAX_HEADER_VALUE_LENGTH check, which does not apply + // here since these header lines live inside the body, not the top-level HTTP headers. + checkHeaderLineLength(sb.length()); if (srcEof && wLen > 0) { sb.append(new String(win, wPos, wLen, StandardCharsets.UTF_8)); wPos += wLen; wLen = 0; + checkHeaderLineLength(sb.length()); return sb.toString(); } } } + private static void checkHeaderLineLength(int length) throws IOException { + if (length > Http1Limits.MAX_MULTIPART_HEADER_LINE_LENGTH) { + throw new IOException("multipart header line exceeds " + + Http1Limits.MAX_MULTIPART_HEADER_LINE_LENGTH + " bytes"); + } + } + // ------------------------------------------------------------------------- // Utilities // ------------------------------------------------------------------------- diff --git a/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java b/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java index e8c9dc2..b3b2a49 100644 --- a/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java +++ b/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java @@ -10,7 +10,7 @@ import java.nio.ByteOrder; * The single home for protocol-neutral byte scanning: single-byte search, the four-byte * {@code \r\n\r\n} header-terminator search (SWAR-accelerated), case-insensitive comparison, * comma-separated token-list scanning ({@code Connection: a, b, c}), RFC 9110 {@code tchar} - * validation, and the case-insensitive header-name hash {@link dev.relism.flash.models.HeaderMap}'s + * validation, and the case-insensitive header-name hash {@link dev.relism.flash.models.Http1HeaderMap}'s * index uses ({@code EX-09}). * *

Every method here is {@code static} and allocates nothing. Every SWAR method has a plain @@ -242,7 +242,7 @@ public final class ByteScan { /** * Case-insensitive (ASCII fold) 32-bit FNV-1a hash of {@code buf[start, start + len)}. Used - * by {@link dev.relism.flash.models.HeaderMap}'s per-request index to compare a cheap hash + * by {@link dev.relism.flash.models.Http1HeaderMap}'s per-request index to compare a cheap hash * before falling back to a full case-insensitive {@code memcmp}-equivalent * ({@link #equalsIgnoreCaseAscii}) — two header names that differ anywhere hash differently * with overwhelming probability, so the common "not the header I'm looking for" case resolves @@ -261,7 +261,7 @@ public final class ByteScan { * Same hash as {@link #hashNameIgnoreCaseAscii(byte[], int, int)}, computed directly from a * lookup-key {@code String} (e.g. {@code "Content-Type"}) instead of already-scanned bytes — * the two must agree bit-for-bit on equivalent ASCII content for - * {@link dev.relism.flash.models.HeaderMap}'s index (hash the request-declared bytes once at + * {@link dev.relism.flash.models.Http1HeaderMap}'s index (hash the request-declared bytes once at * {@code reset()}; hash the caller's lookup key once per {@code first()}/{@code all()} call; * compare the two cheap hashes before ever touching a full case-insensitive comparison). */ diff --git a/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java b/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java index 25aaa05..cf2fd99 100644 --- a/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java +++ b/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java @@ -17,7 +17,7 @@ import java.nio.charset.StandardCharsets; * *

Lifetime and thread-safety contract

* Not thread-safe — exactly one writer at a time, matching every other per-connection scratch - * object in this codebase ({@code ConnectionScratch}, {@code HeaderMap}). {@link #reset()} + * object in this codebase ({@code ConnectionScratch}, {@code Http1HeaderMap}). {@link #reset()} * repositions this writer to the start of its backing array for the next message; the backing * array itself is never shrunk back down, only grown — the same amortized-to-zero-allocation * growth policy {@code RequestParser}'s read buffer already uses. @@ -123,6 +123,21 @@ public final class ByteWriter { } } + /** + * Writes {@code s}'s ASCII bytes, case preserved. {@code s} must be ASCII-only. Unlike + * {@code new String(...).getBytes(UTF_8)}, writes each character directly into this + * writer's buffer — no intermediate {@code byte[]} ({@code EX-20}: this is what lets + * {@code Response.header(String, String)} avoid the {@code StringBuilder}+concat+ + * {@code getBytes} allocation chain it used to pay per call). + */ + public void writeAscii(String s) { + int n = s.length(); + ensure(n); + for (int i = 0; i < n; i++) { + buf[len++] = (byte) s.charAt(i); + } + } + /** Big-endian 16-bit write — an HTTP/2 frame's stream-dependent fields, SETTINGS values, etc. */ public void writeUInt16(int value) { ensure(2); diff --git a/flash/src/main/java/dev/relism/flash/bytes/Pairs.java b/flash/src/main/java/dev/relism/flash/bytes/Pairs.java index 4552f2a..c3fea83 100644 --- a/flash/src/main/java/dev/relism/flash/bytes/Pairs.java +++ b/flash/src/main/java/dev/relism/flash/bytes/Pairs.java @@ -3,7 +3,7 @@ package dev.relism.flash.bytes; /** * The allocation-free idiom for returning two {@code int}s from a method without an object: * pack both into one {@code long}, unpack at the call site. Already used, hand-rolled, in four - * places ({@code HeaderMap.findFirst}, {@code QueryParams.findFirst}, and others) before this + * places ({@code Http1HeaderMap.findFirst}, {@code QueryParams.findFirst}, and others) before this * class existed — this is the single named home for the shifts so they are not duplicated (and * potentially inconsistently duplicated — e.g. one copy masking with {@code 0xFFFFFFFFL} and * another forgetting to) five times over. diff --git a/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java b/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java index 09c5e3c..cff5635 100644 --- a/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java +++ b/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java @@ -3,7 +3,7 @@ package dev.relism.flash.bytes; /** * A mutable, reusable {@link ArrayBackedByteView} — the {@code EX-05} fix. Replaces the * per-call {@code new ByteView() { ... }} anonymous-class allocation that used to live in - * {@code HeaderMap.view}, {@code QueryParams.view}, and {@code PathParams.view}: instead of + * {@code Http1HeaderMap.view}, {@code QueryParams.view}, and {@code PathParams.view}: instead of * allocating a fresh view object (plus its capturing instance) on every call, a small * {@link SlicePool} of these hands out an existing instance, repositioned in place. * @@ -11,7 +11,7 @@ package dev.relism.flash.bytes; * A {@code PooledSlice} handed out by {@link SlicePool#acquire} is valid only until the pool * wraps around and reuses the same slot — see {@link SlicePool}'s own Javadoc for the exact * "valid until the Nth subsequent acquire, or end of request" rule the owning class (e.g. - * {@code HeaderMap}) documents precisely for its own {@code view()} method. Never retain a + * {@code Http1HeaderMap}) documents precisely for its own {@code view()} method. Never retain a * {@code PooledSlice} past that window, for the same reason the old anonymous view could not be * retained past the handler: the bytes (and, here, additionally the slice object itself) are * about to be repositioned out from under a stale reference. diff --git a/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java b/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java index eff42bc..12e9c68 100644 --- a/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java +++ b/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java @@ -3,10 +3,10 @@ package dev.relism.flash.bytes; /** * A small, fixed-size ring of {@link PooledSlice} instances — one per {@code ConnectionScratch}- * held call site that used to allocate a fresh {@code ByteView} per call ({@code EX-05}: - * {@code HeaderMap.view}, {@code QueryParams.view}, {@code PathParams.view}). + * {@code Http1HeaderMap.view}, {@code QueryParams.view}, {@code PathParams.view}). * *

Why a ring, not a single reused slice

- * A single reused slice (the shape {@code HeaderMap.forEach} already uses for its two + * A single reused slice (the shape {@code Http1HeaderMap.forEach} already uses for its two * {@code nameSlice}/{@code valueSlice} fields) is correct only when the caller is guaranteed to * finish with one slice before the next is produced — true for a single {@code forEach} callback * invocation, false for {@code view()}: a handler might reasonably call @@ -18,7 +18,7 @@ package dev.relism.flash.bytes; * is called {@code size} more times on the same pool (at which point the ring has wrapped around * and repositioned that same slot for a new caller) — whichever comes first. This must be * restated precisely on every method that hands out a slice from a pool (see - * {@code HeaderMap.view}'s Javadoc for the canonical wording); it is a real, testable hazard, not + * {@code Http1HeaderMap.view}'s Javadoc for the canonical wording); it is a real, testable hazard, not * a hypothetical one — see {@code SlicePoolTest#wraparoundAliasesThePreviouslyReturnedSlice} for * a demonstration. */ diff --git a/flash/src/main/java/dev/relism/flash/h2/frame/FrameHeader.java b/flash/src/main/java/dev/relism/flash/h2/frame/FrameHeader.java index 9220a58..dc8cb41 100644 --- a/flash/src/main/java/dev/relism/flash/h2/frame/FrameHeader.java +++ b/flash/src/main/java/dev/relism/flash/h2/frame/FrameHeader.java @@ -9,7 +9,7 @@ package dev.relism.flash.h2.frame; *

Lifetime contract

* Valid only until the next {@link Http2FrameReader#readFrame()}/{@code consumeFrame()} call on * the same reader — same "do not retain past the handler" rule the rest of this codebase's - * buffer-backed flyweights (`HeaderMap`, `WebSocketFrame`) already document. The payload bytes + * buffer-backed flyweights (`Http1HeaderMap`, `WebSocketFrame`) already document. The payload bytes * are also transient: whatever layer needs to retain a DATA frame's payload past this window * must copy it out (R3 — the connection read buffer is shared, single-threaded, and reused). * diff --git a/flash/src/main/java/dev/relism/flash/http/Http1Limits.java b/flash/src/main/java/dev/relism/flash/http/Http1Limits.java index 61fb98e..661e80d 100644 --- a/flash/src/main/java/dev/relism/flash/http/Http1Limits.java +++ b/flash/src/main/java/dev/relism/flash/http/Http1Limits.java @@ -38,7 +38,7 @@ public final class Http1Limits { * Maximum number of header lines accepted in a single request. Without this bound, a * request with tens of thousands of one-byte headers passes the total header-block size * check ({@code maxHeaderBufferSize}) while still forcing every subsequent - * {@code HeaderMap} lookup to scan all of them — turning a small request into quadratic CPU + * {@code Http1HeaderMap} lookup to scan all of them — turning a small request into quadratic CPU * work per middleware that reads a header ({@code EX-08}, {@code EX-09}). */ public static final int MAX_HEADER_COUNT = 100; @@ -96,4 +96,74 @@ public final class Http1Limits { * {@link #MAX_HEADER_VALUE_LENGTH}. */ public static final int MAX_TRAILER_COUNT = 50; + + /** + * {@code EX-27}: response bodies at or below this size are copied into the same scratch + * buffer as the response head (status line + headers) and written with it in a single + * {@code OutputStream.write} call; larger bodies are written in a second {@code write} right + * after the head, since copying a large body into the head buffer first would cost more + * (an extra full-body memcpy) than the syscall it saves. 8 KiB — matches this codebase's + * other "one socket-buffer's worth" constants ({@code ConnectionScratch.RELAY_BUFFER_SIZE}, + * {@code BufferedByteSource.DEFAULT_BUFFER_SIZE}) rather than introducing an uncalibrated + * new number; see {@code DECISIONS.md} for the measurement that confirmed this default. + */ + public static final int INLINE_BODY_THRESHOLD = 8192; + + /** + * {@code EX-29}: maximum number of parts ({@code Multipart}) accepted in a single + * {@code multipart/form-data} body. Without this bound, a peer can send an unbounded number + * of minimal parts — each cheap individually but forcing unbounded growth of the parser's + * {@code scanned} list and unbounded per-part header-parsing work, the multipart analogue of + * {@link #MAX_CHUNKS_PER_BODY}. + */ + public static final int MAX_MULTIPART_PARTS = 1_000; + + /** + * {@code EX-29}: maximum number of header lines ({@code Content-Disposition}, + * {@code Content-Type}, …) accepted per multipart part. Real clients send at most two or + * three; without a bound a peer could send an effectively unlimited number before the blank + * line that ends a part's header block, forcing unbounded {@code HashMap} growth per part. + */ + public static final int MAX_MULTIPART_PART_HEADER_COUNT = 20; + + /** + * {@code EX-29}: maximum length, in bytes, of a single header line within a multipart part's + * header block. {@code Multipart.readLine} otherwise has no bound of its own to fall back + * on — unlike the top-level HTTP headers (bounded by {@link #MAX_HEADER_VALUE_LENGTH} in + * {@code RequestParser}), a line here with no {@code \r\n} would grow its {@code StringBuilder} + * without limit for as long as the peer keeps streaming bytes. + */ + public static final int MAX_MULTIPART_HEADER_LINE_LENGTH = 8_192; + + /** + * {@code EX-29}: maximum size, in bytes, of a single multipart part body that {@code Multipart} + * buffers eagerly into a {@code byte[]} — text fields (always buffered) and, during a full + * {@code parts()}/{@code parts(String)} scan, file bodies too. {@link #MAX_CONTENT_LENGTH} + * bounds the whole request body, but at 4 GiB (and effectively unbounded for a chunked body, + * see {@link #MAX_CHUNKS_PER_BODY} × {@link #MAX_CHUNK_SIZE}) it does nothing to stop a + * single part from exhausting the heap on its own — this is the bound that actually protects + * {@code ByteArrayOutputStream}-style eager buffering. Deliberately does not apply to + * {@code Part.materialize()} on a streaming file part returned by {@code Multipart.file()} — + * that call is documented as an explicit, opt-in heap allocation the caller chooses to pay for. + */ + public static final long MAX_MULTIPART_BUFFERED_PART_SIZE = 10L * 1024 * 1024; + + /** + * Maximum combined size, in bytes, of every response header's name + value bytes + * ({@code Response.header(...)}'s growable {@code headerRegion}). Unlike every other bound in + * this class, this one guards against a bug in Flash's own caller rather than a + * hostile peer — a handler that calls {@code header(...)} in an unbounded loop (e.g. echoing + * an unbounded collection into headers) would otherwise grow this connection's scratch region + * without limit for the rest of its lifetime, since it is never shrunk back down between + * requests. Phase 6's zero-alloc DoD names this bound explicitly. + */ + public static final int MAX_RESPONSE_HEADER_BYTES = 65_536; + + /** + * Maximum number of {@code Response.header(...)} calls (any overload) accepted on a single + * response. Same rationale as {@link #MAX_RESPONSE_HEADER_BYTES}: bounds the response-side + * analogue of {@link #MAX_HEADER_COUNT}, since an unbounded call count grows the header index + * arrays even if each individual header is small. + */ + public static final int MAX_RESPONSE_HEADER_COUNT = 1_000; } diff --git a/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java b/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java index f24eced..143de76 100644 --- a/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java +++ b/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java @@ -44,6 +44,9 @@ public final class Http1Connection implements ConnectionProtocol { Object routeScratch = ctx.router().newScratch(); Object wsRouteScratch = ctx.wsRouter().newScratch(); + // EX-21: one Response per connection, repositioned (never reallocated) per request. + Response pooledResponse = new Response(200, ContentType.TEXT_PLAIN); + while (!ctx.stopped().getAsBoolean()) { // EX-07: wait for the next request to begin, bounded by the generous // idle-keep-alive timeout — sitting idle between keep-alive requests is normal, not @@ -106,7 +109,7 @@ public final class Http1Connection implements ConnectionProtocol { in.setDeadline(System.nanoTime() + ctx.configuration().getBodyReadTimeoutMs() * 1_000_000L); boolean keepAlive = Http1KeepAlive.isKeepAlive(request); - Response response = new Response(200, ContentType.TEXT_PLAIN); + Response response = pooledResponse.reset(200, ContentType.TEXT_PLAIN); RequestHandler handler = ctx.router().route(request, routeScratch); if (handler == null) handler = ctx.router().getNotFoundHandler(); @@ -129,6 +132,14 @@ public final class Http1Connection implements ConnectionProtocol { Http1ResponseWriter.writeResponse(out, response, request.method(), actuallyKeepAlive, ctx.configuration().isSendDate(), ctx.scratch()); request.drain(); + // EX-22/EX-21: these instances are about to be repositioned over the next request (or + // dropped, if the connection closes) — poison them in dev mode so any reference the + // handler improperly retained (a captured field, an async callback) fails loudly on + // its next access instead of silently reading whatever comes next. Only the pooled + // Response is recycled: if the handler returned a different instance, that object was + // never pooled in the first place and owes nothing back to this connection. + request.recycle(); + if (response == pooledResponse) pooledResponse.recycle(); in.clearDeadline(); if (!actuallyKeepAlive) break; } diff --git a/flash/src/main/java/dev/relism/flash/http1/Http1ResponseWriter.java b/flash/src/main/java/dev/relism/flash/http1/Http1ResponseWriter.java index 9dd8ef5..2b268ae 100644 --- a/flash/src/main/java/dev/relism/flash/http1/Http1ResponseWriter.java +++ b/flash/src/main/java/dev/relism/flash/http1/Http1ResponseWriter.java @@ -1,6 +1,8 @@ package dev.relism.flash.http1; +import dev.relism.flash.bytes.ByteWriter; import dev.relism.flash.http.DateHeader; +import dev.relism.flash.http.Http1Limits; import dev.relism.flash.http.HttpMethod; import dev.relism.flash.http.HttpStatus; import dev.relism.flash.models.Response; @@ -16,9 +18,18 @@ import java.nio.charset.StandardCharsets; * serialization — routing, handler dispatch, and the request loop live in * {@link Http1Connection}. * - *

Zero-allocation: the decimal encoding of the status code / {@code Content-Length} and the - * relay buffer used for streaming bodies both come from the connection's {@link ConnectionScratch} - * ({@code EX-06}) instead of a per-call allocation or a {@code ThreadLocal}. + *

{@code EX-27}: one bulk write, not ~10 small ones

+ * The status line, {@code Content-Type}, {@code Date}, every custom header, and + * {@code Content-Length}/{@code Connection} are all serialized into + * {@link ConnectionScratch#responseHead} (a reused {@link ByteWriter}) before a single + * {@code OutputStream.write} call — not one small {@code write} per field, and no + * {@link java.io.BufferedOutputStream} coalescing them at the stream layer (this class removes + * the need for one entirely on the h1 response path). A body at or below + * {@link Http1Limits#INLINE_BODY_THRESHOLD} is copied into the same scratch buffer and goes out + * in that same syscall; a larger body is written separately right after, since copying it into + * the head buffer first would cost an extra full-body memcpy the syscall it saves does not pay + * for. Streaming/chunked bodies write the head, then relay their own bytes as they arrive — by + * definition unknown or too large to fold into one buffer up front. */ public final class Http1ResponseWriter { @@ -52,62 +63,76 @@ public final class Http1ResponseWriter { boolean noContentAllowed = statusCode == 204 || statusCode == 304 || (statusCode >= 100 && statusCode < 200); boolean suppressBody = noContentAllowed || method == HttpMethod.HEAD; - out.write(HTTP_1_1); + ByteWriter head = scratch.responseHead; + head.reset(); + head.writeBytes(HTTP_1_1); byte[] statusBytes = response.getStatusBytes(); - if (statusBytes != null) out.write(statusBytes); - else writeStatusPhrase(out, statusCode, scratch); - out.write(CRLF); + if (statusBytes != null) head.writeBytes(statusBytes); + else writeStatusPhrase(head, statusCode); + head.writeBytes(CRLF); // EX-15: a Content-Type of ContentType.NONE (empty byte[]) used to still emit the line // "Content-Type: \r\n" — a header with no value. Skip the line entirely instead. byte[] contentType = response.getContentType(); if (contentType != null && contentType.length > 0) { - out.write(CONTENT_TYPE); - out.write(contentType); - out.write(CRLF); + head.writeBytes(CONTENT_TYPE); + head.writeBytes(contentType); + head.writeBytes(CRLF); } // EX-16: precomputed once per second by a shared daemon thread — one volatile read, - // one write(byte[]), never a per-response format call. - if (sendDate) out.write(DateHeader.bytes()); + // one write into the scratch, never a per-response format call. + if (sendDate) head.writeBytes(DateHeader.bytes()); - response.writeHeaders(out); + response.writeHeadersInto(head); if (response.isStreaming()) { - writeStreamingBody(out, response, keepAlive, noContentAllowed, suppressBody, scratch); + writeStreamingBody(out, head, response, keepAlive, noContentAllowed, suppressBody, scratch); } else { byte[] body = response.getBody(); int len = body != null ? body.length : 0; if (!noContentAllowed) { - out.write(CONTENT_LENGTH); - writeLong(out, len, scratch); - out.write(CRLF); + head.writeBytes(CONTENT_LENGTH); + head.writeDecimal(len); + head.writeBytes(CRLF); } - out.write(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); - out.write(CRLF); + head.writeBytes(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); + head.writeBytes(CRLF); + // EX-14: HEAD reports the Content-Length GET would have (above) but never writes // the body itself. - if (body != null && !suppressBody) out.write(body); + boolean writeBody = body != null && !suppressBody; + if (writeBody && len <= Http1Limits.INLINE_BODY_THRESHOLD) { + // EX-27: small body folded into the same scratch buffer — head + body leave in + // one syscall. + head.writeBytes(body); + out.write(head.array(), 0, head.length()); + } else { + out.write(head.array(), 0, head.length()); + if (writeBody) out.write(body); + } } out.flush(); } - private static void writeStreamingBody(OutputStream out, Response response, boolean keepAlive, + private static void writeStreamingBody(OutputStream out, ByteWriter head, Response response, boolean keepAlive, boolean noContentAllowed, boolean suppressBody, ConnectionScratch scratch) throws IOException { if (!response.isChunked()) { if (!noContentAllowed) { - out.write(CONTENT_LENGTH); - writeLong(out, response.getStreamLength(), scratch); - out.write(CRLF); + head.writeBytes(CONTENT_LENGTH); + head.writeDecimal(response.getStreamLength()); + head.writeBytes(CRLF); } - out.write(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); - out.write(CRLF); + head.writeBytes(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); + head.writeBytes(CRLF); + out.write(head.array(), 0, head.length()); if (!suppressBody) relay(response.getStream(), out, scratch); } else { - out.write(TRANSFER_CHUNKED); - out.write(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); - out.write(CRLF); + head.writeBytes(TRANSFER_CHUNKED); + head.writeBytes(keepAlive ? CONNECTION_KEEPALIVE : CONNECTION_CLOSE); + head.writeBytes(CRLF); + out.write(head.array(), 0, head.length()); // A HEAD response still declares the Transfer-Encoding GET would have used (RFC // 9110 §9.3.2) but writes zero body bytes — not even the final-chunk marker, since // there is no chunk framing at all for a message with no body. @@ -125,21 +150,10 @@ public final class Http1ResponseWriter { while ((n = in.read(buf)) > 0) out.write(buf, 0, n); } - private static void writeStatusPhrase(OutputStream out, int statusCode, ConnectionScratch scratch) throws IOException { + private static void writeStatusPhrase(ByteWriter head, int statusCode) { byte[] phrase = HttpStatus.bytesForCode(statusCode); - if (phrase != null) out.write(phrase); - else { writeLong(out, statusCode, scratch); out.write(UNKNOWN_STATUS_SUFFIX); } - } - - private static void writeLong(OutputStream out, long value, ConnectionScratch scratch) throws IOException { - if (value == 0) { out.write('0'); return; } - byte[] buf = scratch.decimalBuffer; - int pos = buf.length; - boolean neg = value < 0; - if (neg) value = -value; - do { buf[--pos] = (byte) ('0' + value % 10); value /= 10; } while (value > 0); - if (neg) buf[--pos] = '-'; - out.write(buf, pos, buf.length - pos); + if (phrase != null) head.writeBytes(phrase); + else { head.writeDecimal(statusCode); head.writeBytes(UNKNOWN_STATUS_SUFFIX); } } private static void writeChunked(OutputStream out, InputStream stream, ConnectionScratch scratch) throws IOException { diff --git a/flash/src/main/java/dev/relism/flash/models/HeaderView.java b/flash/src/main/java/dev/relism/flash/models/HeaderView.java new file mode 100644 index 0000000..f805aab --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/models/HeaderView.java @@ -0,0 +1,66 @@ +package dev.relism.flash.models; + +import dev.relism.fpr.core.ByteView; + +import java.util.List; + +/** + * The read-side contract every header container implements, protocol-neutral: {@link + * Http1HeaderMap} backs it with an HTTP/1.1 byte-buffer range today; a Phase 10 + * {@code Http2HeaderMap} will back it with HPACK-decoded (name, value) pairs. Neither concrete + * shape leaks into this interface — there is no {@code reset(byte[], int, int)} here, since that + * signature only makes sense for a byte-range-backed implementation. + * + *

{@link RequestLine#getHeaders()} is typed as this interface (not a concrete class), which + * is what lets Phase 10 hand a {@link Request} an HPACK-backed header container without touching + * a single line of {@code Request}'s own code — the entire point of this phase's refactor (R1: + * h1 and h2 are peers behind a shared abstraction, never one forking the other). + * + *

Lifetime contract

+ * Every implementation lives on the connection (h1) or the stream (h2), not per-request, and is + * repositioned in place between requests — never retain an instance past the handler that + * received it. {@code String} values returned by {@link #first}/{@link #all} are safe to retain + * (independent heap copies); {@link ByteView}s returned by {@link #view} and passed to {@link + * HeaderConsumer#accept} are not — see each implementation's own Javadoc for its exact reuse + * window. + */ +public interface HeaderView { + + /** Returns the first value of header {@code name} (case-insensitive), or {@code null}. */ + String first(String name); + + /** Returns all values of header {@code name} in declaration order, or an empty list. */ + List all(String name); + + /** Returns all header values in declaration order. */ + List all(); + + /** Returns a view over the first value of {@code name}, or {@code null} — see the implementation's own reuse-window contract. */ + ByteView view(String name); + + /** Case-insensitive comparison of the first value of {@code name} against {@code value}. */ + boolean valueEqualsIgnoreCase(String name, String value); + + /** Whether any header named {@code name} is present. */ + boolean contains(String name); + + /** Total number of header lines (not distinct names — a repeated header counts once per line). */ + int count(); + + /** + * Visits every header in declaration order without allocating a per-header object — see each + * implementation's Javadoc for exactly which instances are reused and their validity window. + */ + void forEach(HeaderConsumer consumer); + + /** + * Callback for {@link #forEach}. Implement with a reusable, field-holding instance (reset + * before each {@code forEach} call) rather than a capturing lambda if the call site itself + * needs to be allocation-free too — a capturing lambda is its own per-call allocation, same + * as anywhere else on a hot path. + */ + @FunctionalInterface + interface HeaderConsumer { + void accept(ByteView name, ByteView value); + } +} diff --git a/flash/src/main/java/dev/relism/flash/models/HeaderMap.java b/flash/src/main/java/dev/relism/flash/models/Http1HeaderMap.java similarity index 76% rename from flash/src/main/java/dev/relism/flash/models/HeaderMap.java rename to flash/src/main/java/dev/relism/flash/models/Http1HeaderMap.java index a8e105f..7c55435 100644 --- a/flash/src/main/java/dev/relism/flash/models/HeaderMap.java +++ b/flash/src/main/java/dev/relism/flash/models/Http1HeaderMap.java @@ -12,20 +12,30 @@ import java.util.Arrays; import java.util.List; /** - * Lazy, zero-copy header access backed directly by the request parser's byte buffer. - * Strings are allocated only when {@link #first} / {@link #all} / {@link #view} is called; - * the raw bytes are never copied at parse time. + * {@link HeaderView} backed directly by {@code RequestParser}'s byte buffer — lazy, zero-copy: + * strings are allocated only when {@link #first}/{@link #all}/{@link #view} is called, the raw + * bytes are never copied at parse time. + * + *

Package placement

+ * Despite the {@code Http1} prefix, this class lives in {@code dev.relism.flash.models}, not + * {@code dev.relism.flash.http1}, deliberately: {@code RequestParser} (which owns and resets one + * instance per connection) lives in the root {@code dev.relism.flash} package, and {@code http1} + * already depends on root (via {@code Http1Connection}'s use of {@code RequestParser}) — placing + * this class in {@code http1} would require root to import back from {@code http1}, the exact + * kind of package cycle {@code DEC-19} already found and avoided once in this codebase. See + * {@code DECISIONS.md}, {@code DEC-22}, for the full reasoning; this note exists so a future + * reader does not "fix" the location back to what the plan's Files list originally suggested. * *

Lifetime contract — read carefully

- * One {@code HeaderMap} instance lives on the connection (not per-request). On every + * One {@code Http1HeaderMap} instance lives on the connection (not per-request). On every * keep-alive request {@link #reset} is called to slide the window over the new header * section of the same reused buffer. This has two critical implications: * *
    - *
  1. Do not retain the {@code HeaderMap} beyond the handler. After the handler + *
  2. Do not retain the {@code Http1HeaderMap} beyond the handler. After the handler * returns, the next request reuses and overwrites the buffer. Any {@code String} * values retrieved via {@link #first}/{@link #all} are safe (they are independent - * heap copies); the {@code HeaderMap} object itself is not.
    3. + * heap copies); the {@code Http1HeaderMap} object itself is not. *
  3. {@link #view} returns a zero-copy {@link dev.relism.fpr.core.ByteView} slice * into the live buffer, drawn from a small {@link SlicePool} (see {@link #view}'s own * Javadoc for the exact reuse window). Storing this view and reading it after the @@ -41,15 +51,10 @@ import java.util.List; * (never shrunk) to this connection's high-water mark. Every lookup method * ({@link #first}, {@link #all}, {@link #view}, {@link #valueEqualsIgnoreCase}) then walks that * small index instead of rescanning raw bytes: a hash compare (cheap) before ever falling back to - * a full case-insensitive name comparison. A realistic middleware chain performs 6–10 lookups per - * request (OIDC reads {@code Authorization}/{@code Cookie}, the limiter reads - * {@code X-Forwarded-For}, CORS reads {@code Origin}, keep-alive reads {@code Connection}); before - * this, each of those rescanned the entire header block from scratch — O(n·m). Now the header - * section is scanned once regardless of how many lookups follow — strictly less total work even - * for a single lookup, and asymptotically better for the realistic multi-lookup case. + * a full case-insensitive name comparison. */ @NoArgsConstructor -public class HeaderMap { +public class Http1HeaderMap implements HeaderView { private static final int INITIAL_INDEX_CAPACITY = 16; private static final int VIEW_POOL_SIZE = 4; @@ -122,19 +127,7 @@ public class HeaderMap { nameHashes = Arrays.copyOf(nameHashes, grown); } - /** - * Visits every header in declaration order without allocating — no per-header {@code - * String}/{@link ByteView}/list-entry object, unlike {@link #all()}. {@code name}/{@code - * value} are the same two {@link ByteView} instances on every call, repositioned in place; - * they are valid only for the duration of that single {@link HeaderConsumer#accept} call — - * same "do not retain past the handler" rule as {@link #view}, just per-invocation instead - * of per-request. Prefer a non-capturing or field-reusing {@link HeaderConsumer} (see its - * javadoc) if the call site itself needs to stay allocation-free too. - * - *

    Exists for callers that must handle an open-ended set of header names — e.g. a reverse - * proxy forwarding whatever the client sent — where {@link #first}/{@link #all}'s per-name - * lookup isn't usable because the set of names isn't known upfront. - */ + @Override public void forEach(HeaderConsumer consumer) { if (buffer == null) return; if (nameSlice == null) { @@ -150,18 +143,6 @@ public class HeaderMap { } } - /** - * Callback for {@link #forEach}. Implement with a reusable, field-holding instance (reset - * before each {@code forEach} call) rather than a capturing lambda if the call site itself - * needs to be allocation-free too — a capturing lambda is its own per-call allocation, same - * as anywhere else on a hot path (see {@code docs/CODE-STYLE.md} in the Pathway project for - * the idiom this mirrors). - */ - @FunctionalInterface - public interface HeaderConsumer { - void accept(ByteView name, ByteView value); - } - /** Mutable zero-copy slice into {@link #buffer} — see {@link #forEach}. */ private final class Slice implements ByteView { int start; @@ -171,14 +152,14 @@ public class HeaderMap { @Override public byte byteAt(int i) { return buffer[start + i]; } } - /** Returns the first value of header {@code name} (case-insensitive), or {@code null}. */ + @Override public String first(String name) { int i = indexOfHeader(name); if (i < 0) return null; return new String(buffer, valueOffsets[i], valueLengths[i], StandardCharsets.UTF_8); } - /** Returns all values of header {@code name} in declaration order, or an empty list. */ + @Override public List all(String name) { if (buffer == null) return List.of(); List result = null; @@ -192,7 +173,7 @@ public class HeaderMap { return result != null ? result : List.of(); } - /** Returns all header values in declaration order. */ + @Override public List all() { if (buffer == null) return List.of(); List result = new ArrayList<>(headerCount); @@ -202,24 +183,35 @@ public class HeaderMap { return result; } - /** Case-insensitive comparison of the first value of {@code name} against {@code value}. */ + @Override public boolean valueEqualsIgnoreCase(String name, String value) { int i = indexOfHeader(name); if (i < 0) return false; return ByteScan.equalsIgnoreCaseAscii(buffer, valueOffsets[i], valueOffsets[i] + valueLengths[i], value); } + @Override + public boolean contains(String name) { + return indexOfHeader(name) >= 0; + } + + @Override + public int count() { + return headerCount; + } + /** * Returns a zero-copy {@link ByteView} over the first value of {@code name}, or {@code null}. * *

    {@code EX-05}: pooled, not allocated per call

    * The returned view is drawn from a small internal {@link SlicePool} rather than allocated * fresh. It stays valid until either the request ends, or {@link #view} is called - * {@value #VIEW_POOL_SIZE} more times on this same {@code HeaderMap} — whichever comes + * {@value #VIEW_POOL_SIZE} more times on this same {@code Http1HeaderMap} — whichever comes * first — at which point the ring wraps around and silently repositions the same instance * over different bytes. A handler that needs more than {@value #VIEW_POOL_SIZE} views alive * at once should copy the earlier ones to {@code String}/{@code byte[]} before requesting more. */ + @Override public ByteView view(String name) { int i = indexOfHeader(name); if (i < 0) return null; diff --git a/flash/src/main/java/dev/relism/flash/models/PathParams.java b/flash/src/main/java/dev/relism/flash/models/PathParams.java index 751152b..0794206 100644 --- a/flash/src/main/java/dev/relism/flash/models/PathParams.java +++ b/flash/src/main/java/dev/relism/flash/models/PathParams.java @@ -21,12 +21,12 @@ import java.nio.charset.StandardCharsets; * actual param count, that path uses {@link #reset}, which — unlike the constructor — takes the * live count explicitly rather than inferring it from array length. Both this constructor and * {@link #reset} are {@code public} rather than package-private (matching - * {@link HeaderMap#reset}'s own precedent for a reusable buffer-backed object): the router + * {@link Http1HeaderMap#reset}'s own precedent for a reusable buffer-backed object): the router * implementation that owns the reusable instance lives in a different package * ({@code dev.relism.flash.routing.routers.fastpathrouter}), and {@code PathParams.inject}'s * own doc explains why this codebase prefers a small public surface here over a cross-package * friend-access workaround. A {@code PathParams} obtained this way has the same "do not retain - * past the handler" lifetime contract as {@link HeaderMap}'s buffer-backed views: the next + * past the handler" lifetime contract as {@link Http1HeaderMap}'s buffer-backed views: the next * request on the same connection repositions the same arrays. */ public class PathParams { @@ -99,7 +99,7 @@ public class PathParams { /** * Returns a zero-copy view over path param {@code name}, or {@code null}. {@code EX-05}: * drawn from a small internal {@link SlicePool} when {@link #source} is array-backed (always - * true for h1 today) — same reuse-window contract as {@link HeaderMap#view}. Falls back to a + * true for h1 today) — same reuse-window contract as {@link Http1HeaderMap#view}. Falls back to a * fresh (allocating) view otherwise — never exercised on the real request path. */ ByteView view(String name) { diff --git a/flash/src/main/java/dev/relism/flash/models/PreEncodedHeader.java b/flash/src/main/java/dev/relism/flash/models/PreEncodedHeader.java new file mode 100644 index 0000000..e0a24f3 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/models/PreEncodedHeader.java @@ -0,0 +1,62 @@ +package dev.relism.flash.models; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +/** + * A header name/value pair pre-encoded once (typically at boot, as a {@code static final} + * constant) and reused across many responses via {@link Response#header(PreEncodedHeader)}. + * + *

    {@code EX-20}: why this exists alongside {@link Response#header(byte[])}

    + * The older {@code header(byte[])} overload takes an already-fully-rendered h1 field line + * (e.g. {@code "X-RateLimit-Limit: 100\r\n"}) — fine for h1, but not valid HPACK: HPACK encodes + * a header as a compressed (name, value) pair, never as a literal CRLF-terminated line, so a + * pre-rendered h1 line carries no information an HPACK encoder could reuse. {@code + * PreEncodedHeader} instead precomputes the {@code name}/{@code value} bytes separately + * (still once, still at boot) so either protocol's writer can render them in its own format — + * {@link Response#header(byte[])} is kept, working, for h1-only callers, but is documented as + * ignored on a future h2 response path (there is no way to recover structured name/value data + * from an opaque pre-rendered line); prefer this class for any header a handler wants to send on + * both protocols. + * + *

    The HPACK-encoded rendering itself is Phase 9 scope (no HPACK encoder exists yet) — this + * class stores the raw {@code name}/{@code value} bytes now, which is everything a future HPACK + * encoder needs to produce its own rendering from; it does not yet expose a precomputed HPACK + * byte form, since building one before HPACK exists would be speculative, untested API surface. + */ +public final class PreEncodedHeader { + private final byte[] nameBytes; + private final byte[] valueBytes; + + public PreEncodedHeader(String name, String value) { + this.nameBytes = name.getBytes(StandardCharsets.US_ASCII); + this.valueBytes = value.getBytes(StandardCharsets.US_ASCII); + } + + /** The header name's ASCII bytes, case as given to the constructor. Never copy-on-read — treat as immutable. */ + byte[] nameBytes() { + return nameBytes; + } + + /** The header value's ASCII bytes. Never copy-on-read — treat as immutable. */ + byte[] valueBytes() { + return valueBytes; + } + + @Override + public String toString() { + return new String(nameBytes, StandardCharsets.US_ASCII) + ": " + new String(valueBytes, StandardCharsets.US_ASCII); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof PreEncodedHeader other)) return false; + return Arrays.equals(nameBytes, other.nameBytes) && Arrays.equals(valueBytes, other.valueBytes); + } + + @Override + public int hashCode() { + return 31 * Arrays.hashCode(nameBytes) + Arrays.hashCode(valueBytes); + } +} diff --git a/flash/src/main/java/dev/relism/flash/models/QueryParams.java b/flash/src/main/java/dev/relism/flash/models/QueryParams.java index 45c3aaf..26cfc4b 100644 --- a/flash/src/main/java/dev/relism/flash/models/QueryParams.java +++ b/flash/src/main/java/dev/relism/flash/models/QueryParams.java @@ -42,7 +42,7 @@ public class QueryParams { * Returns a view over the first raw (not percent-decoded) value of {@code name}, or * {@code null}. {@code EX-05}: drawn from a small internal {@link SlicePool} when * {@link #raw} is array-backed (always true for h1 today) instead of allocated per call — - * same reuse-window contract as {@link HeaderMap#view}: valid until either the request ends + * same reuse-window contract as {@link Http1HeaderMap#view}: valid until either the request ends * or {@link #view} is called {@value #VIEW_POOL_SIZE} more times on this instance, whichever * comes first. Falls back to a fresh (allocating) view when {@link #raw} is not array-backed * — never exercised on the real request path (see {@link ArrayBackedByteView}'s Javadoc). diff --git a/flash/src/main/java/dev/relism/flash/models/Request.java b/flash/src/main/java/dev/relism/flash/models/Request.java index 3aaa58e..5809710 100644 --- a/flash/src/main/java/dev/relism/flash/models/Request.java +++ b/flash/src/main/java/dev/relism/flash/models/Request.java @@ -1,27 +1,23 @@ package dev.relism.flash.models; +import dev.relism.flash.Flash; import dev.relism.flash.RequestParser; import dev.relism.flash.bytes.ArrayBackedByteView; import dev.relism.fpr.core.ByteView; import dev.relism.flash.http.HttpMethod; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.ToString; -import lombok.Value; -import lombok.experimental.NonFinal; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; -import java.io.InputStream; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.util.List; /** - * Immutable view of an incoming HTTP/1.1 request. Constructed by {@link RequestParser} - * and passed directly to route handlers; never modified after creation (path/query params are - * injected once by the router before the handler runs). + * View of an incoming HTTP/1.1 request. Constructed once per connection by {@link RequestParser} + * and repositioned (never reallocated) for every request on that connection — never modified by + * user code after creation (path/query params are injected once by the router before the + * handler runs). * * 

    {@code
  * server.get("/users/{id}", (req, res) -> {
@@ -32,62 +28,103 @@ import java.util.List;
  *     InputStream in = req.body().stream();         // zero-copy stream
  * });
  * }
    + * + *

    {@code EX-22}: pooled, not allocated per request

    + * A {@code Request} instance is owned by its connection (HTTP/1.1) or its stream (HTTP/2) and is + * recycled after the handler returns. Do not retain it — the same instance is repositioned + * over the next request's data as soon as this one's handler returns. {@code equals}/ + * {@code hashCode} are the inherited identity-based {@link Object} versions and are meaningless + * across requests (compare two different {@code Request}s from the same connection and they may + * be {@code ==} to each other despite describing entirely different requests, at different + * points in time). {@code String} values returned by {@link #path()}, {@link #header(String)}, + * {@link #param(String)}, {@link #query(String)} are independent heap copies and are always safe + * to retain past the handler. + * + *

    Dev-mode use-after-recycle guard

    + * When {@link Flash#DEV} is {@code true}, every accessor checks that this instance is still the + * one currently being handled; a call after the handler has already returned (e.g. from a + * captured reference in an async callback, a {@link java.util.concurrent.CompletableFuture} + * continuation, or a background thread) throws {@link IllegalStateException} immediately, + * loudly, and at the exact call site that misused it — instead of silently reading whatever the + * next (or a completely different) request happened to reset this instance to. In production + * this check is a single {@code boolean} field read gated behind a {@code static final} flag the + * JIT treats as a trusted constant once the class is initialized — see {@code DECISIONS.md} for + * the measured cost. */ -@Value -@ToString public class Request { - @Getter(lombok.AccessLevel.NONE) - @EqualsAndHashCode.Exclude - @ToString.Exclude - RequestBody body; + private RequestBody body; /** Internal: the parsed request line (method, path, query, protocol, headers). */ - RequestLine requestLine; + private RequestLine requestLine; - @NonFinal PathParams pathParams; - @NonFinal QueryParams queryParams; - @NonFinal String cachedPath; + private PathParams pathParams; + private QueryParams queryParams; + private String cachedPath; + + private InetSocketAddress remoteAddress; + private SSLSocket sslSocket; + + // EX-22 dev-mode poisoning guard: true from reset() until recycle() marks this instance + // unsafe to use further. Only consulted when poisoningEnabled is true (see checkActive()). + private boolean active; + + // Defaults to the real Flash.DEV value. Flash.DEV is a static final boolean fixed once at + // JVM startup (from a system property), so no individual test can toggle it — this field + // exists solely so RequestRecycleGuardTest can exercise the dev-mode branch without a + // fragile reflective override of a `static final` field. Package-private: only this + // package's own tests reach for it; production code never touches it. + private static volatile boolean poisoningEnabled = Flash.DEV; + + /** Test-only override of the dev-mode poisoning check — see the field's own comment. */ + static void setPoisoningEnabledForTesting(boolean enabled) { + poisoningEnabled = enabled; + } + + /** Pooled instance, populated later via {@link #reset}. One per connection — see {@link RequestParser}. */ + public Request() { + } + + /** Test / manual constructor — {@code remoteAddress()} returns {@code null}, {@code isSecure()} is {@code false}. */ + public Request(RequestLine requestLine, byte[] body) { + reset(requestLine, RequestBody.of(body), null, null); + } /** - * Remote socket address of the connected client. Set once at connection time from - * {@link java.net.Socket#getRemoteSocketAddress()} : the {@link InetSocketAddress} - * object already exists in the JDK and is passed by reference: zero allocation, - * zero copy. {@code null} only in test-constructed requests. - * - *

    Use {@link #remoteAddress()} to access it. String conversion - * ({@code .getAddress().getHostAddress()}) is deferred to the caller, lazy and - * only paid when actually needed. + * Repositions this instance over a new request. Package-private: only {@link RequestParser} + * (same package) calls this — user code never constructs or resets a {@code Request} + * directly outside the test constructor above. */ - @Getter(lombok.AccessLevel.NONE) - @EqualsAndHashCode.Exclude - @ToString.Exclude - InetSocketAddress remoteAddress; - - /** - * The accepted socket for this connection, or {@code null} if plain HTTP — set once per - * connection by {@link RequestParser}, same lifetime and reference-only cost as - * {@link #remoteAddress}. Every request on the same keep-alive connection shares the - * identical instance. - * - *

    Never exposed directly: {@link #isSecure()} and {@link #sslSession()} are the public - * surface. {@link javax.net.ssl.SSLSocket#getSession()} is deferred to {@link #sslSession()} - * rather than called here — by the time a handler can call it, the handshake this connection - * needed to reach the handler has already completed, so it is a cached-field read, never a - * forced handshake. - */ - @Getter(lombok.AccessLevel.NONE) - @EqualsAndHashCode.Exclude - @ToString.Exclude - SSLSocket sslSocket; - - private Request(RequestLine requestLine, RequestBody body, InetSocketAddress remoteAddress, SSLSocket sslSocket) { + void reset(RequestLine requestLine, RequestBody body, InetSocketAddress remoteAddress, SSLSocket sslSocket) { this.requestLine = requestLine; this.body = body; this.pathParams = null; this.queryParams = null; + this.cachedPath = null; this.remoteAddress = remoteAddress; this.sslSocket = sslSocket; + this.active = true; + } + + /** + * Marks this instance unsafe for further use. Called by the connection driver (e.g. + * {@code Http1Connection}) once the handler (and any automatic post-handler work, e.g. + * {@link #drain()}) has finished with it, before the connection loop reuses it for the next + * request — {@code public} because the connection driver lives in a different package + * (matching {@link RequestLine#reset}'s own precedent), not because user code should ever + * call it. A no-op in production beyond the field write — see the class Javadoc's dev-mode + * guard section. + */ + public void recycle() { + this.active = false; + } + + private void checkActive() { + if (poisoningEnabled && !active) { + throw new IllegalStateException( + "Request used after the handler returned — do not retain a Request past the " + + "handler; copy any String values you need instead"); + } } /** @@ -97,31 +134,30 @@ public class Request { */ void setPathParams(PathParams p) { this.pathParams = p; } - /** Test / manual constructor — {@code remoteAddress()} returns {@code null}, {@code isSecure()} is {@code false}. */ - public Request(RequestLine requestLine, byte[] body) { - this(requestLine, RequestBody.of(body), null, null); - } - - public static Request forParsed(RequestLine requestLine, InputStream stream, - long contentLength, byte[] headerBuf, - int bodyStart, int preBufLen, + /** + * Repositions {@code pooled} over a freshly-parsed request. {@code body} is already fully + * configured by the caller ({@code RequestParser}, which owns and resets its own pooled + * {@link RequestBody} for the fixed-length/chunked/empty cases — see {@code EX-22}) — this + * method's only job is wiring it, {@code requestLine}, and the connection identity fields + * into {@code pooled}. + */ + public static Request forParsed(Request pooled, RequestLine requestLine, RequestBody body, InetSocketAddress remoteAddress, SSLSocket sslSocket) { - RequestBody rb = contentLength > 0 ? new RequestBody(stream, contentLength, headerBuf, bodyStart, preBufLen) - : contentLength == 0 ? RequestBody.empty() - : /* chunked */ new RequestBody(stream, -1L, null, 0, 0); - return new Request(requestLine, rb, remoteAddress, sslSocket); + pooled.reset(requestLine, body, remoteAddress, sslSocket); + return pooled; } // ── Request line ────────────────────────────────────────────────────────── /** HTTP method ({@code GET}, {@code POST}, …). */ - public HttpMethod method() { return requestLine.getMethod(); } + public HttpMethod method() { checkActive(); return requestLine.getMethod(); } /** * Request path decoded as UTF-8. Includes a leading slash; never includes the query string. * Example: a request for {@code /users/42?page=1} returns {@code "/users/42"}. */ public String path() { + checkActive(); if (cachedPath != null) return cachedPath; ByteView v = requestLine.getPath(); // EX-25: one allocation via a direct String(array, offset, length) construction when the @@ -141,20 +177,20 @@ public class Request { * Returns the first value of header {@code name}, or {@code null} if absent. * Lookup is case-insensitive ({@code "content-type"} and {@code "Content-Type"} are equivalent). */ - public String header(String name) { return requestLine.getHeaders().first(name); } + public String header(String name) { checkActive(); return requestLine.getHeaders().first(name); } /** * Returns all values of header {@code name} in declaration order. * Useful for headers that appear multiple times (e.g. {@code Accept}, {@code Cookie}). * Lookup is case-insensitive. Returns an empty list if the header is absent. */ - public List headers(String name) { return requestLine.getHeaders().all(name); } + public List headers(String name) { checkActive(); return requestLine.getHeaders().all(name); } /** * Returns all header values in declaration order, one entry per header line. * Useful for debugging; for targeted access prefer {@link #header(String)}. */ - public List headers() { return requestLine.getHeaders().all(); } + public List headers() { checkActive(); return requestLine.getHeaders().all(); } // ── Path parameters ─────────────────────────────────────────────────────── @@ -164,7 +200,7 @@ public class Request { * injected by the router before the handler runs. Returns {@code null} if this * route has no such parameter or the route is not parametric. */ - public String param(String name) { return pathParams != null ? pathParams.get(name) : null; } + public String param(String name) { checkActive(); return pathParams != null ? pathParams.get(name) : null; } // ── Query parameters ────────────────────────────────────────────────────── @@ -173,14 +209,14 @@ public class Request { * The query string is parsed lazily on the first call and cached for the request lifetime. * For {@code ?a=1&a=2}, returns {@code "1"}. */ - public String query(String name) { return resolveQueryParams().get(name); } + public String query(String name) { checkActive(); return resolveQueryParams().get(name); } /** * Returns all query parameters named {@code name} in declaration order. * For {@code ?tag=a&tag=b}, returns {@code ["a", "b"]}. * Returns an empty list if the parameter is absent. */ - public List queries(String name) { return resolveQueryParams().getAll(name); } + public List queries(String name) { checkActive(); return resolveQueryParams().getAll(name); } // ── Remote address ──────────────────────────────────────────────────────── @@ -196,12 +232,12 @@ public class Request { * if (addr != null) String ip = addr.getAddress().getHostAddress(); * } */ - public InetSocketAddress remoteAddress() { return remoteAddress; } + public InetSocketAddress remoteAddress() { checkActive(); return remoteAddress; } // ── TLS ─────────────────────────────────────────────────────────────────── /** Whether this request arrived over TLS (HTTPS). */ - public boolean isSecure() { return sslSocket != null; } + public boolean isSecure() { checkActive(); return sslSocket != null; } /** * Returns the TLS session for this connection, or {@code null} for plain HTTP. @@ -211,7 +247,7 @@ public class Request { * diagnostics. {@code null} rather than throwing when {@link #isSecure()} is {@code false} — * check that first, or just null-check the result. */ - public SSLSession sslSession() { return sslSocket != null ? sslSocket.getSession() : null; } + public SSLSession sslSession() { checkActive(); return sslSocket != null ? sslSocket.getSession() : null; } // ── Body ────────────────────────────────────────────────────────────────── @@ -220,15 +256,22 @@ public class Request { * the full body or {@link RequestBody#stream()} for zero-copy streaming access. * The two modes are mutually exclusive per request. */ - public RequestBody body() { return body; } + public RequestBody body() { checkActive(); return body; } /** Discards unread body bytes; called by the server after each request on keep-alive connections. */ public void drain() { body.drain(); } // ── Internal ───────────────────────────────────────────────────────────── + /** Internal: the parsed request line (method, path, query, protocol, headers). */ + public RequestLine getRequestLine() { checkActive(); return requestLine; } + + /** Internal: path parameters injected by the router, or {@code null} if none matched. */ + public PathParams getPathParams() { checkActive(); return pathParams; } + /** Internal: case-insensitive header value comparison used by the server keep-alive logic. */ public boolean headerEquals(String name, String value) { + checkActive(); return requestLine.getHeaders().valueEqualsIgnoreCase(name, value); } @@ -239,4 +282,10 @@ public class Request { } return queryParams; } + + @Override + public String toString() { + return "Request(method=" + (requestLine != null ? requestLine.getMethod() : null) + + ", path=" + (requestLine != null ? requestLine.getPath() : null) + ")"; + } } diff --git a/flash/src/main/java/dev/relism/flash/models/RequestBody.java b/flash/src/main/java/dev/relism/flash/models/RequestBody.java index 371a41b..1f71813 100644 --- a/flash/src/main/java/dev/relism/flash/models/RequestBody.java +++ b/flash/src/main/java/dev/relism/flash/models/RequestBody.java @@ -10,9 +10,9 @@ import java.io.*; * Safe to call multiple times; the second call returns the cached array. Throws for * bodies larger than 2 GB.

    4. *
  4. {@link #stream()} — returns a bounded {@link InputStream} without upfront allocation. - * For fixed-length bodies this is a view into the already-buffered header bytes stitched - * to the socket; for chunked bodies it is the raw {@link dev.relism.ChunkedInputStream} - * that de-chunks on the fly.
    5. + * For fixed-length bodies this is a reused, repositioned view (see {@code EX-23} below) + * into the already-buffered header bytes stitched to the socket; for chunked bodies it is + * the raw {@link dev.relism.ChunkedInputStream} that de-chunks on the fly. * * *

    Mutual exclusivity: calling both {@code bytes()} and {@code stream()} on the same @@ -20,37 +20,68 @@ import java.io.*; * *

    Keep-alive: unread body bytes are discarded by {@link Request#drain()} after the * handler returns so the socket is correctly positioned for the next pipelined request. + * + *

    {@code EX-22}: pooled, not allocated per request

    + * One instance per connection (owned by {@code RequestParser}, repositioned via {@link #reset} + * for every request), the same treatment {@link Request}/{@link RequestLine} get. The {@link + * #of(byte[])} factory below remains for test/manual construction and returns a freestanding, + * unpooled instance — exactly like {@link Request}'s own manual constructor. + * + *

    {@code EX-23}/{@code EX-24}: the reusable bounded stream and drain buffer

    + * {@link #stream()} used to allocate a {@link SequenceInputStream}, a {@link ByteArrayInputStream} + * and an anonymous bounded {@link InputStream} on every call. It now hands out one persistent + * {@link BoundedBufferedInputStream}, repositioned per request instead of reallocated. + * {@link #drain()}'s chunked-body path used to call {@code InputStream.transferTo}, which + * allocates a fresh 8 KiB {@code byte[]} internally on every call (the JDK default + * implementation); it now drains through a lazily-created, persistent buffer instead. */ -public final class RequestBody { - private static final byte[] EMPTY_BYTES = new byte[0]; - - private final InputStream socket; - private final long contentLength; - private final byte[] preBuf; - private final int preBufOff; - private final int preBufLen; +public class RequestBody { + private InputStream socket; + private long contentLength; + private byte[] preBuf; + private int preBufOff; + private int preBufLen; private byte[] resolved; private long socketConsumed; + // EX-23: created once, repositioned per request via reset()'s call into boundedStream.reset(...). + private BoundedBufferedInputStream boundedStream; + + // EX-24: created lazily on first chunked-body drain(), then reused for the life of the connection. + private byte[] drainBuffer; + + /** Pooled instance, populated later via {@link #reset}. One per connection — see {@code RequestParser}. */ + public RequestBody() { + } + RequestBody(InputStream socket, long contentLength, byte[] preBuf, int preBufOff, int preBufLen) { + reset(socket, contentLength, preBuf, preBufOff, preBufLen); + } + + /** Pre-resolved body: test payloads and empty body — skips all I/O. Always a freestanding, unpooled instance. */ + private RequestBody(byte[] preResolved) { + reset(null, preResolved.length, null, 0, 0); + this.resolved = preResolved; + } + + /** + * Repositions this instance over a new request. {@code public} because {@code RequestParser} + * (a different package) owns and resets its own pooled instance directly — matching + * {@link RequestLine#reset}'s precedent — not because user code should ever call it. + */ + public void reset(InputStream socket, long contentLength, byte[] preBuf, int preBufOff, int preBufLen) { this.socket = socket; this.contentLength = contentLength; this.preBuf = preBuf; this.preBufOff = preBufOff; this.preBufLen = preBufLen; + this.resolved = null; + this.socketConsumed = 0; } - /** Pre-resolved body: test payloads and empty body — skips all I/O. */ - private RequestBody(byte[] preResolved) { - this(null, preResolved.length, null, 0, 0); - this.resolved = preResolved; - } - - private static final RequestBody EMPTY = new RequestBody(EMPTY_BYTES); - static RequestBody of(byte[] bytes) { return new RequestBody(bytes); } - static RequestBody empty() { return EMPTY; } + static RequestBody empty() { return new RequestBody(new byte[0]); } /** {@code true} if the body has zero bytes ({@code Content-Length: 0} or no body). */ public boolean isEmpty() { return contentLength == 0; } @@ -96,54 +127,92 @@ public final class RequestBody { /** * Returns a bounded {@link InputStream} over the body without upfront allocation. * - *

    For fixed-length bodies: a {@link SequenceInputStream} of any already-buffered header - * bytes followed by a bounded view of the socket stream — zero heap beyond those small - * pre-buffered bytes. + *

    For fixed-length bodies: a reused {@link BoundedBufferedInputStream} (see the class + * Javadoc, {@code EX-23}) serving any already-buffered header bytes followed by a bounded + * view of the socket stream — zero allocation on a warm connection. * *

    For chunked bodies: the raw {@link dev.relism.ChunkedInputStream} that de-chunks on * the fly; EOF signals the end of the logical body and leaves the socket positioned for * the next keep-alive request. * *

    If {@link #bytes()} was called first, returns a fresh {@link java.io.ByteArrayInputStream} - * over the cached array. + * over the cached array — a rare dual-access pattern, not the hot path {@code EX-23} targets. */ public InputStream stream() { if (resolved != null) return new ByteArrayInputStream(resolved); if (contentLength < 0) return socket; // ChunkedInputStream — EOF signals end of body + if (boundedStream == null) boundedStream = new BoundedBufferedInputStream(); int fromBuf = (int) Math.min(preBufLen, contentLength); long fromSocket = contentLength - fromBuf; - InputStream bufPart = new ByteArrayInputStream(preBuf, preBufOff, fromBuf); - return fromSocket == 0 ? bufPart : new SequenceInputStream(bufPart, bounded(socket, fromSocket)); + boundedStream.reset(preBuf, preBufOff, fromBuf, fromSocket); + return boundedStream; } /** Discards unread body bytes to reposition the socket for the next keep-alive request. */ void drain() { if (isEmpty() || resolved != null) return; if (contentLength < 0) { - try { socket.transferTo(OutputStream.nullOutputStream()); } catch (IOException ignored) {} + // EX-24: InputStream.transferTo's default implementation allocates a fresh 8 KiB + // byte[] on every call — replaced with a buffer this instance allocates once + // (lazily, only if a chunked body is ever actually drained) and reuses thereafter. + if (drainBuffer == null) drainBuffer = new byte[8192]; + try { + while (socket.read(drainBuffer) > 0) { /* discard */ } + } catch (IOException ignored) { + } return; } long remaining = (contentLength - preBufLen) - socketConsumed; if (remaining > 0) try { socket.skipNBytes(remaining); } catch (IOException ignored) {} } - private InputStream bounded(InputStream src, long limit) { - return new InputStream() { - private long left = limit; + /** + * {@code EX-23}: a reused, repositionable {@link InputStream} that serves bytes first from a + * caller-owned pre-buffered array, then from the socket, bounded overall to a fixed length — + * replacing the {@code SequenceInputStream}+{@code ByteArrayInputStream}+anonymous-bounded- + * stream trio that used to be allocated fresh on every {@link #stream()} call. One instance + * lives on the owning {@link RequestBody} for the whole connection; {@link #reset} repositions + * it for each new request. + */ + private final class BoundedBufferedInputStream extends InputStream { + private byte[] preBuf; + private int preBufPos; + private int preBufRemaining; + private long socketRemaining; - @Override public int read() throws IOException { - if (left == 0) return -1; - int b = src.read(); - if (b >= 0) { left--; socketConsumed++; } - return b; + void reset(byte[] preBuf, int preBufOff, int preBufLen, long socketRemaining) { + this.preBuf = preBuf; + this.preBufPos = preBufOff; + this.preBufRemaining = preBufLen; + this.socketRemaining = socketRemaining; + } + + @Override + public int read() throws IOException { + if (preBufRemaining > 0) { + preBufRemaining--; + return preBuf[preBufPos++] & 0xFF; } + if (socketRemaining == 0) return -1; + int b = socket.read(); + if (b >= 0) { socketRemaining--; socketConsumed++; } + return b; + } - @Override public int read(byte[] buf, int off, int len) throws IOException { - if (left == 0) return -1; - int n = src.read(buf, off, (int) Math.min(len, left)); - if (n > 0) { left -= n; socketConsumed += n; } + @Override + public int read(byte[] dst, int off, int len) throws IOException { + if (len == 0) return 0; + if (preBufRemaining > 0) { + int n = Math.min(len, preBufRemaining); + System.arraycopy(preBuf, preBufPos, dst, off, n); + preBufPos += n; + preBufRemaining -= n; return n; } - }; + if (socketRemaining == 0) return -1; + int n = socket.read(dst, off, (int) Math.min(len, socketRemaining)); + if (n > 0) { socketRemaining -= n; socketConsumed += n; } + return n; + } } } diff --git a/flash/src/main/java/dev/relism/flash/models/RequestLine.java b/flash/src/main/java/dev/relism/flash/models/RequestLine.java index 92e367e..3eee779 100644 --- a/flash/src/main/java/dev/relism/flash/models/RequestLine.java +++ b/flash/src/main/java/dev/relism/flash/models/RequestLine.java @@ -2,16 +2,65 @@ package dev.relism.flash.models; import dev.relism.fpr.core.ByteView; import dev.relism.flash.http.HttpMethod; -import lombok.ToString; -import lombok.Value; -@ToString -@Value +/** + * The parsed request line plus headers: method, path, optional query, optional protocol token, + * and the header container. Internal — reached via {@link Request#getRequestLine()}, not + * user-facing API. + * + *

    Pooled, like {@link Request} ({@code EX-22})

    + * One instance per connection, repositioned via {@link #reset} for every request rather than + * reallocated — {@code RequestParser} owns it exactly the way it owns {@link Http1HeaderMap}. + * {@link #reset} is {@code public} rather than package-private — matching + * {@link Http1HeaderMap#reset}'s and {@link PathParams#reset}'s own precedent — because + * {@code RequestParser} (the owner and sole caller) lives in a different package + * ({@code dev.relism.flash}, not {@code dev.relism.flash.models}). The public constructor below + * remains for test/manual construction and simply delegates to {@link #reset}. + * + *

    {@code protocol} is optional

    + * HTTP/1.1 always has a protocol token on the wire ({@code "HTTP/1.1"}); HTTP/2 has no equivalent + * — a stream's version is implicit in which connection it belongs to. {@link #getProtocol()} may + * be {@code null} for a header container built by a future non-h1 implementation; h1 always + * supplies a non-null value today. + */ public class RequestLine { - HttpMethod method; - ByteView path; - /** Raw query string bytes (after {@code ?}), {@code null} if the URI has no query string. */ - ByteView query; - ByteView protocol; - HeaderMap headers; + private HttpMethod method; + private ByteView path; + private ByteView query; + private ByteView protocol; + private HeaderView headers; + + /** Pooled instance, populated later via {@link #reset}. */ + public RequestLine() { + } + + /** Test / manual construction — delegates to {@link #reset}. */ + public RequestLine(HttpMethod method, ByteView path, ByteView query, ByteView protocol, HeaderView headers) { + reset(method, path, query, protocol, headers); + } + + /** Repositions this instance over a new request. See the class Javadoc for why this is {@code public}. */ + public void reset(HttpMethod method, ByteView path, ByteView query, ByteView protocol, HeaderView headers) { + this.method = method; + this.path = path; + this.query = query; + this.protocol = protocol; + this.headers = headers; + } + + public HttpMethod getMethod() { return method; } + public ByteView getPath() { return path; } + + /** Raw query string bytes (after {@code ?}), or {@code null} if the URI has no query string. */ + public ByteView getQuery() { return query; } + + /** The wire protocol token (e.g. {@code "HTTP/1.1"}), or {@code null} — see the class Javadoc. */ + public ByteView getProtocol() { return protocol; } + + public HeaderView getHeaders() { return headers; } + + @Override + public String toString() { + return "RequestLine(method=" + method + ", path=" + path + ")"; + } } diff --git a/flash/src/main/java/dev/relism/flash/models/Response.java b/flash/src/main/java/dev/relism/flash/models/Response.java index 4a60d9c..c1d0375 100644 --- a/flash/src/main/java/dev/relism/flash/models/Response.java +++ b/flash/src/main/java/dev/relism/flash/models/Response.java @@ -1,16 +1,17 @@ package dev.relism.flash.models; +import dev.relism.flash.Flash; +import dev.relism.flash.bytes.ByteWriter; import dev.relism.flash.http.ContentType; +import dev.relism.flash.http.Http1Limits; import dev.relism.flash.http.HttpStatus; -import lombok.Getter; -import lombok.Setter; -import lombok.ToString; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -26,20 +27,57 @@ import java.util.List; * // unknown-length stream → Transfer-Encoding: chunked * return new Response(200, ContentType.TEXT_PLAIN).chunked(source); * } + * + *

    {@code EX-21}: pooled, not allocated per request

    + * The connection driver (e.g. {@code Http1Connection}) owns one {@code Response} instance per + * connection, reset before every handler call rather than reallocated — the same treatment + * {@link Request} gets (see its Javadoc for the full pooling/dev-mode-guard rationale, which + * applies identically here). A handler that returns a different {@code Response} instance + * (e.g. {@code return new Response(404, "Not Found", ContentType.TEXT_PLAIN);}) is fully + * supported — that instance is a normal, unpooled, freshly-constructed object like any + * public-constructor {@code Response} always was; only the connection driver's own default + * instance is pooled and poisoned after use. */ -@Getter -@ToString public class Response { - @Setter private int statusCode; - private byte[] statusBytes; // pre-encoded "200 OK"; null when set via status(int) - private byte[] body; - @ToString.Exclude - private InputStream stream; - private long streamLength; // meaningful only when isStreaming() && !chunked - private boolean chunked; - private byte[] contentType; - @Getter(lombok.AccessLevel.NONE) - private List headers; // pre-encoded "Name: Value\r\n" entries + private int statusCode; + private byte[] statusBytes; // pre-encoded "200 OK"; null when set via status(int) + private byte[] body; + private InputStream stream; + private long streamLength; // meaningful only when isStreaming() && !chunked + private boolean chunked; + private byte[] contentType; + + // EX-20: custom headers stored as (name, value) byte pairs in one growable region, instead + // of a List of fully-rendered "Name: Value\r\n" lines (which cost a StringBuilder + + // char[] + String + getBytes() chain per header(String,String) call). Two backing stores, + // unified into one insertion-ordered sequence via headerTags/headerRefs, since a fully + // pre-rendered line (the legacy header(byte[]) overload) has no name/value structure to + // decompose into the same region: + // tag 0 -> a (name, value) pair; headerRefs[i] indexes headerQuads (groups of 4) + // tag 1 -> a raw pre-rendered line; headerRefs[i] indexes rawHeaderLines + private ByteWriter headerRegion; // tag-0 storage: name+value bytes back to back + private int[] headerQuads; // tag-0 storage: groups of (nameOff,nameLen,valOff,valLen) + private int headerQuadCount; + private List rawHeaderLines; // tag-1 storage: legacy header(byte[]) entries, verbatim + private byte[] headerTags; // one entry per header(), in call order: 0 or 1 + private int[] headerRefs; // one entry per header(), in call order: index into the tag's store + private int headerCount; // total header() calls this response has recorded + + // EX-21 dev-mode poisoning guard -- see Request's identical mechanism for the full rationale. + private boolean active = true; + private static volatile boolean poisoningEnabled = Flash.DEV; + + /** Test-only override of the dev-mode poisoning check — mirrors {@code Request}'s identical hook. */ + static void setPoisoningEnabledForTesting(boolean enabled) { + poisoningEnabled = enabled; + } + + private void checkActive() { + if (poisoningEnabled && !active) { + throw new IllegalStateException( + "Response used after the handler returned — do not retain a Response past the handler"); + } + } // ------------------------------------------------------------------------- // Constructors @@ -59,20 +97,57 @@ public class Response { this(statusCode, text.getBytes(StandardCharsets.UTF_8), contentType); } + // ------------------------------------------------------------------------- + // Pooling + // ------------------------------------------------------------------------- + + /** + * Repositions this instance for a new request/response cycle — clears the body, stream, + * status, content type, and every header recorded by the previous cycle. Public because the + * connection driver that owns the pooled instance lives in a different package (matching + * {@link RequestLine#reset}'s precedent); user code never calls this. + */ + public Response reset(int statusCode, ContentType contentType) { + this.statusCode = statusCode; + this.statusBytes = null; + this.body = null; + this.stream = null; + this.streamLength = 0; + this.chunked = false; + this.contentType = contentType.getBytes(); + this.headerQuadCount = 0; + this.headerCount = 0; + if (rawHeaderLines != null) rawHeaderLines.clear(); + this.active = true; + return this; + } + + /** + * Marks this instance unsafe for further use — see {@link Request#recycle()} for the full + * rationale, identical here. {@code public} for the same cross-package reason. + */ + public void recycle() { + this.active = false; + } + // ------------------------------------------------------------------------- // Fluent mutators // ------------------------------------------------------------------------- /** Sets the status code. The phrase is looked up from {@link HttpStatus} on the write path. */ - public Response status(int code) { this.statusCode = code; this.statusBytes = null; return this; } + public Response status(int code) { checkActive(); this.statusCode = code; this.statusBytes = null; return this; } + + /** Lombok-style setter kept for API compatibility — equivalent to {@link #status(int)} without the fluent return. */ + public void setStatusCode(int code) { status(code); } /** Sets the status from an {@link HttpStatus} constant. The pre-encoded bytes are used * directly on the write path — zero lookup, zero allocation. */ - public Response status(HttpStatus status) { this.statusCode = status.code(); this.statusBytes = status.bytes(); return this; } - public Response type(ContentType ct) { this.contentType = ct.getBytes(); return this; } - public Response type(String ct) { this.contentType = ct.getBytes(StandardCharsets.UTF_8); return this; } + public Response status(HttpStatus status) { checkActive(); this.statusCode = status.code(); this.statusBytes = status.bytes(); return this; } + public Response type(ContentType ct) { checkActive(); this.contentType = ct.getBytes(); return this; } + public Response type(String ct) { checkActive(); this.contentType = ct.getBytes(StandardCharsets.UTF_8); return this; } public Response body(byte[] bytes) { + checkActive(); this.body = bytes; this.stream = null; return this; @@ -84,6 +159,7 @@ public class Response { /** Streaming response with known length; written with {@code Content-Length}. */ public Response stream(InputStream is, long length) { + checkActive(); this.stream = is; this.streamLength = length; this.chunked = false; @@ -93,6 +169,7 @@ public class Response { /** Streaming response with unknown length; written with {@code Transfer-Encoding: chunked}. */ public Response chunked(InputStream is) { + checkActive(); this.stream = is; this.chunked = true; this.body = null; @@ -121,37 +198,150 @@ public class Response { * } */ public Response redirect(HttpStatus status, String url) { + checkActive(); this.statusCode = status.code(); this.statusBytes = status.bytes(); this.body = null; this.stream = null; - if (headers == null) headers = new ArrayList<>(); - headers.add(("Location: " + url + "\r\n").getBytes(StandardCharsets.UTF_8)); - return this; + return header("Location", url); } - /** Adds a response header. Encoded once at call time; zero-alloc on the write path. */ + /** + * Adds a response header. {@code EX-20}: writes {@code name}/{@code value} directly into a + * reused byte region (via {@link ByteWriter#writeAscii}) instead of building an intermediate + * {@code String} and re-encoding it — zero allocation once the region has grown to this + * connection's high-water mark. + */ public Response header(String name, String value) { - if (headers == null) headers = new ArrayList<>(); - headers.add((name + ": " + value + "\r\n").getBytes(StandardCharsets.UTF_8)); + checkActive(); + checkHeaderBudget(); + if (headerRegion == null) { + headerRegion = new ByteWriter(128); + headerQuads = new int[16]; + } + ensureQuadCapacity(headerQuadCount + 1); + int nameOff = headerRegion.length(); + headerRegion.writeAscii(name); + int nameLen = headerRegion.length() - nameOff; + int valOff = headerRegion.length(); + headerRegion.writeAscii(value); + int valLen = headerRegion.length() - valOff; + checkHeaderRegionBudget(); + + int base = headerQuadCount * 4; + headerQuads[base] = nameOff; + headerQuads[base + 1] = nameLen; + headerQuads[base + 2] = valOff; + headerQuads[base + 3] = valLen; + recordHeaderEntry((byte) 0, headerQuadCount); + headerQuadCount++; return this; } /** - * Adds a pre-encoded header (e.g. a static {@code "X-RateLimit-Limit: 100\r\n"} byte array - * pre-built at boot time). Zero-alloc on both the call path and the write path. + * Adds a header from a {@link PreEncodedHeader} built once (typically at boot). Copies its + * precomputed {@code name}/{@code value} bytes into this response's region — a memcpy, not a + * re-encode, and usable by a future h2 response path (unlike {@link #header(byte[])}) since + * the name/value structure survives. + */ + public Response header(PreEncodedHeader preEncoded) { + checkActive(); + checkHeaderBudget(); + if (headerRegion == null) { + headerRegion = new ByteWriter(128); + headerQuads = new int[16]; + } + ensureQuadCapacity(headerQuadCount + 1); + byte[] nameBytes = preEncoded.nameBytes(); + byte[] valueBytes = preEncoded.valueBytes(); + int nameOff = headerRegion.length(); + headerRegion.writeBytes(nameBytes); + int valOff = headerRegion.length(); + headerRegion.writeBytes(valueBytes); + checkHeaderRegionBudget(); + + int base = headerQuadCount * 4; + headerQuads[base] = nameOff; + headerQuads[base + 1] = nameBytes.length; + headerQuads[base + 2] = valOff; + headerQuads[base + 3] = valueBytes.length; + recordHeaderEntry((byte) 0, headerQuadCount); + headerQuadCount++; + return this; + } + + /** + * Adds a pre-encoded, fully-rendered header line (e.g. a static + * {@code "X-RateLimit-Limit: 100\r\n"} byte array pre-built at boot time). Zero-alloc on + * both the call path and the h1 write path. + * + *

    h1-only: a rendered {@code "Name: Value\r\n"} line carries no structured + * name/value data an HPACK encoder could use, so this header is not representable on a + * future h2 response path — prefer {@link #header(PreEncodedHeader)} for anything that must + * render correctly on both protocols. Kept for existing h1-only callers. */ public Response header(byte[] preEncoded) { - if (headers == null) headers = new ArrayList<>(); - headers.add(preEncoded); + checkActive(); + checkHeaderBudget(); + if (rawHeaderLines == null) rawHeaderLines = new ArrayList<>(); + rawHeaderLines.add(preEncoded); + recordHeaderEntry((byte) 1, rawHeaderLines.size() - 1); return this; } + /** + * {@code EX-nn}: bounds the response-side analogue of the request header limits — a handler + * that calls {@code header(...)} in an unbounded loop must not grow this connection's + * per-request scratch state without limit (Phase 6's zero-alloc DoD names this explicitly). + */ + private void checkHeaderBudget() { + if (headerCount >= Http1Limits.MAX_RESPONSE_HEADER_COUNT) { + throw new IllegalStateException("response exceeds " + Http1Limits.MAX_RESPONSE_HEADER_COUNT + + " headers — check for an unbounded loop calling header(...)"); + } + } + + private void checkHeaderRegionBudget() { + if (headerRegion.length() > Http1Limits.MAX_RESPONSE_HEADER_BYTES) { + throw new IllegalStateException("response header region exceeds " + + Http1Limits.MAX_RESPONSE_HEADER_BYTES + " bytes — check for an unbounded loop or an oversized value passed to header(...)"); + } + } + + private void recordHeaderEntry(byte tag, int ref) { + if (headerTags == null) { + headerTags = new byte[16]; + headerRefs = new int[16]; + } else if (headerCount == headerTags.length) { + int grown = headerTags.length * 2; + headerTags = Arrays.copyOf(headerTags, grown); + headerRefs = Arrays.copyOf(headerRefs, grown); + } + headerTags[headerCount] = tag; + headerRefs[headerCount] = ref; + headerCount++; + } + + private void ensureQuadCapacity(int neededQuads) { + int neededInts = neededQuads * 4; + if (neededInts <= headerQuads.length) return; + int grown = headerQuads.length; + while (grown < neededInts) grown *= 2; + headerQuads = Arrays.copyOf(headerQuads, grown); + } + // ------------------------------------------------------------------------- // State queries // ------------------------------------------------------------------------- - public boolean isStreaming() { return stream != null; } + public boolean isStreaming() { checkActive(); return stream != null; } + public boolean isChunked() { checkActive(); return chunked; } + public int getStatusCode() { checkActive(); return statusCode; } + public byte[] getStatusBytes() { checkActive(); return statusBytes; } + public byte[] getBody() { checkActive(); return body; } + public byte[] getContentType() { checkActive(); return contentType; } + public InputStream getStream() { checkActive(); return stream; } + public long getStreamLength() { checkActive(); return streamLength; } // ------------------------------------------------------------------------- // Internal setters used by HttpServer for handler return values @@ -164,6 +354,7 @@ public class Response { * serialize to {@code String}/{@code byte[]} before returning. */ public Response setBody(Object body) { + checkActive(); if (body instanceof byte[] bytes) { this.body = bytes; return this; } if (body instanceof String s) { this.body = s.getBytes(StandardCharsets.UTF_8); return this; } if (body instanceof CharSequence s) { this.body = s.toString().getBytes(StandardCharsets.UTF_8); return this; } @@ -173,12 +364,96 @@ public class Response { return this; } - /** Returns custom headers, or an empty list if none were added. */ - public List getHeaders() { return headers != null ? headers : List.of(); } + /** + * Returns custom headers as fully-rendered {@code "Name: Value\r\n"} lines, or an empty list + * if none were added. Introspection/debugging accessor — reconstructs each line from the + * internal region on every call, so it is not on the zero-alloc write path; {@link + * #writeHeaders} and {@link ResponseSerializer} read the internal representation directly + * instead of going through this method. + */ + public List getHeaders() { + checkActive(); + if (headerCount == 0) return List.of(); + List result = new ArrayList<>(headerCount); + for (int i = 0; i < headerCount; i++) { + if (headerTags[i] == 1) { + result.add(rawHeaderLines.get(headerRefs[i])); + } else { + int base = headerRefs[i] * 4; + byte[] region = headerRegion.array(); + int nameOff = headerQuads[base], nameLen = headerQuads[base + 1]; + int valOff = headerQuads[base + 2], valLen = headerQuads[base + 3]; + byte[] line = new byte[nameLen + 2 + valLen + 2]; + int p = 0; + System.arraycopy(region, nameOff, line, p, nameLen); p += nameLen; + line[p++] = ':'; line[p++] = ' '; + System.arraycopy(region, valOff, line, p, valLen); p += valLen; + line[p++] = '\r'; line[p] = '\n'; + result.add(line); + } + } + return result; + } - /** Writes pre-encoded custom headers directly to {@code out}. Zero-alloc when no headers are set. */ + /** + * Writes every custom header directly into {@code head} (a scratch {@link ByteWriter} — + * see {@code EX-27}), in call order. Zero-alloc when no headers are set or on a warm region. + * This is what {@code Http1ResponseWriter} uses; {@link #writeHeaders(OutputStream)} below + * (the {@code OutputStream} equivalent) exists for the streaming-body write paths that + * cannot fold their whole write into one scratch buffer. + */ + public void writeHeadersInto(ByteWriter head) { + for (int i = 0; i < headerCount; i++) { + if (headerTags[i] == 1) { + head.writeBytes(rawHeaderLines.get(headerRefs[i])); + } else { + int base = headerRefs[i] * 4; + byte[] region = headerRegion.array(); + head.writeBytes(region, headerQuads[base], headerQuads[base + 1]); + head.writeByte((byte) ':'); head.writeByte((byte) ' '); + head.writeBytes(region, headerQuads[base + 2], headerQuads[base + 3]); + head.writeByte((byte) '\r'); head.writeByte((byte) '\n'); + } + } + } + + /** Writes every custom header directly to {@code out}, in call order. Zero-alloc when no headers are set or on a warm region. */ public void writeHeaders(OutputStream out) throws IOException { - if (headers == null) return; - for (byte[] header : headers) out.write(header); + for (int i = 0; i < headerCount; i++) { + if (headerTags[i] == 1) { + out.write(rawHeaderLines.get(headerRefs[i])); + } else { + int base = headerRefs[i] * 4; + byte[] region = headerRegion.array(); + out.write(region, headerQuads[base], headerQuads[base + 1]); + out.write(':'); out.write(' '); + out.write(region, headerQuads[base + 2], headerQuads[base + 3]); + out.write('\r'); out.write('\n'); + } + } + } + + // ── Internal: name/value field enumeration for ResponseSerializer ────────── + + /** + * Visits every {@code header(String,String)}/{@code header(PreEncodedHeader)}-added field as + * a structured (name, value) byte range — not the {@code header(byte[])} legacy + * entries, which have no such structure (see that method's own Javadoc). Package-private: + * {@link ResponseSerializer} is this method's only caller. + */ + void forEachStructuredField(ResponseSerializer.FieldConsumer consumer) { + if (headerQuadCount == 0) return; + byte[] region = headerRegion.array(); + for (int i = 0; i < headerQuadCount; i++) { + int base = i * 4; + consumer.accept(region, headerQuads[base], headerQuads[base + 1], + region, headerQuads[base + 2], headerQuads[base + 3]); + } + } + + @Override + public String toString() { + return "Response(statusCode=" + statusCode + ", contentType=" + + (contentType != null ? new String(contentType, StandardCharsets.UTF_8) : null) + ")"; } } diff --git a/flash/src/main/java/dev/relism/flash/models/ResponseSerializer.java b/flash/src/main/java/dev/relism/flash/models/ResponseSerializer.java new file mode 100644 index 0000000..775fb45 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/models/ResponseSerializer.java @@ -0,0 +1,53 @@ +package dev.relism.flash.models; + +import java.nio.charset.StandardCharsets; + +/** + * The protocol-neutral enumeration of a {@link Response}'s header fields — one source of truth + * consumed by every protocol's own writer, so {@code Content-Type}/custom-header logic is never + * duplicated (and cannot drift) between {@code Http1ResponseWriter} and a future h2 encoder + * (Phase 9). {@code Http1ResponseWriter} renders each field as {@code "Name: Value\r\n"}; the h2 + * encoder will render the same fields via HPACK. + * + *

    Scope: response-object fields only, not connection framing

    + * Deliberately does not enumerate {@code Content-Length}, {@code Connection}, or + * {@code Date} — those are connection/transport framing decisions (body length, keep-alive + * negotiation, wall-clock time), not properties of the {@code Response} object itself, and HTTP/2 + * has no equivalent of {@code Connection} at all (RFC 9113 §8.2.2 forbids connection-specific + * fields in h2). Each protocol's own writer computes and emits those itself, exactly as + * {@code Http1ResponseWriter} already did before this class existed. + * + *

    Scope: excludes {@link Response#header(byte[])}'s legacy entries

    + * A header added via the raw, fully-pre-rendered {@code header(byte[])} overload has no + * recoverable (name, value) structure — see that method's own Javadoc — so it cannot appear in + * this enumeration. {@code Http1ResponseWriter} still renders it (via {@link + * Response#writeHeaders}, which handles both structured and raw entries, in the original call + * order); a future h2 writer will not be able to. + */ +public final class ResponseSerializer { + private ResponseSerializer() {} + + /** One rendered header field: a byte range for the name, and a byte range for the value — both slices of caller-owned arrays, never copied. */ + @FunctionalInterface + public interface FieldConsumer { + void accept(byte[] nameBuf, int nameOff, int nameLen, byte[] valueBuf, int valueOff, int valueLen); + } + + private static final byte[] CONTENT_TYPE_NAME = "Content-Type".getBytes(StandardCharsets.US_ASCII); + + /** + * Enumerates {@code response}'s fields in a fixed, deterministic order: {@code Content-Type} + * first (if set to a non-empty value — {@code EX-15}: {@code ContentType.NONE} emits + * nothing, never an empty-valued header line), then every {@code header(String,String)}/ + * {@code header(PreEncodedHeader)}-added field in call order. Zero allocation: every byte + * range handed to {@code consumer} is a slice of {@code response}'s own already-allocated + * buffers. + */ + public static void forEachField(Response response, FieldConsumer consumer) { + byte[] ct = response.getContentType(); + if (ct != null && ct.length > 0) { + consumer.accept(CONTENT_TYPE_NAME, 0, CONTENT_TYPE_NAME.length, ct, 0, ct.length); + } + response.forEachStructuredField(consumer); + } +} diff --git a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java index b2a2abd..13f5b18 100644 --- a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java +++ b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java @@ -41,10 +41,19 @@ public final class FastPathViews { return (long) LONG_VIEW_LE.get(array, pos); } + /** + * {@code EX-42}: not immutable — {@link #reset} repositions an existing instance over new + * bounds instead of requiring a fresh allocation. {@code RequestParser} owns one pooled + * instance per role (path/query/protocol) per connection and calls {@link #reset} on it for + * every request, the same "do not retain past the handler" pooling contract every other + * per-connection object in this codebase already follows ({@code Http1HeaderMap}, + * {@code RequestLine}, {@code Request}, {@code RequestBody}). The public constructor remains + * for one-shot, non-pooled use (tests, other call sites that build a single fixed view). + */ public static final class RequestByteView implements ArrayBackedByteView { - private final byte[] buffer; - private final int start; - private final int length; + private byte[] buffer; + private int start; + private int length; public RequestByteView(byte[] buffer, int start, int length) { this.buffer = buffer; @@ -52,6 +61,13 @@ public final class FastPathViews { this.length = length; } + /** Repositions this instance over new bounds. Zero allocation. */ + public void reset(byte[] buffer, int start, int length) { + this.buffer = buffer; + this.start = start; + this.length = length; + } + @Override public int length() { return length; diff --git a/flash/src/main/java/dev/relism/flash/template/ByteTemplate.java b/flash/src/main/java/dev/relism/flash/template/ByteTemplate.java index 1fff0ff..48762f7 100644 --- a/flash/src/main/java/dev/relism/flash/template/ByteTemplate.java +++ b/flash/src/main/java/dev/relism/flash/template/ByteTemplate.java @@ -2,21 +2,30 @@ package dev.relism.flash.template; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * Precompiled, allocation-minimal byte template. *

    * Placeholders of the form {@code {{name}}} are detected once at construction. - * Each {@link #render} call makes exactly one allocation: the output byte[]. + * Each {@link #render} call makes exactly one allocation: the output byte[] + * (plus one {@code byte[]} per distinct key-value pair, for its UTF-8 bytes). *

    * Layout: seg[0] slot[0] seg[1] slot[1] … seg[n-1] slot[n-1] seg[n] + * + *

    {@code EX-28}: slot lookup is O(1) per key-value pair, not O(slots)

    + * A slot name can appear more than once (e.g. {@code {{var}} == {{var}}}), so the map built at + * construction maps each name to the (usually single-element) array of every slot index using + * that name, instead of the nested "scan every slot for every pair" loop this used to do. */ public final class ByteTemplate { - private final byte[][] segments; // literal byte segments - private final String[] slots; // placeholder names in order - private final int staticLength; // sum of all segment lengths (precomputed) + private final byte[][] segments; // literal byte segments + private final String[] slots; // placeholder names in order + private final int staticLength; // sum of all segment lengths (precomputed) + private final Map slotIndex; // slot name -> every slot index using that name public ByteTemplate(String source) { List segs = new ArrayList<>(); @@ -40,27 +49,68 @@ public final class ByteTemplate { int sl = 0; for (byte[] s : segments) sl += s.length; staticLength = sl; + + Map> byName = new HashMap<>(); + for (int j = 0; j < slots.length; j++) { + byName.computeIfAbsent(slots[j], k -> new ArrayList<>()).add(j); + } + Map idx = new HashMap<>(); + for (Map.Entry> e : byName.entrySet()) { + int[] arr = new int[e.getValue().size()]; + for (int j = 0; j < arr.length; j++) arr[j] = e.getValue().get(j); + idx.put(e.getKey(), arr); + } + slotIndex = idx; } /** * Render with alternating key-value String pairs: {@code k1, v1, k2, v2, …} - * Unmatched slots are rendered as empty. + * Unmatched slots are rendered as empty. Allocates the returned {@code byte[]}; for a + * caller-supplied buffer see {@link #renderInto(byte[], int, String...)}. */ public byte[] render(String... kvPairs) { + byte[][] values = resolveValues(kvPairs); + byte[] out = new byte[length(values)]; + writeInto(out, 0, values); + return out; + } + + /** + * Renders into {@code buffer} starting at {@code offset}, making no allocation beyond the + * per-pair UTF-8 conversion of {@code kvPairs}' values. Returns the number of bytes written. + * + * @throws IndexOutOfBoundsException if {@code buffer} does not have enough room from {@code offset} + */ + public int renderInto(byte[] buffer, int offset, String... kvPairs) { + byte[][] values = resolveValues(kvPairs); + int len = length(values); + if (offset < 0 || offset + len > buffer.length) { + throw new IndexOutOfBoundsException( + "buffer too small: need " + len + " bytes at offset " + offset + ", have " + (buffer.length - offset)); + } + writeInto(buffer, offset, values); + return len; + } + + private byte[][] resolveValues(String[] kvPairs) { byte[][] values = new byte[slots.length][]; for (int i = 0; i + 1 < kvPairs.length; i += 2) { - String key = kvPairs[i]; + int[] matches = slotIndex.get(kvPairs[i]); + if (matches == null) continue; byte[] val = kvPairs[i + 1].getBytes(StandardCharsets.UTF_8); - for (int j = 0; j < slots.length; j++) { - if (slots[j].equals(key)) { values[j] = val; } - } + for (int idx : matches) values[idx] = val; } + return values; + } + private int length(byte[][] values) { int len = staticLength; for (byte[] v : values) if (v != null) len += v.length; + return len; + } - byte[] out = new byte[len]; - int pos = 0; + private void writeInto(byte[] out, int offset, byte[][] values) { + int pos = offset; for (int i = 0; i < slots.length; i++) { System.arraycopy(segments[i], 0, out, pos, segments[i].length); pos += segments[i].length; @@ -70,6 +120,5 @@ public final class ByteTemplate { } } System.arraycopy(segments[slots.length], 0, out, pos, segments[slots.length].length); - return out; } } diff --git a/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java b/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java index 61c4fe3..58b9ecf 100644 --- a/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java +++ b/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java @@ -1,5 +1,7 @@ package dev.relism.flash.transport; +import dev.relism.flash.bytes.ByteWriter; + import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -36,15 +38,20 @@ public final class ConnectionScratch { /** Matches the relay-buffer size the {@code ThreadLocal} it replaces used. */ public static final int RELAY_BUFFER_SIZE = 8192; - /** Large enough for the decimal digits of any {@code long}, including a sign. */ - public static final int DECIMAL_BUFFER_SIZE = 20; - - /** Scratch for {@code Http1ResponseWriter}'s decimal (status code / Content-Length) encoding. */ - public final byte[] decimalBuffer = new byte[DECIMAL_BUFFER_SIZE]; + /** Initial capacity for {@link #responseHead}; grows on demand like any {@link ByteWriter}. */ + public static final int RESPONSE_HEAD_INITIAL_SIZE = 1024; /** Scratch for relaying a streaming or chunked response body without allocating per response. */ public final byte[] relayBuffer = new byte[RELAY_BUFFER_SIZE]; + /** + * {@code EX-27}: the scratch {@code Http1ResponseWriter} serializes a whole response head + * (status line, {@code Content-Type}, {@code Date}, custom headers, {@code Content-Length}/ + * {@code Connection}, and — for small fixed bodies — the body itself) into before issuing a + * single bulk {@code write()}, instead of ~10 small {@code OutputStream.write} calls. + */ + public final ByteWriter responseHead = new ByteWriter(RESPONSE_HEAD_INITIAL_SIZE); + /** Scratch for the WebSocket handshake's {@code Sec-WebSocket-Accept} SHA-1 digest. */ public final MessageDigest sha1; @@ -60,9 +67,9 @@ public final class ConnectionScratch { /** Called by {@link ScratchPool} before handing a reused instance to a new connection. */ void reset() { sha1.reset(); - // decimalBuffer/relayBuffer need no clearing: every reader of either only ever reads - // back exactly the region the immediately preceding writer just wrote (writeLong fills - // from the end backward and reports its own start position; relay() reports its own - // fill length), so stale bytes from a previous connection are never observed. + responseHead.reset(); + // relayBuffer needs no clearing: every reader only ever reads back exactly the region + // the immediately preceding relay() call reports it filled, so stale bytes from a + // previous connection are never observed. } } diff --git a/flash/src/test/java/dev/relism/flash/RequestParserTest.java b/flash/src/test/java/dev/relism/flash/RequestParserTest.java index 5bb3df5..0bbc424 100644 --- a/flash/src/test/java/dev/relism/flash/RequestParserTest.java +++ b/flash/src/test/java/dev/relism/flash/RequestParserTest.java @@ -56,6 +56,32 @@ class RequestParserTest { assertEquals("2", r.query("page")); } + // --- EX-42: pooled RequestByteViews (path/query/protocol) don't leak across requests --- + + @Test + void samePooledParser_secondRequestWithoutQuery_doesNotLeakFirstRequestsQuery() throws IOException { + RequestParser parser = new RequestParser(); + byte[] first = req("GET /search?token=super-secret HTTP/1.1", "Host: a").replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8); + Request r1 = parser.parse(source(first)); + assertEquals("token=super-secret", r1.getRequestLine().getQuery().toString()); + + byte[] second = req("GET /health HTTP/1.1", "Host: a").replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8); + Request r2 = parser.parse(source(second)); + assertNull(r2.getRequestLine().getQuery(), "the second request must not see the first request's leftover query view"); + assertEquals("/health", r2.getRequestLine().getPath().toString()); + } + + @Test + void samePooledParser_secondRequest_seesOnlyItsOwnPathAndProtocol() throws IOException { + RequestParser parser = new RequestParser(); + Request r1 = parser.parse(source(req("GET /first HTTP/1.1", "Host: a").replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8))); + assertEquals("/first", r1.getRequestLine().getPath().toString()); + + Request r2 = parser.parse(source(req("POST /second HTTP/1.0", "Host: a").replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8))); + assertEquals("/second", r2.getRequestLine().getPath().toString()); + assertEquals("HTTP/1.0", r2.getRequestLine().getProtocol().toString()); + } + // --- headers --- @Test diff --git a/flash/src/test/java/dev/relism/flash/api/multipart/MultipartTest.java b/flash/src/test/java/dev/relism/flash/api/multipart/MultipartTest.java index 475267e..7ed3cbd 100644 --- a/flash/src/test/java/dev/relism/flash/api/multipart/MultipartTest.java +++ b/flash/src/test/java/dev/relism/flash/api/multipart/MultipartTest.java @@ -2,7 +2,7 @@ package dev.relism.flash.api.multipart; import dev.relism.fpr.core.ByteView; import dev.relism.flash.http.HttpMethod; -import dev.relism.flash.models.HeaderMap; +import dev.relism.flash.models.Http1HeaderMap; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestLine; import org.junit.jupiter.api.Test; @@ -41,7 +41,7 @@ class MultipartTest { private static Request request(byte[] bodyBytes) { String ct = "multipart/form-data; boundary=" + BOUNDARY; byte[] headerBuf = ("Content-Type: " + ct).getBytes(StandardCharsets.US_ASCII); - HeaderMap headers = new HeaderMap(); + Http1HeaderMap headers = new Http1HeaderMap(); headers.reset(headerBuf, 0, headerBuf.length); RequestLine line = new RequestLine(HttpMethod.POST, viewOf("/upload"), null, viewOf("HTTP/1.1"), headers); return new Request(line, bodyBytes); @@ -236,11 +236,65 @@ class MultipartTest { @Test void of_notMultipart_throws() { byte[] headerBuf = "Content-Type: application/json".getBytes(StandardCharsets.US_ASCII); - HeaderMap headers = new HeaderMap(); + Http1HeaderMap headers = new Http1HeaderMap(); headers.reset(headerBuf, 0, headerBuf.length); RequestLine line = new RequestLine(HttpMethod.POST, viewOf("/"), null, viewOf("HTTP/1.1"), headers); Request req = new Request(line, new byte[0]); assertThrows(IllegalArgumentException.class, () -> Multipart.of(req)); } + + // ------------------------------------------------------------------------- + // EX-29: resource-exhaustion bounds + // ------------------------------------------------------------------------- + + @Test + void field_bodyAboveMaxBufferedSize_throws() throws IOException { + // MAX_MULTIPART_BUFFERED_PART_SIZE is 10 MiB — one byte over must be rejected, not + // buffered whole into a single byte[]. + String tooBig = "z".repeat((int) dev.relism.flash.http.Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE + 1); + Multipart mp = Multipart.of(request(body(textPart("huge", tooBig)))); + assertThrows(IOException.class, () -> mp.field("huge")); + } + + @Test + void file_materializedDuringFullScan_aboveMaxBufferedSize_throws() throws IOException { + String tooBig = "z".repeat((int) dev.relism.flash.http.Http1Limits.MAX_MULTIPART_BUFFERED_PART_SIZE + 1); + Multipart mp = Multipart.of(request(body(filePart("f", "f.bin", "application/octet-stream", tooBig)))); + assertThrows(IOException.class, mp::parts); + } + + @Test + void scan_tooManyParts_throws() throws IOException { + String[] parts = new String[dev.relism.flash.http.Http1Limits.MAX_MULTIPART_PARTS + 1]; + for (int i = 0; i < parts.length; i++) parts[i] = textPart("f" + i, "v"); + Multipart mp = Multipart.of(request(body(parts))); + assertThrows(IOException.class, mp::parts); + } + + @Test + void partHeaders_tooManyHeaderLines_throws() throws IOException { + StringBuilder part = new StringBuilder("Content-Disposition: form-data; name=\"x\"\r\n"); + for (int i = 0; i <= dev.relism.flash.http.Http1Limits.MAX_MULTIPART_PART_HEADER_COUNT; i++) { + part.append("X-Extra-").append(i).append(": v\r\n"); + } + part.append("\r\nbody"); + Multipart mp = Multipart.of(request(body(part.toString()))); + assertThrows(IOException.class, () -> mp.field("x")); + } + + @Test + void partHeaderLine_tooLong_throws() throws IOException { + String longValue = "v".repeat(dev.relism.flash.http.Http1Limits.MAX_MULTIPART_HEADER_LINE_LENGTH + 1); + String part = "Content-Disposition: form-data; name=\"x\"\r\n" + + "X-Long: " + longValue + "\r\n\r\nbody"; + Multipart mp = Multipart.of(request(body(part))); + assertThrows(IOException.class, () -> mp.field("x")); + } + + @Test + void withinAllLimits_stillWorksNormally() throws IOException { + // Sanity check the bounds above don't false-positive on a normal small request. + assertEquals("alice", Multipart.of(request(body(textPart("username", "alice")))).field("username")); + } } diff --git a/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java b/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java index 222fe80..2b978d0 100644 --- a/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java +++ b/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java @@ -93,6 +93,13 @@ class ByteWriterTest { assertEquals("content-type", asString(w)); } + @Test + void writeAscii_preservesCase() { + ByteWriter w = new ByteWriter(4); + w.writeAscii("Content-TYPE"); + assertEquals("Content-TYPE", asString(w)); + } + @Test void writeUInt16_bigEndian() { ByteWriter w = new ByteWriter(4); diff --git a/flash/src/test/java/dev/relism/flash/http1/Http1ResponseWriterTest.java b/flash/src/test/java/dev/relism/flash/http1/Http1ResponseWriterTest.java index a2f4233..19e09cc 100644 --- a/flash/src/test/java/dev/relism/flash/http1/Http1ResponseWriterTest.java +++ b/flash/src/test/java/dev/relism/flash/http1/Http1ResponseWriterTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.api.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import static org.junit.jupiter.api.Assertions.*; @@ -133,4 +134,52 @@ class Http1ResponseWriterTest { String raw = write(response, HttpMethod.GET, false, false); assertTrue(raw.contains("Connection: close\r\n"), raw); } + + // --- EX-27: one bulk write for a small fixed body ----------------------------- + + /** Counts calls to {@code write(byte[], int, int)} — the only overload {@link Http1ResponseWriter} uses. */ + private static final class CountingOutputStream extends java.io.OutputStream { + final ByteArrayOutputStream sink = new ByteArrayOutputStream(); + int arrayWriteCalls; + + @Override public void write(int b) { sink.write(b); } + + @Override + public void write(byte[] b, int off, int len) { + arrayWriteCalls++; + sink.write(b, off, len); + } + } + + @Test + void smallFixedBody_isWrittenInExactlyOneCall() throws IOException { + CountingOutputStream out = new CountingOutputStream(); + Response response = new Response(200, "hello world", ContentType.TEXT_PLAIN); + Http1ResponseWriter.writeResponse(out, response, HttpMethod.GET, true, false, scratch()); + + assertEquals(1, out.arrayWriteCalls, "head + small body must leave in a single write() call"); + assertTrue(out.sink.toString(StandardCharsets.UTF_8).endsWith("hello world")); + } + + @Test + void bodyAboveInlineThreshold_isWrittenInTwoCalls() throws IOException { + CountingOutputStream out = new CountingOutputStream(); + byte[] bigBody = new byte[dev.relism.flash.http.Http1Limits.INLINE_BODY_THRESHOLD + 1]; + Arrays.fill(bigBody, (byte) 'x'); + Response response = new Response(200, bigBody, ContentType.BINARY); + Http1ResponseWriter.writeResponse(out, response, HttpMethod.GET, true, false, scratch()); + + assertEquals(2, out.arrayWriteCalls, "head and an over-threshold body are written separately"); + assertTrue(out.sink.toString(StandardCharsets.UTF_8).endsWith("x".repeat(bigBody.length))); + } + + @Test + void headResponse_stillOneCall_noBodyBytes() throws IOException { + CountingOutputStream out = new CountingOutputStream(); + Response response = new Response(200, "hello world", ContentType.TEXT_PLAIN); + Http1ResponseWriter.writeResponse(out, response, HttpMethod.HEAD, true, false, scratch()); + + assertEquals(1, out.arrayWriteCalls); + assertFalse(out.sink.toString(StandardCharsets.UTF_8).contains("hello world")); + } } diff --git a/flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java b/flash/src/test/java/dev/relism/flash/models/Http1HeaderMapIndexTest.java similarity index 85% rename from flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java rename to flash/src/test/java/dev/relism/flash/models/Http1HeaderMapIndexTest.java index 4d24c9d..cc5d0c7 100644 --- a/flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java +++ b/flash/src/test/java/dev/relism/flash/models/Http1HeaderMapIndexTest.java @@ -8,25 +8,25 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.*; /** - * {@code EX-09}: dedicated correctness coverage for {@link HeaderMap}'s per-{@code reset()} + * {@code EX-09}: dedicated correctness coverage for {@link Http1HeaderMap}'s per-{@code reset()} * index — duplicate names, case variation, zero headers, and growth past the initial index * capacity up to {@code Http1Limits.MAX_HEADER_COUNT}. {@link HeaderMapTest} already covers the * ordinary lookup/forEach contract; this class targets the index machinery specifically. */ -class HeaderMapIndexTest { +class Http1HeaderMapIndexTest { - private static HeaderMap parse(String... headers) { + private static Http1HeaderMap parse(String... headers) { StringBuilder sb = new StringBuilder(); for (String h : headers) sb.append(h).append("\r\n"); byte[] buffer = sb.toString().getBytes(StandardCharsets.UTF_8); - HeaderMap map = new HeaderMap(); + Http1HeaderMap map = new Http1HeaderMap(); map.reset(buffer, 0, buffer.length); return map; } @Test void zeroHeaders_everyLookupIsEmpty() { - HeaderMap map = parse(); + Http1HeaderMap map = parse(); assertNull(map.first("Host")); assertTrue(map.all("Host").isEmpty()); assertTrue(map.all().isEmpty()); @@ -36,14 +36,14 @@ class HeaderMapIndexTest { @Test void duplicateHeaderNames_firstReturnsTheFirstOne_allReturnsAllInOrder() { - HeaderMap map = parse("X-Trace: a", "X-Trace: b", "X-Trace: c"); + Http1HeaderMap map = parse("X-Trace: a", "X-Trace: b", "X-Trace: c"); assertEquals("a", map.first("X-Trace")); assertEquals(List.of("a", "b", "c"), map.all("X-Trace")); } @Test void caseVariation_indexHashAndCompareBothIgnoreCase() { - HeaderMap map = parse("X-Custom-Header: value1"); + Http1HeaderMap map = parse("X-Custom-Header: value1"); assertEquals("value1", map.first("x-custom-header")); assertEquals("value1", map.first("X-CUSTOM-HEADER")); assertEquals("value1", map.first("X-cUsToM-hEaDeR")); @@ -52,7 +52,7 @@ class HeaderMapIndexTest { @Test void similarButDistinctNames_doNotCollideInTheIndex() { // Names sharing a hash-prefix-adjacent shape must still resolve independently. - HeaderMap map = parse("Accept: a", "Accept-Encoding: b", "Accept-Language: c"); + Http1HeaderMap map = parse("Accept: a", "Accept-Encoding: b", "Accept-Language: c"); assertEquals("a", map.first("Accept")); assertEquals("b", map.first("Accept-Encoding")); assertEquals("c", map.first("Accept-Language")); @@ -63,7 +63,7 @@ class HeaderMapIndexTest { int n = dev.relism.flash.http.Http1Limits.MAX_HEADER_COUNT; String[] headers = new String[n]; for (int i = 0; i < n; i++) headers[i] = "X-Header-" + i + ": value-" + i; - HeaderMap map = parse(headers); + Http1HeaderMap map = parse(headers); assertEquals("value-0", map.first("X-Header-0")); assertEquals("value-" + (n - 1), map.first("X-Header-" + (n - 1))); @@ -73,7 +73,7 @@ class HeaderMapIndexTest { @Test void reset_rebuildsIndexFromScratch_noStaleEntriesFromPreviousRequest() { - HeaderMap map = parse("Host: first-request"); + Http1HeaderMap map = parse("Host: first-request"); assertEquals("first-request", map.first("Host")); assertNull(map.first("X-Only-In-Second")); @@ -88,7 +88,7 @@ class HeaderMapIndexTest { void repeatedResetsAcrossVaryingHeaderCounts_shrinkAndGrowSafely() { // A connection whose successive keep-alive requests have very different header counts // must never see stale entries from a larger previous request bleed into a smaller one. - HeaderMap map = new HeaderMap(); + Http1HeaderMap map = new Http1HeaderMap(); for (int round = 0; round < 5; round++) { int n = (round % 2 == 0) ? 20 : 2; String[] headers = new String[n]; @@ -109,7 +109,7 @@ class HeaderMapIndexTest { // re-trigger index growth (Arrays.copyOf inside ensureIndexCapacity) after the first // reset() has already sized the arrays for this header count — asserted by identity: the // backing array references must be the exact same objects before and after 100k lookups. - HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4"); + Http1HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4"); int[] namesBefore = arrayFieldValue(map, "nameOffsets"); for (int i = 0; i < 100_000; i++) { @@ -124,11 +124,11 @@ class HeaderMapIndexTest { @Test void view_poolWraparound_aliasesAnEarlierReturnedView() { - // EX-05's documented hazard, demonstrated through the actual public API: HeaderMap's + // EX-05's documented hazard, demonstrated through the actual public API: Http1HeaderMap's // view() pool is sized 4 (VIEW_POOL_SIZE); a 5th call in the same request wraps around // and silently repositions the object the 1st call returned. dev.relism.fpr.core.ByteView v1 = null; - HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4", "E: 5"); + Http1HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4", "E: 5"); for (String name : new String[]{"A", "B", "C", "D"}) { dev.relism.fpr.core.ByteView v = map.view(name); if (v1 == null) v1 = v; @@ -139,9 +139,9 @@ class HeaderMapIndexTest { assertEquals('5', v1.byteAt(0)); // v1 is now silently "E"'s value, not "A"'s } - private static int[] arrayFieldValue(HeaderMap map, String fieldName) { + private static int[] arrayFieldValue(Http1HeaderMap map, String fieldName) { try { - var field = HeaderMap.class.getDeclaredField(fieldName); + var field = Http1HeaderMap.class.getDeclaredField(fieldName); field.setAccessible(true); return (int[]) field.get(map); } catch (ReflectiveOperationException e) { diff --git a/flash/src/test/java/dev/relism/flash/models/HeaderMapTest.java b/flash/src/test/java/dev/relism/flash/models/Http1HeaderMapTest.java similarity index 78% rename from flash/src/test/java/dev/relism/flash/models/HeaderMapTest.java rename to flash/src/test/java/dev/relism/flash/models/Http1HeaderMapTest.java index 08a6ad0..140378a 100644 --- a/flash/src/test/java/dev/relism/flash/models/HeaderMapTest.java +++ b/flash/src/test/java/dev/relism/flash/models/Http1HeaderMapTest.java @@ -8,15 +8,15 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.*; -class HeaderMapTest { +class Http1HeaderMapTest { // --- helpers --- - private static HeaderMap parse(String... headers) { + private static Http1HeaderMap parse(String... headers) { StringBuilder sb = new StringBuilder(); for (String h : headers) sb.append(h).append("\r\n"); byte[] buffer = sb.toString().getBytes(StandardCharsets.UTF_8); - HeaderMap map = new HeaderMap(); + Http1HeaderMap map = new Http1HeaderMap(); map.reset(buffer, 0, buffer.length); return map; } @@ -25,21 +25,21 @@ class HeaderMapTest { @Test void first_existingHeader() { - HeaderMap map = parse("Host: localhost", "Accept: text/plain"); + Http1HeaderMap map = parse("Host: localhost", "Accept: text/plain"); assertEquals("localhost", map.first("Host")); assertEquals("text/plain", map.first("Accept")); } @Test void first_caseInsensitive() { - HeaderMap map = parse("ConteNT-tYPe: application/json"); + Http1HeaderMap map = parse("ConteNT-tYPe: application/json"); assertEquals("application/json", map.first("content-type")); assertEquals("application/json", map.first("CONTENT-TYPE")); } @Test void first_missingHeader_returnsNull() { - HeaderMap map = parse("Host: localhost"); + Http1HeaderMap map = parse("Host: localhost"); assertNull(map.first("Accept")); } @@ -47,19 +47,19 @@ class HeaderMapTest { @Test void all_multipleValuesByName() { - HeaderMap map = parse("Cookie: a=1", "Set-Cookie: token=123", "Cookie: b=2"); + Http1HeaderMap map = parse("Cookie: a=1", "Set-Cookie: token=123", "Cookie: b=2"); assertEquals(List.of("a=1", "b=2"), map.all("Cookie")); } @Test void all_missingHeader_returnsEmptyList() { - HeaderMap map = parse("Host: localhost"); + Http1HeaderMap map = parse("Host: localhost"); assertTrue(map.all("Cookie").isEmpty()); } @Test void all_returnsAllHeaders() { - HeaderMap map = parse("A: 1", "B: 2"); + Http1HeaderMap map = parse("A: 1", "B: 2"); assertEquals(List.of("1", "2"), map.all()); } @@ -67,7 +67,7 @@ class HeaderMapTest { @Test void view_returnsZeroCopyView() { - HeaderMap map = parse("Host: localhost"); + Http1HeaderMap map = parse("Host: localhost"); ByteView view = map.view("Host"); assertNotNull(view); assertEquals(9, view.length()); @@ -77,7 +77,7 @@ class HeaderMapTest { @Test void view_missingHeader_returnsNull() { - HeaderMap map = parse("Host: localhost"); + Http1HeaderMap map = parse("Host: localhost"); assertNull(map.view("Accept")); } @@ -85,7 +85,7 @@ class HeaderMapTest { @Test void emptyMap_returnsNullAndEmptyList() { - HeaderMap map = new HeaderMap(); + Http1HeaderMap map = new Http1HeaderMap(); assertNull(map.first("Host")); assertTrue(map.all("Host").isEmpty()); assertTrue(map.all().isEmpty()); @@ -95,7 +95,7 @@ class HeaderMapTest { @Test void forEach_visitsEveryHeaderInDeclarationOrder() { - HeaderMap map = parse("Host: localhost", "Accept: text/plain", "Cookie: a=1"); + Http1HeaderMap map = parse("Host: localhost", "Accept: text/plain", "Cookie: a=1"); List seen = new java.util.ArrayList<>(); map.forEach((name, value) -> seen.add(toStr(name) + "=" + toStr(value))); assertEquals(List.of("Host=localhost", "Accept=text/plain", "Cookie=a=1"), seen); @@ -103,7 +103,7 @@ class HeaderMapTest { @Test void forEach_emptyMap_neverInvokesConsumer() { - HeaderMap map = new HeaderMap(); + Http1HeaderMap map = new Http1HeaderMap(); map.forEach((name, value) -> fail("must not be called on an empty map")); } @@ -111,7 +111,7 @@ class HeaderMapTest { void forEach_reusesTheSameTwoViewInstancesAcrossEveryHeader() { // The zero-allocation contract: forEach must reposition two ByteViews in place, not // allocate a fresh pair per header — same instances across all three calls here. - HeaderMap map = parse("A: 1", "B: 2", "C: 3"); + Http1HeaderMap map = parse("A: 1", "B: 2", "C: 3"); List names = new java.util.ArrayList<>(); List values = new java.util.ArrayList<>(); map.forEach((name, value) -> { names.add(name); values.add(value); }); diff --git a/flash/src/test/java/dev/relism/flash/models/RequestBodyTest.java b/flash/src/test/java/dev/relism/flash/models/RequestBodyTest.java index ea7f2b4..e3b4b4e 100644 --- a/flash/src/test/java/dev/relism/flash/models/RequestBodyTest.java +++ b/flash/src/test/java/dev/relism/flash/models/RequestBodyTest.java @@ -161,4 +161,46 @@ class RequestBodyTest { body.drain(); assertEquals(0, socket.available()); } + + // --- EX-22/EX-23: pooled instance, repositioned via reset() -------------------- + + @Test + void reset_repositionsSamePooledInstance_overSuccessiveRequests() throws IOException { + RequestBody body = new RequestBody(); // pooled ctor — no I/O configured yet + + byte[] first = "first".getBytes(StandardCharsets.UTF_8); + body.reset(new ByteArrayInputStream(first), 5, new byte[0], 0, 0); + assertArrayEquals(first, body.bytes()); + + byte[] second = "second-request".getBytes(StandardCharsets.UTF_8); + body.reset(new ByteArrayInputStream(second), second.length, new byte[0], 0, 0); + assertArrayEquals(second, body.bytes(), "reset() must not leak the previous request's resolved body"); + } + + @Test + void stream_reusesTheSameBoundedStreamInstance_acrossResets() throws IOException { + RequestBody body = new RequestBody(); + + body.reset(new ByteArrayInputStream("one".getBytes(StandardCharsets.UTF_8)), 3, new byte[0], 0, 0); + InputStream stream1 = body.stream(); + assertEquals("one", new String(stream1.readAllBytes(), StandardCharsets.UTF_8)); + + body.reset(new ByteArrayInputStream("two".getBytes(StandardCharsets.UTF_8)), 3, new byte[0], 0, 0); + InputStream stream2 = body.stream(); + assertSame(stream1, stream2, "EX-23: stream() must reposition the one pooled BoundedBufferedInputStream, not allocate a new one per request"); + assertEquals("two", new String(stream2.readAllBytes(), StandardCharsets.UTF_8)); + } + + @Test + void drain_reusesTheSameDrainBuffer_acrossChunkedResets() throws IOException { + RequestBody body = new RequestBody(); + + body.reset(new ByteArrayInputStream("chunk one".getBytes(StandardCharsets.UTF_8)), -1L, null, 0, 0); + body.drain(); + + ByteArrayInputStream secondSocket = new ByteArrayInputStream("chunk two".getBytes(StandardCharsets.UTF_8)); + body.reset(secondSocket, -1L, null, 0, 0); + body.drain(); + assertEquals(0, secondSocket.available(), "drain() must fully consume the second request's chunked body too"); + } } diff --git a/flash/src/test/java/dev/relism/flash/models/RequestLineTest.java b/flash/src/test/java/dev/relism/flash/models/RequestLineTest.java index bccb764..572ed4d 100644 --- a/flash/src/test/java/dev/relism/flash/models/RequestLineTest.java +++ b/flash/src/test/java/dev/relism/flash/models/RequestLineTest.java @@ -28,7 +28,7 @@ class RequestLineTest { ByteView path = viewOf("/api"); ByteView query = viewOf("q=1"); ByteView proto = viewOf("HTTP/1.1"); - HeaderMap headers = new HeaderMap(); + Http1HeaderMap headers = new Http1HeaderMap(); RequestLine rl = new RequestLine(HttpMethod.GET, path, query, proto, headers); diff --git a/flash/src/test/java/dev/relism/flash/models/RequestPoolingTest.java b/flash/src/test/java/dev/relism/flash/models/RequestPoolingTest.java new file mode 100644 index 0000000..9634dd7 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/RequestPoolingTest.java @@ -0,0 +1,103 @@ +package dev.relism.flash.models; + +import dev.relism.flash.http.HttpMethod; +import dev.relism.fpr.core.ByteView; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-22}: {@link Request} is pooled per connection (one instance owned by + * {@code RequestParser}, repositioned via {@link Request#forParsed} for every request on that + * connection) — not via a shared cross-connection pool. The plan's own safety-check wording + * ("connection A's {@code Authorization} header must never be visible on connection B") describes + * a threat model that does not structurally apply to this design: two different connections + * never share a {@code Request} instance at all (each owns its own {@code RequestParser}, hence + * its own {@code Request}) — see {@code DECISIONS.md} for the pooling-granularity decision this + * follows from. The real, applicable threat this class actually tests: request N+1 on + * the *same* keep-alive connection must never see stale data left over from request N, + * since those two requests genuinely do share one {@code Request} instance. + */ +class RequestPoolingTest { + + private static ByteView viewOf(String s) { + byte[] bytes = s.getBytes(StandardCharsets.UTF_8); + return new ByteView() { + public int length() { return bytes.length; } + public byte byteAt(int idx) { return bytes[idx]; } + }; + } + + private static Http1HeaderMap headersOf(String... rawLines) { + StringBuilder sb = new StringBuilder(); + for (String line : rawLines) sb.append(line).append("\r\n"); + byte[] buf = sb.toString().getBytes(StandardCharsets.UTF_8); + Http1HeaderMap map = new Http1HeaderMap(); + map.reset(buf, 0, buf.length); + return map; + } + + @Test + void forParsed_reusesTheSamePooledInstance_neverAllocatesANewOne() { + Request pooled = new Request(); + RequestLine line1 = new RequestLine(HttpMethod.GET, viewOf("/a"), null, viewOf("HTTP/1.1"), headersOf()); + Request r1 = Request.forParsed(pooled, line1, RequestBody.empty(), null, null); + assertSame(pooled, r1); + + RequestLine line2 = new RequestLine(HttpMethod.POST, viewOf("/b"), null, viewOf("HTTP/1.1"), headersOf()); + Request r2 = Request.forParsed(pooled, line2, RequestBody.empty(), null, null); + assertSame(pooled, r2); + assertSame(r1, r2, "the same pooled instance must be returned for every request on one connection"); + } + + @Test + void secondRequest_onSameConnection_doesNotSeeFirstRequestsAuthorizationHeader() { + Request pooled = new Request(); + + RequestLine first = new RequestLine(HttpMethod.GET, viewOf("/secure"), null, viewOf("HTTP/1.1"), + headersOf("Authorization: Bearer super-secret-token-A")); + Request r1 = Request.forParsed(pooled, first, RequestBody.empty(), null, null); + assertEquals("Bearer super-secret-token-A", r1.header("Authorization")); + + // A second request on the same keep-alive connection, with no Authorization header at all. + RequestLine second = new RequestLine(HttpMethod.GET, viewOf("/public"), null, viewOf("HTTP/1.1"), + headersOf("Host: example.com")); + Request r2 = Request.forParsed(pooled, second, RequestBody.empty(), null, null); + + assertNull(r2.header("Authorization"), "the second request must not see the first request's Authorization header"); + assertNull(r2.header("authorization")); + for (String value : r2.headers()) { + assertFalse(value.contains("super-secret-token-A"), "leaked secret found in: " + value); + } + } + + @Test + void secondRequest_doesNotSeeFirstRequestsPathParams() { + Request pooled = new Request(); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/users/123"), null, viewOf("HTTP/1.1"), headersOf()); + Request r1 = Request.forParsed(pooled, line, RequestBody.empty(), null, null); + PathParams.inject(r1, new PathParams(viewOf("/users/123"), new String[]{"id"}, new int[]{7}, new int[]{3})); + assertEquals("123", r1.param("id")); + + RequestLine line2 = new RequestLine(HttpMethod.GET, viewOf("/health"), null, viewOf("HTTP/1.1"), headersOf()); + Request r2 = Request.forParsed(pooled, line2, RequestBody.empty(), null, null); + assertNull(r2.param("id"), "path params from the previous request on this connection must not leak"); + assertNull(r2.getPathParams()); + } + + @Test + void secondRequest_doesNotSeeFirstRequestsCachedPathOrQueryParams() { + Request pooled = new Request(); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/first"), viewOf("token=abc"), viewOf("HTTP/1.1"), headersOf()); + Request r1 = Request.forParsed(pooled, line, RequestBody.empty(), null, null); + assertEquals("/first", r1.path()); + assertEquals("abc", r1.query("token")); + + RequestLine line2 = new RequestLine(HttpMethod.GET, viewOf("/second"), null, viewOf("HTTP/1.1"), headersOf()); + Request r2 = Request.forParsed(pooled, line2, RequestBody.empty(), null, null); + assertEquals("/second", r2.path(), "cachedPath from the previous request must not leak"); + assertNull(r2.query("token"), "query params from the previous request must not leak"); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/RequestRecycleGuardTest.java b/flash/src/test/java/dev/relism/flash/models/RequestRecycleGuardTest.java new file mode 100644 index 0000000..b43533c --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/RequestRecycleGuardTest.java @@ -0,0 +1,128 @@ +package dev.relism.flash.models; + +import dev.relism.flash.http.HttpMethod; +import dev.relism.fpr.core.ByteView; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-22}'s dev-mode use-after-recycle guard. Exercises the poisoning check directly via + * {@code Request.setPoisoningEnabledForTesting} rather than the real {@code Flash.DEV} flag, + * which is a {@code static final boolean} fixed once at JVM startup and cannot be toggled by an + * individual test — see that field's own comment in {@code Request.java}. + */ +class RequestRecycleGuardTest { + + @AfterEach + void restoreProductionDefault() { + // Never leak the test override into other test classes sharing this JVM/fork. + Request.setPoisoningEnabledForTesting(false); + } + + private static ByteView viewOf(String s) { + byte[] bytes = s.getBytes(StandardCharsets.UTF_8); + return new ByteView() { + public int length() { return bytes.length; } + public byte byteAt(int idx) { return bytes[idx]; } + }; + } + + private static Request active() { + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/x"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); + return new Request(line, new byte[0]); + } + + @Test + void poisoningDisabled_recycledRequestStillAccessible() { + Request.setPoisoningEnabledForTesting(false); + Request r = active(); + r.recycle(); + assertDoesNotThrow(r::method, "poisoning disabled (production default) must never throw"); + } + + @Test + void poisoningEnabled_freshRequest_accessibleNormally() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + assertDoesNotThrow(r::path); + assertDoesNotThrow(() -> r.header("Host")); + assertDoesNotThrow(r::method); + } + + @Test + void poisoningEnabled_afterRecycle_methodThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, r::method); + } + + @Test + void poisoningEnabled_afterRecycle_pathThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, r::path); + } + + @Test + void poisoningEnabled_afterRecycle_headerThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, () -> r.header("Host")); + } + + @Test + void poisoningEnabled_afterRecycle_paramThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, () -> r.param("id")); + } + + @Test + void poisoningEnabled_afterRecycle_queryThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, () -> r.query("q")); + } + + @Test + void poisoningEnabled_afterRecycle_remoteAddressThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, r::remoteAddress); + } + + @Test + void poisoningEnabled_afterRecycle_isSecureThrows() { + Request.setPoisoningEnabledForTesting(true); + Request r = active(); + r.recycle(); + assertThrows(IllegalStateException.class, r::isSecure); + } + + @Test + void reusedAfterReset_becomesAccessibleAgain() { + Request.setPoisoningEnabledForTesting(true); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/first"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); + Request r = new Request(line, new byte[0]); + r.recycle(); + assertThrows(IllegalStateException.class, r::path); + + // Simulate the connection loop pulling this pooled instance back out for the next + // request: Request.forParsed's reset() call re-activates it. + RequestLine line2 = new RequestLine(HttpMethod.GET, viewOf("/second"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); + Request reused = Request.forParsed(r, line2, RequestBody.empty(), null, null); + assertSame(r, reused, "forParsed must reposition the same pooled instance, not allocate a new one"); + assertDoesNotThrow(reused::path); + assertEquals("/second", reused.path()); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/RequestTest.java b/flash/src/test/java/dev/relism/flash/models/RequestTest.java index 908079e..354417e 100644 --- a/flash/src/test/java/dev/relism/flash/models/RequestTest.java +++ b/flash/src/test/java/dev/relism/flash/models/RequestTest.java @@ -26,7 +26,7 @@ class RequestTest { @Test void request_creationAndAccessors() { - HeaderMap headers = new HeaderMap(); + Http1HeaderMap headers = new Http1HeaderMap(); RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/path"), viewOf("q=1"), viewOf("HTTP/1.1"), headers); byte[] body = "body".getBytes(StandardCharsets.UTF_8); @@ -43,7 +43,7 @@ class RequestTest { @Test void header_delegatesToRequestLine() { byte[] buffer = "Host: localhost\r\n".getBytes(StandardCharsets.UTF_8); - HeaderMap headers = new HeaderMap(); + Http1HeaderMap headers = new Http1HeaderMap(); headers.reset(buffer, 0, buffer.length); RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), null, viewOf("HTTP/1.1"), headers); Request r = new Request(line, new byte[0]); @@ -57,7 +57,7 @@ class RequestTest { @Test void param_lazyGet() { - RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), null, viewOf("HTTP/1.1"), new HeaderMap()); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); Request r = new Request(line, new byte[0]); assertNull(r.param("id")); @@ -70,7 +70,7 @@ class RequestTest { @Test void query_lazyGet_fromQueryString() { - RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), viewOf("a=1&b=2&b=3"), viewOf("HTTP/1.1"), new HeaderMap()); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), viewOf("a=1&b=2&b=3"), viewOf("HTTP/1.1"), new Http1HeaderMap()); Request r = new Request(line, new byte[0]); assertEquals("1", r.query("a")); @@ -80,7 +80,7 @@ class RequestTest { @Test void query_lazyGet_nullQueryString() { - RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), null, viewOf("HTTP/1.1"), new HeaderMap()); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); Request r = new Request(line, new byte[0]); assertNull(r.query("a")); diff --git a/flash/src/test/java/dev/relism/flash/models/ResponsePoolingTest.java b/flash/src/test/java/dev/relism/flash/models/ResponsePoolingTest.java new file mode 100644 index 0000000..c835788 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/ResponsePoolingTest.java @@ -0,0 +1,75 @@ +package dev.relism.flash.models; + +import dev.relism.flash.http.ContentType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** {@code EX-21}: mirrors {@code RequestPoolingTest} for {@link Response}. */ +class ResponsePoolingTest { + + @Test + void reset_returnsSameInstanceAndClearsPreviousState() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.header("X-Trace", "abc123").status(201).body("first body"); + assertEquals(1, r.getHeaders().size()); + + Response reset = r.reset(200, ContentType.JSON); + assertSame(r, reset, "reset() must reposition the same instance, not allocate a new one"); + assertEquals(200, reset.getStatusCode()); + assertNull(reset.getBody(), "body from the previous cycle must not leak"); + assertTrue(reset.getHeaders().isEmpty(), "headers from the previous cycle must not leak"); + assertArrayEquals(ContentType.JSON.getBytes(), reset.getContentType()); + } + + @Test + void secondCycle_doesNotSeeFirstCyclesCustomHeader() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.header("X-Secret", "leaked-if-broken"); + assertEquals(1, r.getHeaders().size()); + + r.reset(200, ContentType.TEXT_PLAIN); + r.header("X-Public", "fine"); + + assertEquals(1, r.getHeaders().size()); + String only = new String(r.getHeaders().get(0)); + assertTrue(only.contains("X-Public")); + assertFalse(only.contains("X-Secret"), "stale header from the previous cycle leaked: " + only); + } + + @Test + void secondCycle_reusesHeaderRegionAcrossManyHeaders_staysCorrect() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + for (int cycle = 0; cycle < 5; cycle++) { + r.reset(200, ContentType.TEXT_PLAIN); + for (int i = 0; i < 10; i++) { + r.header("X-Cycle" + cycle + "-H" + i, "v" + i); + } + assertEquals(10, r.getHeaders().size(), "cycle " + cycle); + String last = new String(r.getHeaders().get(9)); + assertTrue(last.contains("X-Cycle" + cycle + "-H9: v9"), "cycle " + cycle + ": " + last); + } + } + + @Test + void mixedStructuredAndRawHeaders_preserveInsertionOrder() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.header("A", "1"); + r.header("B-raw: 2\r\n".getBytes()); + r.header("C", "3"); + + var headers = r.getHeaders(); + assertEquals(3, headers.size()); + assertEquals("A: 1\r\n", new String(headers.get(0))); + assertEquals("B-raw: 2\r\n", new String(headers.get(1))); + assertEquals("C: 3\r\n", new String(headers.get(2))); + } + + @Test + void preEncodedHeader_roundTripsThroughGetHeaders() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + PreEncodedHeader h = new PreEncodedHeader("X-Static", "value"); + r.header(h); + assertEquals("X-Static: value\r\n", new String(r.getHeaders().get(0))); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/ResponseRecycleGuardTest.java b/flash/src/test/java/dev/relism/flash/models/ResponseRecycleGuardTest.java new file mode 100644 index 0000000..48fbae4 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/ResponseRecycleGuardTest.java @@ -0,0 +1,58 @@ +package dev.relism.flash.models; + +import dev.relism.flash.http.ContentType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** {@code EX-21}'s dev-mode use-after-recycle guard — mirrors {@code RequestRecycleGuardTest}. */ +class ResponseRecycleGuardTest { + + @AfterEach + void restoreProductionDefault() { + Response.setPoisoningEnabledForTesting(false); + } + + @Test + void poisoningDisabled_recycledResponseStillAccessible() { + Response.setPoisoningEnabledForTesting(false); + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.recycle(); + assertDoesNotThrow(r::getStatusCode); + } + + @Test + void poisoningEnabled_afterRecycle_getStatusCodeThrows() { + Response.setPoisoningEnabledForTesting(true); + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.recycle(); + assertThrows(IllegalStateException.class, r::getStatusCode); + } + + @Test + void poisoningEnabled_afterRecycle_headerThrows() { + Response.setPoisoningEnabledForTesting(true); + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.recycle(); + assertThrows(IllegalStateException.class, () -> r.header("X", "Y")); + } + + @Test + void poisoningEnabled_afterRecycle_bodyThrows() { + Response.setPoisoningEnabledForTesting(true); + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.recycle(); + assertThrows(IllegalStateException.class, () -> r.body("x")); + } + + @Test + void poisoningEnabled_afterReset_accessibleAgain() { + Response.setPoisoningEnabledForTesting(true); + Response r = new Response(200, ContentType.TEXT_PLAIN); + r.recycle(); + assertThrows(IllegalStateException.class, r::getStatusCode); + r.reset(200, ContentType.TEXT_PLAIN); + assertDoesNotThrow(r::getStatusCode); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/ResponseSerializerTest.java b/flash/src/test/java/dev/relism/flash/models/ResponseSerializerTest.java new file mode 100644 index 0000000..bcfc2b0 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/ResponseSerializerTest.java @@ -0,0 +1,73 @@ +package dev.relism.flash.models; + +import dev.relism.flash.http.ContentType; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +class ResponseSerializerTest { + + private static List collect(Response r) { + List fields = new ArrayList<>(); + ResponseSerializer.forEachField(r, (nameBuf, nameOff, nameLen, valueBuf, valueOff, valueLen) -> + fields.add(new String(nameBuf, nameOff, nameLen, StandardCharsets.US_ASCII) + + "=" + new String(valueBuf, valueOff, valueLen, StandardCharsets.US_ASCII))); + return fields; + } + + @Test + void contentTypeFirst_thenCustomHeadersInOrder() { + Response r = new Response(200, ContentType.JSON); + r.header("X-A", "1").header("X-B", "2"); + assertEquals(List.of("Content-Type=application/json", "X-A=1", "X-B=2"), collect(r)); + } + + @Test + void contentTypeNone_isSkipped_notEmptyValue() { + Response r = new Response(200, ContentType.NONE); + r.header("X-Only", "here"); + assertEquals(List.of("X-Only=here"), collect(r)); + } + + @Test + void noHeadersAtAll_onlyContentType() { + Response r = new Response(200, ContentType.TEXT_PLAIN); + assertEquals(List.of("Content-Type=text/plain"), collect(r)); + } + + @Test + void rawPreEncodedHeaderBytes_areExcludedFromEnumeration() { + // header(byte[]) has no recoverable (name, value) structure -- ResponseSerializer must + // skip it (Http1ResponseWriter still renders it, via writeHeaders, just not through this + // protocol-neutral path). + Response r = new Response(200, ContentType.NONE); + r.header("X-Structured", "yes"); + r.header("X-Raw: no-structure\r\n".getBytes()); + assertEquals(List.of("X-Structured=yes"), collect(r)); + } + + @Test + void preEncodedHeaderObject_isIncluded_withStructure() { + Response r = new Response(200, ContentType.NONE); + r.header(new PreEncodedHeader("X-Boot", "constant")); + assertEquals(List.of("X-Boot=constant"), collect(r)); + } + + @Test + void zeroAllocation_byteRangesAreSlicesOfResponsesOwnBuffers_notCopies() { + Response r = new Response(200, ContentType.NONE); + r.header("X-A", "value-a"); + byte[][] captured = new byte[2][]; + ResponseSerializer.forEachField(r, (nameBuf, nameOff, nameLen, valueBuf, valueOff, valueLen) -> { + captured[0] = nameBuf; + captured[1] = valueBuf; + }); + // Both slices must reference the SAME backing array (the response's own header region) -- + // proves no copy was made to hand the field to the consumer. + assertSame(captured[0], captured[1]); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/ResponseTest.java b/flash/src/test/java/dev/relism/flash/models/ResponseTest.java index 33ffb73..8278d5b 100644 --- a/flash/src/test/java/dev/relism/flash/models/ResponseTest.java +++ b/flash/src/test/java/dev/relism/flash/models/ResponseTest.java @@ -134,4 +134,35 @@ class ResponseTest { void getHeaders_emptyWhenNoneAdded() { assertTrue(new Response(200, new byte[0], ContentType.TEXT_PLAIN).getHeaders().isEmpty()); } + + // --- EX-43: response header budget (Phase 6 zero-alloc DoD) --- + + @Test + void header_exceedingMaxCount_throws() { + Response r = new Response(200, new byte[0], ContentType.TEXT_PLAIN); + for (int i = 0; i < dev.relism.flash.http.Http1Limits.MAX_RESPONSE_HEADER_COUNT; i++) { + r.header("X-" + i, "v"); + } + assertThrows(IllegalStateException.class, () -> r.header("one-too-many", "v")); + } + + @Test + void header_exceedingMaxRegionBytes_throws() { + Response r = new Response(200, new byte[0], ContentType.TEXT_PLAIN); + String bigValue = "v".repeat(1024); + assertThrows(IllegalStateException.class, () -> { + // Each call adds ~1024 bytes; comfortably crosses MAX_RESPONSE_HEADER_BYTES well + // before MAX_RESPONSE_HEADER_COUNT would trigger first. + for (int i = 0; i < dev.relism.flash.http.Http1Limits.MAX_RESPONSE_HEADER_COUNT; i++) { + r.header("X-" + i, bigValue); + } + }); + } + + @Test + void header_withinBudget_stillWorksNormally() { + Response r = new Response(200, new byte[0], ContentType.TEXT_PLAIN); + r.header("X-Foo", "bar"); + assertEquals(1, r.getHeaders().size()); + } } diff --git a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java index c621367..3edd7f2 100644 --- a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java @@ -1,7 +1,7 @@ package dev.relism.flash.routing.routers.fastpathrouter; import dev.relism.flash.http.HttpMethod; -import dev.relism.flash.models.HeaderMap; +import dev.relism.flash.models.Http1HeaderMap; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestHandler; import dev.relism.flash.models.RequestLine; @@ -23,7 +23,7 @@ class FastPathRouterImplTest { RequestLine line = new RequestLine( method, pathView, null, new FastPathViews.RequestByteView("HTTP/1.1".getBytes(StandardCharsets.UTF_8), 0, 8), - new HeaderMap() + new Http1HeaderMap() ); return new Request(line, new byte[0]); } diff --git a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsTest.java b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsTest.java index f8b7093..3d9c380 100644 --- a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsTest.java @@ -28,6 +28,31 @@ class FastPathViewsTest { assertThrows(IndexOutOfBoundsException.class, () -> view.byteAt(10)); } + // --- EX-42: reset() repositions the same instance, zero allocation --------- + + @Test + void requestByteView_reset_repositionsSameInstance() { + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(SHARED_BUFFER, 4, 10); + assertEquals("/api/users", view.toString()); + + byte[] other = "PUT /orders/9 HTTP/1.1".getBytes(StandardCharsets.UTF_8); + view.reset(other, 4, 8); + assertEquals(8, view.length()); + assertEquals("/orders/", view.toString()); + } + + @Test + void requestByteView_reset_updatesArrayBackedByteViewAccessors() { + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(SHARED_BUFFER, 0, 3); + byte[] other = "zzHELLOzz".getBytes(StandardCharsets.UTF_8); + view.reset(other, 2, 5); + + assertSame(other, view.array()); + assertEquals(2, view.offset()); + assertEquals(5, view.length()); + assertEquals("HELLO", view.toString()); + } + // --- MethodPathByteView --- @Test diff --git a/flash/src/test/java/dev/relism/flash/template/ByteTemplateTest.java b/flash/src/test/java/dev/relism/flash/template/ByteTemplateTest.java index 1fb3070..3b7ee7d 100644 --- a/flash/src/test/java/dev/relism/flash/template/ByteTemplateTest.java +++ b/flash/src/test/java/dev/relism/flash/template/ByteTemplateTest.java @@ -55,4 +55,38 @@ class ByteTemplateTest { byte[] result = tpl.render("v1", "1", "v2", "2"); assertEquals("A12B", new String(result, StandardCharsets.UTF_8)); } + + // --- EX-28: renderInto(buffer, offset, ...) ------------------------------------ + + @Test + void renderInto_writesAtOffset_andReturnsLength() { + ByteTemplate tpl = new ByteTemplate("Hello {{name}}!"); + byte[] buffer = new byte[64]; + int len = tpl.renderInto(buffer, 5, "name", "World"); + + assertEquals("Hello World!".length(), len); + assertEquals("Hello World!", new String(buffer, 5, len, StandardCharsets.UTF_8)); + } + + @Test + void renderInto_repeatedPlaceholder_fillsEveryOccurrence() { + ByteTemplate tpl = new ByteTemplate("{{var}} == {{var}}"); + byte[] buffer = new byte[32]; + int len = tpl.renderInto(buffer, 0, "var", "test"); + assertEquals("test == test", new String(buffer, 0, len, StandardCharsets.UTF_8)); + } + + @Test + void renderInto_bufferTooSmall_throws() { + ByteTemplate tpl = new ByteTemplate("Hello {{name}}!"); + byte[] buffer = new byte[5]; + assertThrows(IndexOutOfBoundsException.class, () -> tpl.renderInto(buffer, 0, "name", "World")); + } + + @Test + void renderInto_negativeOffset_throws() { + ByteTemplate tpl = new ByteTemplate("Hi {{name}}"); + byte[] buffer = new byte[32]; + assertThrows(IndexOutOfBoundsException.class, () -> tpl.renderInto(buffer, -1, "name", "X")); + } } diff --git a/flash/src/test/java/dev/relism/flash/template/ErrorPagesTest.java b/flash/src/test/java/dev/relism/flash/template/ErrorPagesTest.java index 36f5876..ae52a6c 100644 --- a/flash/src/test/java/dev/relism/flash/template/ErrorPagesTest.java +++ b/flash/src/test/java/dev/relism/flash/template/ErrorPagesTest.java @@ -1,7 +1,7 @@ package dev.relism.flash.template; import dev.relism.flash.http.HttpMethod; -import dev.relism.flash.models.HeaderMap; +import dev.relism.flash.models.Http1HeaderMap; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestLine; import dev.relism.flash.routing.routers.fastpathrouter.FastPathViews; @@ -24,7 +24,7 @@ class ErrorPagesTest { byte[] protoBytes = protocol.getBytes(StandardCharsets.UTF_8); FastPathViews.RequestByteView protoView = new FastPathViews.RequestByteView(protoBytes, 0, protoBytes.length); - RequestLine line = new RequestLine(HttpMethod.GET, pathView, null, protoView, new HeaderMap()); + RequestLine line = new RequestLine(HttpMethod.GET, pathView, null, protoView, new Http1HeaderMap()); return new Request(line, new byte[0]); } diff --git a/flash/src/test/java/dev/relism/flash/transport/ScratchPoolTest.java b/flash/src/test/java/dev/relism/flash/transport/ScratchPoolTest.java index ac3b77a..337d3a9 100644 --- a/flash/src/test/java/dev/relism/flash/transport/ScratchPoolTest.java +++ b/flash/src/test/java/dev/relism/flash/transport/ScratchPoolTest.java @@ -15,8 +15,9 @@ class ScratchPoolTest { ConnectionScratch scratch = pool.acquire(); assertNotNull(scratch); assertNotNull(scratch.sha1); - assertEquals(ConnectionScratch.DECIMAL_BUFFER_SIZE, scratch.decimalBuffer.length); assertEquals(ConnectionScratch.RELAY_BUFFER_SIZE, scratch.relayBuffer.length); + assertNotNull(scratch.responseHead); + assertEquals(0, scratch.responseHead.length()); } @Test diff --git a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java index 21f202a..e1d0b9c 100644 --- a/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java +++ b/flash/src/test/java/dev/relism/flash/websocket/WebSocketSessionTest.java @@ -1,7 +1,7 @@ package dev.relism.flash.websocket; import dev.relism.flash.http.HttpMethod; -import dev.relism.flash.models.HeaderMap; +import dev.relism.flash.models.Http1HeaderMap; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestLine; import dev.relism.fpr.core.ByteView; @@ -25,7 +25,7 @@ class WebSocketSessionTest { @Test void request_returnsWhatWasPassedToConstructor() { - RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/chat"), null, viewOf("HTTP/1.1"), new HeaderMap()); + RequestLine line = new RequestLine(HttpMethod.GET, viewOf("/chat"), null, viewOf("HTTP/1.1"), new Http1HeaderMap()); Request req = new Request(line, new byte[0]); WebSocketSession session = new WebSocketSession( new ByteArrayInputStream(new byte[0]), new ByteArrayOutputStream(), 64, req, false);