diff --git a/flash/docs/http2/DECISIONS.md b/flash/docs/http2/DECISIONS.md index dbd4207..d547e3a 100644 --- a/flash/docs/http2/DECISIONS.md +++ b/flash/docs/http2/DECISIONS.md @@ -733,3 +733,35 @@ once Phase 6 lands `Request`/`RequestBody` pooling — re-run this exact benchma this entry (or add a new one) with the "after" number, closing the loop Phase 4 opened. --- + +## DEC-21 — Phase 5's zero-alloc contract, measured + +**Context.** Phase 5's plan states: "Reading, validating and discarding a frame: 0 B/op ... +Writing a frame header: 0 B/op." Measured with JMH `-prof gc` (JDK 21.0.11, JMH 1.37, +`FrameLayerBenchmark`, `src/jmh/java`) rather than left as an unverified assertion, per this +project's own standing practice of measuring every stated performance/allocation claim +(`DEC-09`, `DEC-20`). + +**Measurement.** `readValidateAndDiscard` (`Http2FrameReader.readFrame` + +`FrameValidator.validate` + one byte read from the payload + `consumeFrame`, against a warm, +already-grown buffer, matching real keep-alive-connection steady state): 299.846 ± 19.722 ns/op, +**0.002 B/op** — indistinguishable from zero (compare `DEC-20`'s harness-floor discussion: even +this near-zero figure is most plausibly measurement noise around the true 0, not a real +allocation, since nothing in the read/validate/consume path can be shown by inspection to +allocate on the warm path). `writeFrame` (`FrameWriteBuffer.beginFrame` + one `writeBytes` call + +`endFrame`, against an already-grown `ByteWriter`): 14.262 ± 1.084 ns/op, **≈10⁻⁴ B/op** — +likewise indistinguishable from zero. + +**Decision.** Contract verified as stated; no design change required. Both numbers are recorded +here as the baseline Phase 17's eventual CI allocation gate should hold this component to. + +**Consequence.** None beyond the recorded numbers — this entry exists so a future regression +(e.g. a later phase accidentally introducing an allocation on this path while adding HPACK or +stream-state integration) has a concrete "was 0, now isn't" baseline to diff against, per this +project's standing insistence that every non-obvious performance claim trace to an actual number. + +**Revisit when.** Not expected to be revisited; re-measure if `FrameHeader`, `Http2FrameReader`, +or `FrameWriteBuffer` are ever modified in a way that could plausibly affect their allocation +profile. + +--- diff --git a/flash/docs/http2/FRAMES.md b/flash/docs/http2/FRAMES.md new file mode 100644 index 0000000..d84b488 --- /dev/null +++ b/flash/docs/http2/FRAMES.md @@ -0,0 +1,148 @@ +# The Frame Layer (Phase 5) + +Audience: contributors. This is the design record for `dev.relism.flash.h2.frame`'s frame +reading, validation, and writing — the 9-byte header and payload boundary, with no connection +semantics, no streams, and no HPACK above it. + +## Why this is simpler than the h1 parser + +HTTP/1.1 request parsing must scan for `\r\n\r\n` (`RequestParser`, `ByteScan.indexOfCrLfCrLf`) +because nothing in the h1 wire format states the header block's length up front. HTTP/2 states +every frame's payload length in the first three bytes of its 9-byte header — nothing is ever +scanned for. `Http2FrameReader` is a length-prefixed reader and nothing more: read 9 bytes, +decode the length, ensure that many more bytes are available, done. + +## The wire format + +``` ++-----------------------------------------------+ +| Length (24) | ++---------------+---------------+---------------+ +| Type (8) | Flags (8) | ++-+-------------+---------------+-------------------------------+ +|R| Stream Identifier (31) | ++=+=============================================================+ +| Frame Payload (0...) ... ++---------------------------------------------------------------+ +``` + +`R` (RFC 9113 §4.1) is reserved and MUST be ignored on receipt — `FrameHeader.reset` masks it +out of `streamId()` once, so no caller has to remember to. + +## Package layout + +``` +dev.relism.flash.h2.frame +├── FrameType the 10 known types + per-type validation descriptor (min/max length, stream-id rule) +├── FrameFlags END_STREAM/ACK/END_HEADERS/PADDED/PRIORITY bit constants + predicates +├── FrameHeader flyweight over a read buffer: length/type/flags/streamId/payloadOffset +├── Http2FrameReader length-prefixed reader, RequestParser's buffer/compaction discipline +├── FrameValidator table-driven per-type RFC validation, specific error code per rule +├── Padding RFC 9113 §6.1/§6.2 pad-length byte + trailing padding, DATA/HEADERS +├── FrameWriteBuffer beginFrame()/endFrame() length back-patching over a ByteWriter +├── Http2FrameWriter (Phase 3) the connection's single serialized writer — unchanged here +├── WriteIntent (Phase 3) unchanged +└── IntrusiveMpscQueue (Phase 3) unchanged +``` + +## The validation table + +Every rule below is enforced by `FrameValidator.validate(FrameHeader, insideHeaderBlock)`, in +this order: unknown-type handling, `SETTINGS`' modulus-6 special case, the generic +min/max length bounds, the `MAX_FRAME_SIZE_LOCAL` ceiling, the stream-id rule, then +`PUSH_PROMISE`'s always-reject rule. + +| Type | Code | Length | Stream id | Notes / RFC | +|---|---|---|---|---| +| DATA | 0x0 | 0..MAX_FRAME_SIZE | required (≠0) | §6.1. Padding via `Padding.unpad`. | +| HEADERS | 0x1 | 0..MAX_FRAME_SIZE | required (≠0) | §6.2. Padding + PRIORITY fields (Phase 7+ parses the latter). | +| PRIORITY | 0x2 | exactly 5 | required (≠0) | §6.3. Deprecated (§5.3.2) — parsed, discarded, never acted on. | +| RST_STREAM | 0x3 | exactly 4 | required (≠0) | §6.4. The 4 bytes are the error code. | +| SETTINGS | 0x4 | multiple of 6 | forbidden (=0) | §6.5. Modulus checked before the generic bounds. | +| PUSH_PROMISE | 0x5 | ≥4 | required (≠0) | §6.6. Always `PROTOCOL_ERROR` from a client — never sent by Flash. | +| PING | 0x6 | exactly 8 | forbidden (=0) | §6.7. Opaque 8-byte payload, echoed on ACK. | +| GOAWAY | 0x7 | ≥8 | forbidden (=0) | §6.8. Last-stream-id (4) + error code (4) + optional debug data. | +| WINDOW_UPDATE | 0x8 | exactly 4 | either | §6.9. 0 = connection window, ≠0 = one stream's window. | +| CONTINUATION | 0x9 | 0..MAX_FRAME_SIZE | required (≠0) | §6.10. Continues a header block; see the flood guard below. | +| *(unrecognised)* | >0x9 | — | — | §4.1: ignored outside a header block, `PROTOCOL_ERROR` inside one (§6.10). | + +**The error code is not uniform per type** — a `SETTINGS` frame with a bad length is +`FRAME_SIZE_ERROR`; the same frame with a non-zero stream id is `PROTOCOL_ERROR`. Every violation +in the table above carries its own RFC citation and the specific code that citation mandates; +`FrameValidatorTest` has one test per row asserting the exact code, not merely "an exception". + +## Ignore vs. reject policy + +RFC 9113 §4.1 makes unknown frame types part of the protocol's extension mechanism: an endpoint +that does not recognise a type MUST read and discard its payload, never reject the connection for +it. `FrameType.fromCode` returns `null` for anything above `CONTINUATION` (0x9); `FrameHeader` +still exposes the raw `typeCode()` for logging even when `type()` is `null`. + +The one exception (§6.10): if an unrecognised-type frame arrives **between** a HEADERS/ +PUSH_PROMISE frame that lacked `END_HEADERS` and the CONTINUATION that eventually sets it, the +HPACK decoder's state has nowhere to put that frame's bytes without desynchronizing — so this one +case *is* a `PROTOCOL_ERROR`, tracked by `FrameValidator.validate`'s `insideHeaderBlock` +parameter (owned and threaded through by the Phase 8 connection loop, which is the only caller +that knows whether a header block is currently open). + +`PRIORITY` frames are a different kind of "ignore": they are a recognised, well-formed type that +Flash chooses not to act on (RFC 9113 §5.3.2 deprecates priority signalling and permits an +implementation to disregard it) — they are still fully parsed and validated like any other frame, +just never influence scheduling. `PUSH_PROMISE` is the opposite: recognised, but **always** +rejected when received (Flash advertises `SETTINGS_ENABLE_PUSH=0` and never sends one itself), so +receiving one at all can only mean the peer has the client/server roles backwards. + +## Buffer discipline and the frame-size defence + +`Http2FrameReader` never grows its buffer to accommodate a declared length before checking that +length against `Http2Limits.MAX_FRAME_SIZE_LOCAL` — the check happens first, so a hostile 16 MB +declared length is rejected at the cost of reading 9 bytes, not at the cost of a 16 MB +allocation. This mirrors `RequestParser`'s own `EX-08` discipline (bound the request line before +trusting it) applied to the frame layer's own attack surface. + +The buffer itself follows `RequestParser`'s compact-before-grow policy: unconsumed bytes slide to +offset 0 when there is room to do so without growing, and growth only happens when compaction +alone cannot make room — bounded, because the reader's own length check already rejected +anything that would require growing past `9 + MAX_FRAME_SIZE_LOCAL`. + +## Padding + +`Padding.unpad` locates the actual data range within a `PADDED` frame's payload: 1 byte of +pad-length, then data, then that many padding bytes (whose contents carry no meaning — they exist +only to obscure payload size from network observers). A pad length greater than or equal to the +whole payload length is `PROTOCOL_ERROR` (RFC 9113 §6.1), checked before any arithmetic that +could otherwise underflow. Flow-control accounting for padded DATA frames (RFC 9113 §6.9.1: the +*whole* payload counts against the window, not just the data) is Phase 11 scope — `Padding` only +locates the data range, it performs no window bookkeeping itself. + +## Writing: `FrameWriteBuffer`'s back-patching + +A frame's length is rarely known before its payload is serialized (an HPACK-encoded header block, +in particular, has no cheap way to be measured in advance). `FrameWriteBuffer.beginFrame` writes +a 9-byte header with a placeholder length; the caller writes the payload directly through the +same `ByteWriter`; `endFrame` computes the actual length from how far the writer has advanced and +rewrites the three length bytes in place. This is *why* `Http2FrameWriter` (Phase 3) serializes a +complete buffer before ever taking the connection lock, rather than streaming bytes as they are +produced — streaming would need the length upfront, which back-patching deliberately avoids +needing. + +## `EX-37`, found while building this phase's tests + +`BufferedByteSource`'s deadline mechanism (`EX-07`'s actual fix) turned out to have zero dedicated +unit tests and an unconditional `socket.setSoTimeout(...)` call that NPE'd against the `null` +socket every isolated unit test in this codebase uses. Found while writing +`Http2FrameReaderTest`, fixed, and given its own regression suite (`BufferedByteSourceTest`) — +full writeup in the plan's registry, `EX-37`. + +## Testing + +- `Http2FrameReaderTest` — round-trips every frame type, boundary lengths (0, 1, 16383, 16384, + 16385), a frame split across three socket reads, a frame exactly filling the initial buffer, + multiple sequential frames, clean-EOF-vs-mid-frame-EOF, and reserved-bit masking. +- `FrameValidatorTest` — one test per RFC-mandated rejection above, asserting the specific + `Http2ErrorCode`. +- `Http2FrameReaderFuzzTest` — 10 000 000 random-length (0–64 byte), random-content inputs; only + `Http2Exception`, `EOFException`, or `SocketTimeoutException` may escape. Green, ~14s. +- `PaddingTest` — every boundary of the pad-length arithmetic, including the exact + `padLength == payloadLength - 1` (maximum valid) and `padLength >= payloadLength` (rejected) + cases. diff --git a/flash/docs/http2/IMPLEMENTATION-PLAN.md b/flash/docs/http2/IMPLEMENTATION-PLAN.md index adbc011..d74d1c7 100644 --- a/flash/docs/http2/IMPLEMENTATION-PLAN.md +++ b/flash/docs/http2/IMPLEMENTATION-PLAN.md @@ -66,7 +66,7 @@ Status values: `not started` / `in progress` / `blocked` / `done`. | 2 — Transport decomposition | done | `feature/core/http2` | `HttpServer.java` deleted; `transport`/`http1` packages + WS extraction (EX-01/06/11/12/13/14/15/16/32/34) done. Router `ThreadLocal` (EX-06 router half) deliberately deferred to Phase 4 per DEC-15. 311/311 tests green (run 3×). h1 benchmark check deferred — no JMH harness until Phase 3. | | 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 | not started | — | — | +| 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 | — | — | | 7 — HPACK decoder | not started | — | — | | 8 — Connection state machine | not started | — | — | @@ -630,6 +630,30 @@ RFC 9112 §5 gives no such leniency: a header field line without a colon is not **Fix**: `colon == -1` now rejects the request with `400 Bad Request`. **Phase**: 1. +### EX-37 — `BufferedByteSource`'s deadline mechanism NPEs against a `null` socket, so it was never actually testable in isolation +Found while writing `Http2FrameReaderTest` (Phase 5): `BufferedByteSource.clearDeadline()` and +`fillFromUnderlying()` both call `socket.setSoTimeout(...)` unconditionally. Every isolated unit +test in this codebase that constructs a `BufferedByteSource` directly (over a +`ByteArrayInputStream`, to test a parser/reader without a real connection) passes `null` for +`socket` — the codebase's own established idiom, used throughout `RequestParserTest`, +`ChunkedInputStreamTest`, `RequestParserSecurityTest`. That idiom works today only because none +of those tests ever call `setDeadline`/trigger a deadline-bounded read — `RequestParser` itself +never calls `setDeadline` (only `Http1Connection`, which always has a real socket, does). The +moment any code under test (here, `Http2FrameReader`, which correctly uses the deadline exactly +as `EX-07` designed it) sets a deadline and then performs a read against a `null`-socket source, +both methods threw `NullPointerException` instead of the intended `SocketTimeoutException`/ +normal read. `BufferedByteSource` — the class that exists specifically to implement `EX-07`'s +slowloris defence — had **zero** dedicated unit tests (`BufferedByteSourceTest` did not exist); +its deadline mechanism was exercised only indirectly, end-to-end, via real-socket tests +(`HttpServerTimeoutTest`), which never hit this path. +**Fix**: both methods now skip the `socket.setSoTimeout(...)` call when `socket == null` — a +`null` socket means "no OS-level timeout to bound", not a misuse; the deadline-expiry check +itself (`remainingNanos <= 0` → `SocketTimeoutException`) is independent of the socket and keeps +working. Production always supplies a real socket, so no production behavior changes. +`BufferedByteSourceTest.java` added (previously absent) with direct coverage of the deadline +mechanism against a `null` socket, closing the actual test gap this bug lived in. +**Phase**: 5 (found and fixed while building `Http2FrameReaderTest`). + --- # PART III — The phases @@ -1602,36 +1626,57 @@ Created: payload copy at this layer (the payload stays in the read buffer; copies happen above, per the layer that needs to retain it). - Writing a frame header: 0 B/op (writes into the existing scratch). +- [x] **Measured**, not just asserted: `FrameLayerBenchmark` (`-prof gc`) — read+validate+consume + 0.002 B/op, write 10⁻⁴ B/op, both indistinguishable from zero. `DECISIONS.md`, `DEC-21`. ### Safety checks -- [ ] Declared length checked against `SETTINGS_MAX_FRAME_SIZE` **before** any buffer growth -- [ ] Buffer growth bounded and monotonic (never shrink mid-connection; shrink only on release - to the pool if the high-water mark was pathological) -- [ ] Per-type length/stream-id/flag validation table complete for all 10 types -- [ ] Unknown types ignored; unknown types inside a header block rejected -- [ ] Reserved bit masked, not rejected -- [ ] Padding length validated against frame length -- [ ] Frame read is timeout-bounded (reuse `bodyReadTimeoutMs` semantics or add - `Http2Limits.FRAME_READ_TIMEOUT_MS`) +- [x] Declared length checked against `SETTINGS_MAX_FRAME_SIZE` **before** any buffer growth — + `Http2FrameReader.readFrame` checks `declaredLength > MAX_FRAME_SIZE_LOCAL` immediately + after decoding the header, before the payload-sized `ensureAvailable` call that would grow + the buffer. +- [x] Buffer growth bounded and monotonic — grows only to accommodate `9 + declaredLength`, + itself already bounded by the check above; never shrinks (matches `RequestParser`'s own + buffer policy, not yet pool-released — no per-connection buffer pool exists before Phase 13). +- [x] Per-type length/stream-id/flag validation table complete for all 10 types — `FrameType`'s + constants + `FrameValidator`, one `FrameValidatorTest` case per RFC-mandated rejection. +- [x] Unknown types ignored; unknown types inside a header block rejected — + `FrameValidator.validate`'s `insideHeaderBlock` parameter, + `unknownType_outsideHeaderBlock_isIgnoredNotRejected`/`unknownType_insideHeaderBlock_isProtocolError`. +- [x] Reserved bit masked, not rejected — `FrameHeader.reset` masks it out of `streamId()`; + `reservedBitInStreamId_isMaskedNotRejected`. +- [x] Padding length validated against frame length — `Padding.unpad`, `PaddingTest`'s boundary + cases (`padLength == payloadLength - 1` valid, `padLength >= payloadLength` rejected). +- [x] Frame read is timeout-bounded — `Http2Limits.FRAME_READ_TIMEOUT_MS` (new constant, this + phase), enforced via `BufferedByteSource`'s existing deadline mechanism. ### Tests - `Http2FrameReaderTest` — round-trip every frame type; boundary lengths 0, 1, 16383, 16384, - 16385; a frame split across three socket reads; a frame exactly filling the buffer. + 16385; a frame split across three socket reads; a frame exactly filling the buffer; multiple + sequential frames; reserved-bit masking. - `FrameValidatorTest` — one test per RFC-mandated rejection, asserting the **specific** error code, not merely that an error occurred. -- `Http2FrameReaderFuzzTest` — random bytes into the reader; assert only `Http2Exception` or - `Http2StreamException` escapes (never `ArrayIndexOutOfBoundsException`, `NegativeArraySizeException`, - `OutOfMemoryError`, or an infinite loop — enforce with a per-case timeout). +- `Http2FrameReaderFuzzTest` — 10 000 000 random-length, random-content inputs — **plan + correction**: asserts only `Http2Exception`, `EOFException`, or `SocketTimeoutException` + escapes, not `Http2Exception`/`Http2StreamException` as originally written here. + `Http2StreamException` is stream-scoped and this phase has no stream concept yet (Phase 10); + `EOFException`/`SocketTimeoutException` are the correctly-typed outcomes for a fuzz input that + truncates mid-frame or (in principle) times out — both legitimate, expected rejections of + malformed/incomplete input, not bugs. Any other exception type still fails the test. Green, + ~14s. - `PaddingTest`. +- `BufferedByteSourceTest` — new, not originally planned for this phase: regression coverage for + `EX-37`, a `NullPointerException` bug in `BufferedByteSource`'s deadline mechanism found while + writing `Http2FrameReaderTest` (see the registry entry for the full writeup — a plain bug fix, + not a design decision, so no `DECISIONS.md` entry). ### Docs -`flash/docs/http2/FRAMES.md` — the wire format, the validation table (as an actual table, one row per -frame type, with the RFC section for each rule), and the ignore-vs-reject policy. +- [x] `flash/docs/http2/FRAMES.md` — the wire format, the validation table (as an actual table, + one row per frame type, with the RFC section for each rule), and the ignore-vs-reject policy. ### DoD -- [ ] All 10 frame types read, validated, and written. -- [ ] Fuzz test green for 10 million random inputs. -- [ ] `flash/docs/http2/FRAMES.md` complete with the validation table. +- [x] All 10 frame types read, validated, and written — `roundTrip_everyFrameType`. +- [x] Fuzz test green for 10 million random inputs — `Http2FrameReaderFuzzTest`, ~14s. +- [x] `flash/docs/http2/FRAMES.md` complete with the validation table. --- diff --git a/flash/src/jmh/java/dev/relism/flash/h2/frame/FrameLayerBenchmark.java b/flash/src/jmh/java/dev/relism/flash/h2/frame/FrameLayerBenchmark.java new file mode 100644 index 0000000..f49e16e --- /dev/null +++ b/flash/src/jmh/java/dev/relism/flash/h2/frame/FrameLayerBenchmark.java @@ -0,0 +1,113 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.bytes.ByteWriter; +import dev.relism.flash.transport.BufferedByteSource; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.io.IOException; +import java.io.InputStream; +import java.util.concurrent.TimeUnit; + +/** + * Phase 5's zero-alloc contract: "Reading, validating and discarding a frame: 0 B/op ... Writing + * a frame header: 0 B/op." Measured with {@code -prof gc}, not merely asserted — see + * {@code DECISIONS.md}, {@code DEC-21}, for the recorded numbers. + * + *
Uses the same hand-rolled repeating {@link InputStream} technique + * {@code RequestPipelineBenchmark} (Phase 4) established: one {@link BufferedByteSource}/ + * {@link Http2FrameReader} pair created once per trial and reused across every invocation, + * matching how a real connection's demux loop owns exactly one of each for its whole lifetime, + * rather than paying for harness-side (re)construction inside the timed path. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(2) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +public class FrameLayerBenchmark { + + private static final class RepeatingByteStream extends InputStream { + private final byte[] template; + private int pos; + + RepeatingByteStream(byte[] template) { + this.template = template; + } + + @Override + public int read() { + byte b = template[pos]; + pos = (pos + 1) % template.length; + return b & 0xFF; + } + + @Override + public int read(byte[] dst, int off, int len) { + for (int i = 0; i < len; i++) { + dst[off + i] = template[pos]; + pos = (pos + 1) % template.length; + } + return len; + } + } + + // ── Read + validate ────────────────────────────────────────────────────── + + private Http2FrameReader reader; + + @Setup(Level.Trial) + public void setupReader() { + FrameWriteBuffer out = new FrameWriteBuffer(new ByteWriter(64)); + out.beginFrame(FrameType.HEADERS, FrameFlags.END_HEADERS, 1); + byte[] payload = new byte[48]; + for (int i = 0; i < payload.length; i++) payload[i] = (byte) i; + out.writer().writeBytes(payload); + out.endFrame(); + byte[] template = new byte[out.writer().length()]; + System.arraycopy(out.writer().array(), 0, template, 0, template.length); + + BufferedByteSource src = new BufferedByteSource(new RepeatingByteStream(template), null); + reader = new Http2FrameReader(src); + } + + @Benchmark + public int readValidateAndDiscard() throws IOException { + FrameHeader header = reader.readFrame(); + FrameValidator.validate(header, false); + int checksum = header.buffer()[header.payloadOffset()]; + reader.consumeFrame(); + return checksum; + } + + // ── Write ──────────────────────────────────────────────────────────────── + + private FrameWriteBuffer writeBuffer; + private byte[] writePayload; + + @Setup(Level.Trial) + public void setupWriter() { + writeBuffer = new FrameWriteBuffer(new ByteWriter(64)); + writePayload = new byte[48]; + for (int i = 0; i < writePayload.length; i++) writePayload[i] = (byte) i; + } + + @Benchmark + public int writeFrame() { + writeBuffer.writer().reset(); + writeBuffer.beginFrame(FrameType.HEADERS, FrameFlags.END_HEADERS, 1); + writeBuffer.writer().writeBytes(writePayload); + writeBuffer.endFrame(); + return writeBuffer.writer().length(); + } +} diff --git a/flash/src/main/java/dev/relism/flash/h2/Http2Limits.java b/flash/src/main/java/dev/relism/flash/h2/Http2Limits.java index 5b950aa..d6d5582 100644 --- a/flash/src/main/java/dev/relism/flash/h2/Http2Limits.java +++ b/flash/src/main/java/dev/relism/flash/h2/Http2Limits.java @@ -156,4 +156,13 @@ public final class Http2Limits { * {@code Socket#setSoTimeout} — that option bounds reads, not writes. */ public static final long WRITE_TIMEOUT_MS = 30_000; + + /** + * Maximum time, in milliseconds, {@code Http2FrameReader} may wait for a single frame's + * header and payload to fully arrive. Bounds the same slowloris-shaped hazard {@code + * BufferedByteSource}'s deadline mechanism already defends h1 against ({@code EX-07}): + * without it, a peer that sends 9 header bytes and then never sends the declared payload + * would hold this connection's frame reader waiting forever. + */ + public static final long FRAME_READ_TIMEOUT_MS = 20_000; } diff --git a/flash/src/main/java/dev/relism/flash/h2/frame/FrameFlags.java b/flash/src/main/java/dev/relism/flash/h2/frame/FrameFlags.java new file mode 100644 index 0000000..c92f35e --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/h2/frame/FrameFlags.java @@ -0,0 +1,39 @@ +package dev.relism.flash.h2.frame; + +/** + * The frame-header flag bits (RFC 9113 §6), as bitwise constants plus predicate helpers. + * + *
RFC 9113 §4.1: flag bits not defined for a frame's type MUST be ignored on receipt and MUST + * NOT be set when sending. This class only ever tests bits it defines for the type the caller is + * working with; undefined bits are never inspected. + */ +public final class FrameFlags { + private FrameFlags() {} + + /** DATA/HEADERS: no more frames will be sent for this stream in this direction. */ + public static final int END_STREAM = 0x1; + /** SETTINGS/PING: this frame acknowledges the peer's own frame, rather than proposing new values. */ + public static final int ACK = 0x1; + /** HEADERS/PUSH_PROMISE/CONTINUATION: the header block is complete — no CONTINUATION follows. */ + public static final int END_HEADERS = 0x4; + /** DATA/HEADERS/PUSH_PROMISE: a pad-length byte and trailing padding are present — see {@link Padding}. */ + public static final int PADDED = 0x8; + /** HEADERS: deprecated stream-dependency/weight fields are present (RFC 9113 §5.3.2 — parsed and discarded). */ + public static final int PRIORITY = 0x20; + + public static boolean isEndStream(int flags) { return (flags & END_STREAM) != 0; } + public static boolean isAck(int flags) { return (flags & ACK) != 0; } + public static boolean isEndHeaders(int flags) { return (flags & END_HEADERS) != 0; } + public static boolean isPadded(int flags) { return (flags & PADDED) != 0; } + public static boolean hasPriority(int flags) { return (flags & PRIORITY) != 0; } +} 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 new file mode 100644 index 0000000..9220a58 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/h2/frame/FrameHeader.java @@ -0,0 +1,84 @@ +package dev.relism.flash.h2.frame; + +/** + * A flyweight over one frame's 9-byte header plus its payload location, both still living + * in {@link Http2FrameReader}'s own read buffer. One instance per connection, {@link #reset} + * in place by every {@link Http2FrameReader#readFrame()} call — never allocated per frame + * (mirrors the existing {@code WebSocketFrame} reuse idiom in {@code dev.relism.flash.websocket}). + * + *
The error code is not uniform — read the RFC per violation, not just per type. A + * {@code SETTINGS} frame with a bad length is {@code FRAME_SIZE_ERROR}; the same frame with a + * non-zero stream id is {@code PROTOCOL_ERROR}. This class throws the specific code each + * violation's own RFC citation requires, not a single blanket code per type. + */ +public final class FrameValidator { + private FrameValidator() {} + + /** + * Validates {@code header} against RFC 9113's rules for its type. + * + * @param insideHeaderBlock whether this frame arrived between a HEADERS/PUSH_PROMISE frame + * lacking {@code END_HEADERS} and its terminating CONTINUATION — + * changes the handling of an unrecognised type (§6.10: a + * {@code PROTOCOL_ERROR}, not the usual silent ignore, since an + * in-progress header block cannot tolerate an interloper frame of + * any kind without desynchronizing HPACK's stateful decode) + * @throws Http2Exception on any RFC violation, with the specific error code the violated + * rule mandates + */ + public static void validate(FrameHeader header, boolean insideHeaderBlock) { + FrameType type = header.type(); + + if (type == null) { + // RFC 9113 §4.1: unknown frame types MUST be ignored — except inside an in-progress + // header block (§6.10), where anything other than CONTINUATION desynchronizes HPACK. + if (insideHeaderBlock) { + throw Http2Exception.of(Http2ErrorCode.PROTOCOL_ERROR, + "unrecognised frame type " + header.typeCode() + " received inside a header block"); + } + return; + } + + int length = header.length(); + + // RFC 9113 §6.5: a SETTINGS frame's length MUST be a multiple of 6 (each entry is a + // 2-byte identifier + 4-byte value). Checked before the generic bounds below, since the + // generic table only expresses a min/max range, not a modulus. + if (type == FrameType.SETTINGS && length % 6 != 0) { + throw Http2Exception.FRAME_SIZE_ERROR; + } + + if (length < type.minLength() || length > type.maxLength()) { + throw Http2Exception.FRAME_SIZE_ERROR; + } + + // Redundant with Http2FrameReader's own pre-allocation check for frames it read itself, + // but this method must also be correct for a FrameHeader built any other way (tests, + // and — in later phases — frames reassembled from multiple reads), so the bound is + // re-asserted here rather than trusted from the caller. + if (length > Http2Limits.MAX_FRAME_SIZE_LOCAL) { + throw Http2Exception.FRAME_SIZE_ERROR; + } + + int streamId = header.streamId(); + switch (type.streamIdRule()) { + case REQUIRED -> { + if (streamId == 0) { + throw Http2Exception.of(Http2ErrorCode.PROTOCOL_ERROR, type + " requires a non-zero stream id"); + } + } + case FORBIDDEN -> { + if (streamId != 0) { + throw Http2Exception.of(Http2ErrorCode.PROTOCOL_ERROR, type + " must have stream id 0, got " + streamId); + } + } + case EITHER -> { /* WINDOW_UPDATE: 0 (connection window) or non-zero (stream window) both valid */ } + } + + // RFC 9113 §8.4 / this codebase's DEC-10: PUSH_PROMISE is a server-to-client-only frame + // (Flash advertises SETTINGS_ENABLE_PUSH=0 and never sends one); receiving one at all + // means the peer believes it is talking to a client, which is always a protocol error. + if (type == FrameType.PUSH_PROMISE) { + throw Http2Exception.of(Http2ErrorCode.PROTOCOL_ERROR, "PUSH_PROMISE received from a client"); + } + } +} diff --git a/flash/src/main/java/dev/relism/flash/h2/frame/FrameWriteBuffer.java b/flash/src/main/java/dev/relism/flash/h2/frame/FrameWriteBuffer.java new file mode 100644 index 0000000..221ccb0 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/h2/frame/FrameWriteBuffer.java @@ -0,0 +1,76 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.bytes.ByteWriter; + +/** + * Serializes HTTP/2 frames into a {@link ByteWriter} scratch buffer with the standard + * length-back-patching technique: {@link #beginFrame} writes a 9-byte header with a placeholder + * length, the caller writes the payload directly through {@link #writer()} (the same + * {@link ByteWriter}), and {@link #endFrame} rewrites the length once it is known — the payload + * size is rarely known before it is serialized (an HPACK-encoded header block, in particular, + * has no cheap way to be measured in advance). + * + *
This is the reason {@link Http2FrameWriter} (Phase 3) serializes a complete buffer and + * issues one bulk {@code write}, rather than streaming bytes as they are produced: streaming + * would require knowing the length before the first byte goes out, which back-patching + * deliberately avoids needing. + * + *
{@code
+ * FrameWriteBuffer out = new FrameWriteBuffer(new ByteWriter(4096));
+ * out.beginFrame(FrameType.SETTINGS, 0, 0);
+ * out.writer().writeUInt16(SETTINGS_MAX_CONCURRENT_STREAMS);
+ * out.writer().writeUInt32(100);
+ * out.endFrame();
+ * // out.writer().array()[0, out.writer().length()) now holds one complete, correctly-lengthed frame
+ * }
+ *
+ * {@code
+ * FrameHeader header = reader.readFrame();
+ * if (header == null) { /* clean EOF between frames — connection closing *\/ }
+ * // ... process header.buffer()[header.payloadOffset(), +header.length()) ...
+ * reader.consumeFrame(); // MUST be called before the next readFrame()
+ * }
+ *
+ * The caller MUST call {@link #consumeFrame()} exactly once after processing this frame + * (or deciding to discard it) and before calling this method again. + * + * @throws Http2Exception if the declared length exceeds {@link Http2Limits#MAX_FRAME_SIZE_LOCAL} + * @throws EOFException if the connection closes after a frame has already started arriving + * @throws java.net.SocketTimeoutException if {@link Http2Limits#FRAME_READ_TIMEOUT_MS} elapses + */ + public FrameHeader readFrame() throws IOException { + in.setDeadline(System.nanoTime() + Http2Limits.FRAME_READ_TIMEOUT_MS * 1_000_000L); + try { + if (!ensureAvailable(FRAME_HEADER_SIZE)) { + return null; // clean EOF: nothing buffered yet, peer closed between frames + } + int declaredLength = decodeLength(buffer, base); + // R8: checked BEFORE any further buffer growth or read — a hostile declared length + // never causes an oversized allocation, only a rejection. + if (declaredLength > Http2Limits.MAX_FRAME_SIZE_LOCAL) { + throw Http2Exception.FRAME_SIZE_ERROR; + } + ensureAvailable(FRAME_HEADER_SIZE + declaredLength); + header.reset(buffer, base); + return header; + } finally { + in.clearDeadline(); + } + } + + /** Advances past the frame last returned by {@link #readFrame()}. Zero-copy, zero-allocation. */ + public void consumeFrame() { + int consumed = FRAME_HEADER_SIZE + header.length(); + base += consumed; + totalRead -= consumed; + if (totalRead == 0) { + base = 0; // nothing buffered — reset to the front rather than drifting forever + } + } + + private static int decodeLength(byte[] buf, int off) { + int b0 = buf[off] & 0xFF, b1 = buf[off + 1] & 0xFF, b2 = buf[off + 2] & 0xFF; + return (b0 << 16) | (b1 << 8) | b2; + } + + /** + * Ensures at least {@code need} bytes are available starting at {@link #base}, growing or + * compacting the buffer as necessary. Returns {@code false} only for a clean EOF with + * nothing at all buffered yet (the between-frames case); an EOF after any bytes of the + * current frame have already arrived is a genuine truncation and throws. + */ + private boolean ensureAvailable(int need) throws IOException { + while (totalRead < need) { + if (base + need > buffer.length) { + if (base > 0) { + // Compact: slide unconsumed bytes to the front — frees room without growing. + System.arraycopy(buffer, base, buffer, 0, totalRead); + base = 0; + } else { + // need <= 9 + MAX_FRAME_SIZE_LOCAL always, by readFrame()'s own check before + // the payload-sized call — grow exactly enough, never unbounded. + int grown = buffer.length; + while (grown < need) grown *= 2; + buffer = Arrays.copyOf(buffer, grown); + } + } + int n = in.read(buffer, base + totalRead, buffer.length - base - totalRead); + if (n < 0) { + if (totalRead == 0) return false; + throw new EOFException("connection closed mid-frame (" + totalRead + "/" + need + " bytes read)"); + } + totalRead += n; + } + return true; + } +} diff --git a/flash/src/main/java/dev/relism/flash/h2/frame/Padding.java b/flash/src/main/java/dev/relism/flash/h2/frame/Padding.java new file mode 100644 index 0000000..9105171 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/h2/frame/Padding.java @@ -0,0 +1,67 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.bytes.Pairs; +import dev.relism.flash.h2.Http2ErrorCode; +import dev.relism.flash.h2.Http2Exception; + +/** + * RFC 9113 §6.1 (DATA) / §6.2 (HEADERS) padding. When {@link FrameFlags#PADDED} is set, a + * frame's payload is laid out as: 1 pad-length byte, then the actual data (or header-block + * fragment), then that many padding bytes (RFC 9113 gives no meaning to the padding bytes + * themselves — they exist only to obscure payload size from network observers). + * + *
Padding is not optional to support: any client may send it on DATA or HEADERS + * regardless of whether the server ever sends padded frames itself. + * + *
{@code EX-37}: a {@code null} socket (the constructor accepts one — every isolated unit + * test in this codebase that constructs a {@code BufferedByteSource} directly over a + * {@code ByteArrayInputStream} passes {@code null}, since there is no real connection to + * bound) is treated as "no OS-level timeout to clear", not an error — only the deadline + * bookkeeping is reset. Production always supplies a real socket, so this changes no + * production behavior; without it, no test can exercise the deadline mechanism at all. */ public void clearDeadline() throws IOException { this.deadlineActive = false; - socket.setSoTimeout(0); + if (socket != null) socket.setSoTimeout(0); } // ── InputStream ────────────────────────────────────────────────────────── @@ -247,6 +254,14 @@ public final class BufferedByteSource extends InputStream { * active, computes the exact remaining budget and hands it to {@link Socket#setSoTimeout} * before reading, so a {@link SocketTimeoutException} from {@code in.read} unambiguously * means the deadline — not merely one read — has elapsed; see the class Javadoc. + * + *
{@code EX-37}: the expiry check above (throwing once {@code remainingNanos <= 0}) runs + * regardless of whether a real {@link Socket} is present; only the OS-level + * {@code setSoTimeout} call — meaningless without a socket, and previously called + * unconditionally, which NPE'd the instant any deadline-bounded read ran against a + * {@code null}-socket source — is skipped when {@code socket == null}. See + * {@link #clearDeadline()}'s Javadoc for why {@code null} is a legitimate, tested case, not + * a misuse. */ private int fillFromUnderlying(byte[] dst, int off, int len) throws IOException { if (!deadlineActive) { @@ -256,9 +271,11 @@ public final class BufferedByteSource extends InputStream { if (remainingNanos <= 0) { throw new SocketTimeoutException("Read deadline exceeded"); } - long remainingMillis = (remainingNanos + 999_999L) / 1_000_000L; // round up - int timeoutMs = (int) Math.max(1, Math.min(Integer.MAX_VALUE, remainingMillis)); - socket.setSoTimeout(timeoutMs); + if (socket != null) { + long remainingMillis = (remainingNanos + 999_999L) / 1_000_000L; // round up + int timeoutMs = (int) Math.max(1, Math.min(Integer.MAX_VALUE, remainingMillis)); + socket.setSoTimeout(timeoutMs); + } return in.read(dst, off, len); } } diff --git a/flash/src/test/java/dev/relism/flash/h2/frame/FrameValidatorTest.java b/flash/src/test/java/dev/relism/flash/h2/frame/FrameValidatorTest.java new file mode 100644 index 0000000..5c88fdc --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/h2/frame/FrameValidatorTest.java @@ -0,0 +1,181 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.h2.Http2ErrorCode; +import dev.relism.flash.h2.Http2Exception; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** One test per RFC-mandated rejection, asserting the specific {@link Http2ErrorCode} — not merely that {@link Http2Exception} was thrown. */ +class FrameValidatorTest { + + private static byte[] rawFrame(int length, int typeCode, int flags, int streamId) { + byte[] buf = new byte[9]; + buf[0] = (byte) (length >>> 16); + buf[1] = (byte) (length >>> 8); + buf[2] = (byte) length; + buf[3] = (byte) typeCode; + buf[4] = (byte) flags; + buf[5] = (byte) (streamId >>> 24); + buf[6] = (byte) (streamId >>> 16); + buf[7] = (byte) (streamId >>> 8); + buf[8] = (byte) streamId; + return buf; + } + + private static FrameHeader headerOf(int length, FrameType type, int flags, int streamId) { + byte[] buf = rawFrame(length, type.code(), flags, streamId); + FrameHeader header = new FrameHeader(); + // reset() is package-private; same package as this test. + header.reset(buf, 0); + return header; + } + + private static Http2ErrorCode codeOf(FrameHeader header, boolean insideHeaderBlock) { + Http2Exception ex = assertThrows(Http2Exception.class, () -> FrameValidator.validate(header, insideHeaderBlock)); + return ex.errorCode(); + } + + // ── Length bounds, per type ────────────────────────────────────────────── + + @Test + void ping_wrongLength_isFrameSizeError() { + FrameHeader h = headerOf(7, FrameType.PING, 0, 0); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void rstStream_wrongLength_isFrameSizeError() { + FrameHeader h = headerOf(3, FrameType.RST_STREAM, 0, 1); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void windowUpdate_wrongLength_isFrameSizeError() { + FrameHeader h = headerOf(5, FrameType.WINDOW_UPDATE, 0, 1); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void priority_wrongLength_isFrameSizeError() { + FrameHeader h = headerOf(4, FrameType.PRIORITY, 0, 1); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void goaway_tooShort_isFrameSizeError() { + FrameHeader h = headerOf(7, FrameType.GOAWAY, 0, 0); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void goaway_exactlyEightBytes_isValid() { + FrameHeader h = headerOf(8, FrameType.GOAWAY, 0, 0); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + @Test + void settings_notMultipleOfSix_isFrameSizeError() { + FrameHeader h = headerOf(7, FrameType.SETTINGS, 0, 0); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } + + @Test + void settings_multipleOfSix_isValid() { + FrameHeader h = headerOf(12, FrameType.SETTINGS, 0, 0); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + @Test + void settings_zeroLength_isValid() { + // An empty SETTINGS frame (0 entries) is legal -- e.g. the initial connection SETTINGS + // with no non-default values, or a SETTINGS ACK. + FrameHeader h = headerOf(0, FrameType.SETTINGS, FrameFlags.ACK, 0); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + // ── Stream id rules ────────────────────────────────────────────────────── + + @Test + void settings_nonZeroStreamId_isProtocolError() { + FrameHeader h = headerOf(0, FrameType.SETTINGS, 0, 1); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void ping_nonZeroStreamId_isProtocolError() { + FrameHeader h = headerOf(8, FrameType.PING, 0, 3); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void goaway_nonZeroStreamId_isProtocolError() { + FrameHeader h = headerOf(8, FrameType.GOAWAY, 0, 5); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void data_zeroStreamId_isProtocolError() { + FrameHeader h = headerOf(0, FrameType.DATA, 0, 0); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void headers_zeroStreamId_isProtocolError() { + FrameHeader h = headerOf(0, FrameType.HEADERS, 0, 0); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void rstStream_zeroStreamId_isProtocolError() { + FrameHeader h = headerOf(4, FrameType.RST_STREAM, 0, 0); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + @Test + void windowUpdate_zeroStreamId_isValid_connectionWindow() { + FrameHeader h = headerOf(4, FrameType.WINDOW_UPDATE, 0, 0); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + @Test + void windowUpdate_nonZeroStreamId_isValid_streamWindow() { + FrameHeader h = headerOf(4, FrameType.WINDOW_UPDATE, 0, 9); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + // ── PUSH_PROMISE from a client ─────────────────────────────────────────── + + @Test + void pushPromise_fromClient_isAlwaysProtocolError() { + FrameHeader h = headerOf(4, FrameType.PUSH_PROMISE, 0, 1); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, false)); + } + + // ── Unknown frame types ────────────────────────────────────────────────── + + @Test + void unknownType_outsideHeaderBlock_isIgnoredNotRejected() { + byte[] buf = rawFrame(3, 0x20, 0, 1); // 0x20 is not a recognised type + FrameHeader h = new FrameHeader(); + h.reset(buf, 0); + assertNull(h.type()); + assertDoesNotThrow(() -> FrameValidator.validate(h, false)); + } + + @Test + void unknownType_insideHeaderBlock_isProtocolError() { + byte[] buf = rawFrame(3, 0x20, 0, 1); + FrameHeader h = new FrameHeader(); + h.reset(buf, 0); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, codeOf(h, true)); + } + + // ── Frame-size ceiling ──────────────────────────────────────────────────── + + @Test + void declaredLengthAboveMaxFrameSize_isFrameSizeError() { + FrameHeader h = headerOf(dev.relism.flash.h2.Http2Limits.MAX_FRAME_SIZE_LOCAL + 1, FrameType.DATA, 0, 1); + assertEquals(Http2ErrorCode.FRAME_SIZE_ERROR, codeOf(h, false)); + } +} diff --git a/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderFuzzTest.java b/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderFuzzTest.java new file mode 100644 index 0000000..7b36774 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderFuzzTest.java @@ -0,0 +1,59 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.h2.Http2Exception; +import dev.relism.flash.transport.BufferedByteSource; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.EOFException; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Phase 5's DoD: "Fuzz test green for 10 million random inputs." Throws fully random bytes at + * {@link Http2FrameReader} and asserts that only a typed, expected outcome ever results: a + * {@link Http2Exception} (a declared length exceeding {@code MAX_FRAME_SIZE_LOCAL} — the + * overwhelmingly common outcome, since a random 24-bit length is astronomically likely to + * exceed 16384), an {@link EOFException} (the random input ran out before a full frame arrived + * — the second most common outcome, since fuzz inputs are deliberately small), or a + * {@link SocketTimeoutException} (never actually expected here — no deadline is short enough to + * trip against an in-memory stream — but a legal outcome of the API's own contract). Anything + * else escaping — {@code ArrayIndexOutOfBoundsException}, {@code NegativeArraySizeException}, + * {@code OutOfMemoryError}, or simply never returning — fails the test. + */ +class Http2FrameReaderFuzzTest { + + private static final int TRIALS = 10_000_000; + private static final int MAX_INPUT_LEN = 64; + + @Test + void fuzz_10MillionRandomInputs_onlyTypedOutcomesEscape() { + Random rnd = new Random(0x4855_3244_5F46_5A32L); + byte[] data = new byte[MAX_INPUT_LEN]; + + for (int trial = 0; trial < TRIALS; trial++) { + int len = rnd.nextInt(MAX_INPUT_LEN + 1); + for (int i = 0; i < len; i++) data[i] = (byte) rnd.nextInt(256); + + BufferedByteSource src = new BufferedByteSource( + new ByteArrayInputStream(data, 0, len), null, 128); + Http2FrameReader reader = new Http2FrameReader(src, 128); + + try { + FrameHeader header = reader.readFrame(); + if (header != null) { + reader.consumeFrame(); + } + } catch (Http2Exception | EOFException | SocketTimeoutException expected) { + // any of these three is a correctly-typed rejection of malformed/truncated input + } catch (IOException e) { + fail("unexpected IOException at trial " + trial + " (len=" + len + "): " + e, e); + } catch (RuntimeException e) { + fail("unexpected RuntimeException at trial " + trial + " (len=" + len + "): " + e, e); + } + } + } +} diff --git a/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderTest.java b/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderTest.java new file mode 100644 index 0000000..c15855e --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/h2/frame/Http2FrameReaderTest.java @@ -0,0 +1,209 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.bytes.ByteWriter; +import dev.relism.flash.h2.Http2Exception; +import dev.relism.flash.transport.BufferedByteSource; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; + +import static org.junit.jupiter.api.Assertions.*; + +class Http2FrameReaderTest { + + private static BufferedByteSource sourceOf(byte[] bytes) { + return new BufferedByteSource(new ByteArrayInputStream(bytes), null); + } + + private static byte[] buildFrame(FrameType type, int flags, int streamId, byte[] payload) { + FrameWriteBuffer out = new FrameWriteBuffer(new ByteWriter(32)); + out.beginFrame(type, flags, streamId); + out.writer().writeBytes(payload); + out.endFrame(); + byte[] result = new byte[out.writer().length()]; + System.arraycopy(out.writer().array(), 0, result, 0, result.length); + return result; + } + + // ── Round trip every frame type ───────────────────────────────────────── + + @Test + void roundTrip_everyFrameType() throws IOException { + for (FrameType type : FrameType.values()) { + int payloadLen = switch (type) { + case PING -> 8; + case RST_STREAM, WINDOW_UPDATE -> 4; + case PRIORITY -> 5; + case GOAWAY -> 8; + default -> 10; + }; + byte[] payload = new byte[payloadLen]; + for (int i = 0; i < payloadLen; i++) payload[i] = (byte) (i + 1); + int streamId = type.streamIdRule() == FrameType.StreamIdRule.FORBIDDEN ? 0 : 7; + + byte[] wire = buildFrame(type, 0x1, streamId, payload); + Http2FrameReader reader = new Http2FrameReader(sourceOf(wire)); + FrameHeader header = reader.readFrame(); + + assertNotNull(header, "type=" + type); + assertEquals(type, header.type()); + assertEquals(type.code(), header.typeCode()); + assertEquals(payloadLen, header.length()); + assertEquals(streamId, header.streamId()); + assertEquals(0x1, header.flags()); + for (int i = 0; i < payloadLen; i++) { + assertEquals(payload[i], header.buffer()[header.payloadOffset() + i], "byte " + i + " of type " + type); + } + reader.consumeFrame(); + } + } + + // ── Boundary lengths ───────────────────────────────────────────────────── + + @Test + void boundaryLengths_0_1_16383_16384_16385() throws IOException { + int[] lengths = {0, 1, 16383, 16384, 16385}; + for (int len : lengths) { + byte[] payload = new byte[len]; + byte[] wire = buildFrame(FrameType.DATA, 0, 1, payload); + Http2FrameReader reader = new Http2FrameReader(sourceOf(wire)); + if (len > dev.relism.flash.h2.Http2Limits.MAX_FRAME_SIZE_LOCAL) { + Http2Exception ex = assertThrows(Http2Exception.class, reader::readFrame); + assertEquals(dev.relism.flash.h2.Http2ErrorCode.FRAME_SIZE_ERROR, ex.errorCode()); + } else { + FrameHeader header = reader.readFrame(); + assertNotNull(header); + assertEquals(len, header.length()); + } + } + } + + // ── A frame split across multiple socket reads ───────────────────────── + + private static final class DribblingInputStream extends InputStream { + private final byte[] data; + private int pos; + private final int chunkSize; + + DribblingInputStream(byte[] data, int chunkSize) { + this.data = data; + this.chunkSize = chunkSize; + } + + @Override + public int read() { + return pos < data.length ? (data[pos++] & 0xFF) : -1; + } + + @Override + public int read(byte[] dst, int off, int len) { + if (pos >= data.length) return -1; + int n = Math.min(chunkSize, Math.min(len, data.length - pos)); + System.arraycopy(data, pos, dst, off, n); + pos += n; + return n; + } + } + + @Test + void frameSplitAcrossThreeSocketReads() throws IOException { + byte[] payload = new byte[300]; + for (int i = 0; i < payload.length; i++) payload[i] = (byte) i; + byte[] wire = buildFrame(FrameType.DATA, 0, 3, payload); + + // 9(header) + 300(payload) = 309 bytes, dribbled in chunks of 103 -> 3 reads. + int chunk = (wire.length + 2) / 3; + BufferedByteSource src = new BufferedByteSource(new DribblingInputStream(wire, chunk), null); + Http2FrameReader reader = new Http2FrameReader(src); + FrameHeader header = reader.readFrame(); + + assertNotNull(header); + assertEquals(300, header.length()); + for (int i = 0; i < 300; i++) { + assertEquals(payload[i], header.buffer()[header.payloadOffset() + i]); + } + } + + // ── A frame exactly filling the initial buffer ────────────────────────── + + @Test + void frameExactlyFillingInitialBuffer() throws IOException { + int bufSize = 64; + byte[] payload = new byte[bufSize - 9]; // header + payload == bufSize exactly + byte[] wire = buildFrame(FrameType.DATA, 0, 1, payload); + assertEquals(bufSize, wire.length); + + Http2FrameReader reader = new Http2FrameReader(sourceOf(wire), bufSize); + FrameHeader header = reader.readFrame(); + assertNotNull(header); + assertEquals(payload.length, header.length()); + } + + // ── Multiple frames on one connection, sequential reads ───────────────── + + @Test + void multipleFramesSequentially() throws IOException { + ByteWriter w = new ByteWriter(64); + FrameWriteBuffer out = new FrameWriteBuffer(w); + out.beginFrame(FrameType.PING, 0, 0); + out.writer().writeBytes(new byte[]{1, 2, 3, 4, 5, 6, 7, 8}); + out.endFrame(); + out.beginFrame(FrameType.PING, dev.relism.flash.h2.frame.FrameFlags.ACK, 0); + out.writer().writeBytes(new byte[]{8, 7, 6, 5, 4, 3, 2, 1}); + out.endFrame(); + byte[] wire = new byte[w.length()]; + System.arraycopy(w.array(), 0, wire, 0, wire.length); + + Http2FrameReader reader = new Http2FrameReader(sourceOf(wire)); + FrameHeader first = reader.readFrame(); + assertEquals(1, first.buffer()[first.payloadOffset()]); + assertEquals(0, first.flags()); + reader.consumeFrame(); + + FrameHeader second = reader.readFrame(); + assertEquals(8, second.buffer()[second.payloadOffset()]); + assertEquals(FrameFlags.ACK, second.flags()); + reader.consumeFrame(); + + assertNull(reader.readFrame()); // clean EOF after both frames consumed + } + + // ── EOF handling ───────────────────────────────────────────────────────── + + @Test + void cleanEofBetweenFrames_returnsNull() throws IOException { + Http2FrameReader reader = new Http2FrameReader(sourceOf(new byte[0])); + assertNull(reader.readFrame()); + } + + @Test + void eofMidFrame_throwsEOFException() { + byte[] wire = buildFrame(FrameType.DATA, 0, 1, new byte[100]); + byte[] truncated = new byte[50]; // header + partial payload + System.arraycopy(wire, 0, truncated, 0, 50); + + Http2FrameReader reader = new Http2FrameReader(sourceOf(truncated)); + assertThrows(EOFException.class, reader::readFrame); + } + + @Test + void eofMidHeader_throwsEOFException() { + byte[] truncated = new byte[5]; // fewer than the 9 header bytes + Http2FrameReader reader = new Http2FrameReader(sourceOf(truncated)); + assertThrows(EOFException.class, reader::readFrame); + } + + // ── Reserved bit masking ───────────────────────────────────────────────── + + @Test + void reservedBitInStreamId_isMaskedNotRejected() throws IOException { + byte[] wire = buildFrame(FrameType.DATA, 0, 5, new byte[]{1, 2, 3}); + wire[5] |= (byte) 0x80; // set the reserved high bit of the stream-id field + Http2FrameReader reader = new Http2FrameReader(sourceOf(wire)); + FrameHeader header = reader.readFrame(); + assertEquals(5, header.streamId(), "reserved bit must be masked, not folded into the stream id"); + } +} diff --git a/flash/src/test/java/dev/relism/flash/h2/frame/PaddingTest.java b/flash/src/test/java/dev/relism/flash/h2/frame/PaddingTest.java new file mode 100644 index 0000000..0b63852 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/h2/frame/PaddingTest.java @@ -0,0 +1,88 @@ +package dev.relism.flash.h2.frame; + +import dev.relism.flash.h2.Http2ErrorCode; +import dev.relism.flash.h2.Http2Exception; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class PaddingTest { + + @Test + void notPadded_returnsWholePayloadUnchanged() { + byte[] buf = {1, 2, 3, 4, 5}; + long r = Padding.unpad(buf, 1, 4, false); + assertEquals(1, Padding.dataOffset(r)); + assertEquals(4, Padding.dataLength(r)); + } + + @Test + void padded_zeroPadLength_allBytesAreData() { + // [padLength=0][data...] + byte[] buf = {0, 10, 20, 30}; + long r = Padding.unpad(buf, 0, 4, true); + assertEquals(1, Padding.dataOffset(r)); + assertEquals(3, Padding.dataLength(r)); + assertEquals(10, buf[Padding.dataOffset(r)]); + } + + @Test + void padded_someData_somePadding() { + // [padLength=2][data: 3 bytes][padding: 2 bytes] -> payload length 6 + byte[] buf = {2, 7, 8, 9, 0, 0}; + long r = Padding.unpad(buf, 0, 6, true); + assertEquals(1, Padding.dataOffset(r)); + assertEquals(3, Padding.dataLength(r)); + assertEquals(7, buf[Padding.dataOffset(r)]); + assertEquals(9, buf[Padding.dataOffset(r) + 2]); + } + + @Test + void padded_allPaddingNoData() { + // [padLength=3][padding x3] -> payload length 4, dataLength 0 + byte[] buf = {3, 0, 0, 0}; + long r = Padding.unpad(buf, 0, 4, true); + assertEquals(0, Padding.dataLength(r)); + } + + @Test + void padded_atNonZeroOffset_withinLargerBuffer() { + byte[] buf = {(byte) 0xFF, (byte) 0xFF, 1, 5, 6, 0, (byte) 0xFF}; + // payload starts at index 2, length 4: [padLength=1][data:5,6][padding:1] + long r = Padding.unpad(buf, 2, 4, true); + assertEquals(3, Padding.dataOffset(r)); + assertEquals(2, Padding.dataLength(r)); + assertEquals(5, buf[Padding.dataOffset(r)]); + assertEquals(6, buf[Padding.dataOffset(r) + 1]); + } + + @Test + void padded_zeroPayloadLength_isProtocolError() { + byte[] buf = {}; + Http2Exception ex = assertThrows(Http2Exception.class, () -> Padding.unpad(buf, 0, 0, true)); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, ex.errorCode()); + } + + @Test + void padded_padLengthEqualsPayloadLength_isProtocolError() { + // payloadLength=3, claimed padLength=3 -- leaves -1 bytes for data, invalid. + byte[] buf = {3, 0, 0}; + Http2Exception ex = assertThrows(Http2Exception.class, () -> Padding.unpad(buf, 0, 3, true)); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, ex.errorCode()); + } + + @Test + void padded_padLengthGreaterThanPayloadLength_isProtocolError() { + byte[] buf = {(byte) 255, 0, 0}; + Http2Exception ex = assertThrows(Http2Exception.class, () -> Padding.unpad(buf, 0, 3, true)); + assertEquals(Http2ErrorCode.PROTOCOL_ERROR, ex.errorCode()); + } + + @Test + void padded_maxValidPadLength_leavesZeroData() { + // payloadLength=5: [padLength=4][padding x4] -- valid, dataLength 0. + byte[] buf = {4, 0, 0, 0, 0}; + long r = Padding.unpad(buf, 0, 5, true); + assertEquals(0, Padding.dataLength(r)); + } +} diff --git a/flash/src/test/java/dev/relism/flash/transport/BufferedByteSourceTest.java b/flash/src/test/java/dev/relism/flash/transport/BufferedByteSourceTest.java new file mode 100644 index 0000000..33dbb57 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/transport/BufferedByteSourceTest.java @@ -0,0 +1,164 @@ +package dev.relism.flash.transport; + +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-37}: this class previously had zero dedicated tests — its deadline mechanism (the + * actual {@code EX-07} slowloris fix) was exercised only indirectly through real-socket, + * end-to-end tests, which never hit the {@code null}-socket path every isolated unit test in + * this codebase actually uses. Found and fixed while building {@code Http2FrameReaderTest} + * (Phase 5); this class closes the gap. + */ +class BufferedByteSourceTest { + + private static BufferedByteSource sourceOf(String s) { + return new BufferedByteSource(new ByteArrayInputStream(s.getBytes(StandardCharsets.US_ASCII)), null); + } + + // ── Plain InputStream passthrough ─────────────────────────────────────── + + @Test + void read_singleByte() throws IOException { + BufferedByteSource src = sourceOf("AB"); + assertEquals('A', src.read()); + assertEquals('B', src.read()); + assertEquals(-1, src.read()); + } + + @Test + void read_intoArray() throws IOException { + BufferedByteSource src = sourceOf("hello world"); + byte[] buf = new byte[5]; + int n = src.read(buf, 0, 5); + assertEquals(5, n); + assertEquals("hello", new String(buf, StandardCharsets.US_ASCII)); + } + + @Test + void read_largerThanInternalBuffer_bypassesBufferCorrectly() throws IOException { + String big = "x".repeat(20_000); + BufferedByteSource src = new BufferedByteSource( + new ByteArrayInputStream(big.getBytes(StandardCharsets.US_ASCII)), null, 4096); + byte[] out = new byte[20_000]; + int total = 0; + while (total < out.length) { + int n = src.read(out, total, out.length - total); + if (n < 0) break; + total += n; + } + assertEquals(20_000, total); + } + + // ── peek / prependOnce ─────────────────────────────────────────────────── + + @Test + void peek_doesNotConsume() throws IOException { + BufferedByteSource src = sourceOf("abcdef"); + byte[] dst = new byte[3]; + int n = src.peek(dst, 0, 3); + assertEquals(3, n); + assertEquals("abc", new String(dst, StandardCharsets.US_ASCII)); + // Still readable from the start — peek must not have advanced the position. + assertEquals('a', src.read()); + assertEquals('b', src.read()); + } + + @Test + void peek_rejectsLengthAboveBufferCapacity() { + BufferedByteSource src = new BufferedByteSource(new ByteArrayInputStream(new byte[0]), null, 16); + assertThrows(IllegalArgumentException.class, () -> src.peek(new byte[20], 0, 20)); + } + + @Test + void prependOnce_servedBeforeUnderlyingBytes() throws IOException { + BufferedByteSource src = sourceOf("world"); + byte[] prefix = "hello ".getBytes(StandardCharsets.US_ASCII); + src.prependOnce(prefix, 0, prefix.length); + + byte[] out = new byte[11]; + int total = 0; + while (total < out.length) { + int n = src.read(out, total, out.length - total); + if (n < 0) break; + total += n; + } + assertEquals("hello world", new String(out, 0, total, StandardCharsets.US_ASCII)); + } + + @Test + void prependOnce_rejectsSecondCallBeforeFirstIsConsumed() { + BufferedByteSource src = sourceOf("x"); + byte[] a = "a".getBytes(StandardCharsets.US_ASCII); + src.prependOnce(a, 0, 1); + assertThrows(IllegalStateException.class, () -> src.prependOnce(a, 0, 1)); + } + + // ── Deadline mechanism, EX-37's actual regression coverage ────────────── + + @Test + void clearDeadline_withNullSocket_doesNotThrow() throws IOException { + BufferedByteSource src = sourceOf("data"); + src.setDeadline(System.nanoTime() + 1_000_000_000L); + assertDoesNotThrow(src::clearDeadline); + } + + @Test + void deadlineAlreadyExpired_throwsSocketTimeoutException_evenWithNullSocket() { + BufferedByteSource src = sourceOf(""); // empty: forces fillFromUnderlying on the next read + src.setDeadline(System.nanoTime() - 1_000_000_000L); // already in the past + assertThrows(SocketTimeoutException.class, () -> src.read(new byte[1], 0, 1)); + } + + @Test + void deadlineNotYetExpired_readsNormally_withNullSocket() throws IOException { + BufferedByteSource src = sourceOf("z"); + src.setDeadline(System.nanoTime() + 30_000_000_000L); // 30s in the future + assertEquals('z', src.read()); + } + + @Test + void bytesAlreadyBuffered_areServedRegardlessOfDeadline() throws IOException { + // peek() fills the internal buffer without a deadline; a since-expired deadline must not + // block already-buffered bytes from being read (only underlying-stream reads are bounded). + BufferedByteSource src = sourceOf("buffered"); + src.peek(new byte[8], 0, 8); + src.setDeadline(System.nanoTime() - 1); // already expired + assertEquals('b', src.read()); // served from the buffer — no underlying read needed + } + + @Test + void clearDeadline_thenRead_neverThrowsTimeoutAfterward() throws IOException { + BufferedByteSource src = sourceOf("ok"); + src.setDeadline(System.nanoTime() - 1); // expired + src.clearDeadline(); + assertEquals('o', src.read()); // deadline cleared — must not time out + } + + // ── available / skip / close ───────────────────────────────────────────── + + @Test + void skip_advancesPastBufferedAndUnderlyingBytes() throws IOException { + BufferedByteSource src = sourceOf("abcdef"); + long skipped = src.skip(3); + assertEquals(3, skipped); + assertEquals('d', src.read()); + } + + @Test + void close_delegatesToUnderlyingStream() { + java.io.InputStream[] closed = new java.io.InputStream[1]; + java.io.InputStream in = new ByteArrayInputStream(new byte[0]) { + @Override public void close() throws IOException { closed[0] = this; super.close(); } + }; + BufferedByteSource src = new BufferedByteSource(in, null); + assertDoesNotThrow(src::close); + assertSame(in, closed[0]); + } +}