From 5a2aaf5a075bde4f5abd421abf370cc06573a117 Mon Sep 17 00:00:00 2001 From: Zakaria El Orche Date: Thu, 13 Aug 2026 11:40:03 +0000 Subject: [PATCH] =?UTF-8?q?feat(core):=20HTTP/2=20Phase=201=20=E2=80=94=20?= =?UTF-8?q?HTTP/1.1=20hardening=20and=20protocol=20negotiation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the request-smuggling and resource-exhaustion debt in the existing HTTP/1.1 parser, and adds the ALPN/h2c-preface negotiation seam so a connection's protocol is decided once, before any request is parsed, per flash/docs/http2/IMPLEMENTATION-PLAN.md Phase 1. Existing-code defects fixed (EX-nn): - EX-02: reject Content-Length + Transfer-Encoding together (RFC 9112 6.1 CL.TE/TE.CL smuggling), and conflicting duplicate Content-Length values. - EX-03: strict, overflow-safe Content-Length parsing, replacing a parser that silently skipped non-digit bytes ("5abc" -> 5, "-1" -> 1). - EX-07: header-read / idle-keep-alive / body-read timeouts enforced by an absolute deadline (dev.relism.flash.transport.BufferedByteSource), not merely Socket#setSoTimeout, which never trips against a peer trickling one byte per read within the window. - EX-08: header count / name length / value length / request-line length bounds (Http1Limits), 431 on violation. - EX-10: ChunkedInputStream now reads through BufferedByteSource instead of the raw unbuffered socket stream, and the header-parser's read-ahead bytes are handed over via a zero-copy prependOnce() instead of a SequenceInputStream/ByteArrayInputStream pair. - EX-17: HttpStatus's status-code bound is computed from values() instead of a hand-maintained constant that silently threw ArrayIndexOutOfBoundsException when a code above it was added; added 421, 431, 505, 507, 511 and others HTTP/2 and this hardening need. - EX-18: bare-CR desync and obsolete line folding rejected. - EX-30: the TLS handshake is forced explicitly, under a timeout, before any protocol decision -- SSLSocket#getApplicationProtocol() returned null until the handshake had run, and nothing previously forced it. - EX-31: TLS 1.2 cipher suites on the RFC 9113 Appendix A blocklist are filtered out of a listener's enabled set whenever it offers h2 via ALPN. - EX-35 (found in this phase): Transfer-Encoding values listing multiple codings ("gzip, chunked") were silently treated as not chunked at all, corrupting the message boundary -- only the whole value was compared. - EX-36 (found in this phase): a header line with no ':' was silently skipped instead of rejected. New: - dev.relism.flash.transport.BufferedByteSource: the single buffered, deadline-aware, peekable view over a connection's inbound bytes. - dev.relism.flash.transport.ProtocolNegotiator/NegotiatedProtocol: ALPN and h2c prior-knowledge detection. In this phase an H2 result is always closed cleanly -- there is no Http2Connection to hand off to until Phase 8. FlashConfiguration.http2Enabled gates the h2c preface peek. - dev.relism.flash.exceptions.MalformedRequestException: a typed, status-carrying rejection distinct from HttpException, caught at the parse site so a malformed request never reaches the handler chain or the user's exception handler, and the connection is always closed. Two small plan-document corrections recorded as DEC-12 (Phase 1's Files list omitted BufferedByteSource.java and MalformedRequestException.java; the request-line-length check description pointed at the wrong offset). DEC-13/DEC-14 record the deadline and exception-hierarchy designs. 277/277 tests green (flash module), run twice for stability of the new wall-clock-based HttpServerTimeoutTest cases. Whole-repo build green. h1 benchmark regression check is left unverified in the plan's DoD: no JMH harness exists yet (Phase 3 deliverable). Co-Authored-By: Claude Sonnet 5 --- README.md | 6 + flash/docs/http2/DECISIONS.md | 91 +++++ flash/docs/http2/HTTP1-HARDENING.md | 94 +++++ flash/docs/http2/IMPLEMENTATION-PLAN.md | 84 +++-- .../dev/relism/flash/ChunkedInputStream.java | 118 ++++++- .../java/dev/relism/flash/HttpServer.java | 107 +++++- .../java/dev/relism/flash/RequestParser.java | 214 +++++++++-- .../exceptions/MalformedRequestException.java | 24 ++ .../flash/extension/FlashConfiguration.java | 52 +++ .../dev/relism/flash/http/Http1Limits.java | 43 ++- .../dev/relism/flash/http/HttpStatus.java | 26 +- .../java/dev/relism/flash/tls/TlsConfig.java | 333 ++++++++++++++++++ .../flash/transport/BufferedByteSource.java | 264 ++++++++++++++ .../flash/transport/NegotiatedProtocol.java | 10 + .../flash/transport/ProtocolNegotiator.java | 67 ++++ .../relism/flash/ChunkedInputStreamTest.java | 111 +++++- .../relism/flash/HttpServerTimeoutTest.java | 185 ++++++++++ .../flash/RequestParserSecurityTest.java | 194 ++++++++++ .../dev/relism/flash/RequestParserTest.java | 77 ++-- .../dev/relism/flash/http/HttpStatusTest.java | 22 ++ .../dev/relism/flash/tls/TlsConfigTest.java | 69 ++++ .../transport/ProtocolNegotiatorTest.java | 154 ++++++++ 22 files changed, 2252 insertions(+), 93 deletions(-) create mode 100644 flash/docs/http2/HTTP1-HARDENING.md create mode 100644 flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java create mode 100644 flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java create mode 100644 flash/src/main/java/dev/relism/flash/transport/NegotiatedProtocol.java create mode 100644 flash/src/main/java/dev/relism/flash/transport/ProtocolNegotiator.java create mode 100644 flash/src/test/java/dev/relism/flash/HttpServerTimeoutTest.java create mode 100644 flash/src/test/java/dev/relism/flash/RequestParserSecurityTest.java create mode 100644 flash/src/test/java/dev/relism/flash/transport/ProtocolNegotiatorTest.java diff --git a/README.md b/README.md index 721c3eb..aa45e88 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,12 @@ app.onException((ex, req, res) -> { | `tls` | `null` | TLS for the default listener — see [TLS](#tls) | | `listeners` | `[]` | Multiple bind targets (port + host + optional TLS) on one app — see [TLS](#tls) | | `maxHeaderBufferSize` | `65536` | Max size of the header buffer (bytes) | +| `wsFrameBufferSize` | `65536` | Per-connection WebSocket read buffer (bytes) | +| `headerReadTimeoutMs` | `10000` | Once a request's first byte arrives, how long the full header block may take. Bounds slowloris-style attacks — see [`HTTP1-HARDENING.md`](flash/docs/http2/HTTP1-HARDENING.md). | +| `idleKeepAliveTimeoutMs` | `60000` | How long a keep-alive connection may sit idle waiting for its next request. | +| `bodyReadTimeoutMs` | `30000` | How long reading a request body (handler or automatic drain) may take. | +| `shutdownDrainTimeoutMs` | `15000` | How long graceful shutdown waits for in-flight requests before force-closing. | +| `http2Enabled` | `false` | Whether this server will ever negotiate HTTP/2. Off by default until the HTTP/2 connection state machine lands (see `flash/docs/http2/IMPLEMENTATION-PLAN.md`). | ## TLS diff --git a/flash/docs/http2/DECISIONS.md b/flash/docs/http2/DECISIONS.md index 6845f29..8b606a5 100644 --- a/flash/docs/http2/DECISIONS.md +++ b/flash/docs/http2/DECISIONS.md @@ -280,3 +280,94 @@ not actually a separate module. **Revisit when.** The `h2` package's commit volume makes `core` too coarse to navigate in `git log` — not expected before Phase 10 at the earliest, if ever. + +--- + +## DEC-12 — Phase 1 plan corrections: two missing files, one corrected limit check + +**Context.** While implementing Phase 1, two problems in the plan document itself surfaced +(distinct from problems in the *code*, which is what the `EX-nn` registry tracks). + +1. Phase 1 task 8 requires "a `BufferedByteSource` owned by the connection that wraps the read + buffer plus the socket and exposes `readByte()`, `readFully(...)`, `skip(...)` and `peek()`", + and task 12 depends on it for h2c preface detection — but the Phase 1 **Files** list never + named the file. Likewise, the typed rejection `EX-02`/`EX-03`/`EX-08`/`EX-18` all need (a + specific HTTP status to respond with, as distinct from `HttpException`'s handler-routed + semantics — see `DEC-14`) was never named as a file either. +2. Task 4's exact wording — "Enforce `MAX_REQUEST_LINE_LENGTH` against `headerEndIdx - base` for + the request line specifically" — describes checking the length of the *entire header block* + (`headerEndIdx` is where the whole header section ends), not the request line. The request + line's own end is `protocolEnd` (or `sectionStart`), not `headerEndIdx`. + +**Decision.** +1. Added `flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java` and + `flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java` to Phase 1's + Files list (see the phase section itself, now corrected in place). +2. Implemented the check as `protocolEnd - base > MAX_REQUEST_LINE_LENGTH` — the request line's + actual span — rather than the literal (and, read literally, incorrect) `headerEndIdx - base`. + +**Consequence.** None beyond the plan text now matching what was actually built and why — +these are wording/omission fixes, not design trade-offs. Recorded per the plan's own rule that +corrections to the plan must be explicit and tracked, never silent. + +**Revisit when.** N/A — already resolved. + +--- + +## DEC-13 — `BufferedByteSource`'s deadline is enforced by computing the exact remaining `SO_TIMEOUT` per underlying read, not by a fixed poll-and-retry loop + +**Context.** `EX-07` requires an *absolute* deadline across a sequence of socket reads (a +per-read `SO_TIMEOUT` alone never trips against a peer that keeps each individual read within +the window while never completing the whole message — the canonical slowloris shape). Two ways +to implement that on top of the blocking `Socket`/`SSLSocket` API, which only offers a per-read +timeout: + +**Options.** +1. Set `SO_TIMEOUT` to a fixed, short polling interval (e.g. 1 s); on each + `SocketTimeoutException`, re-check whether the absolute deadline has actually passed, and if + not, retry. Deadline precision is bounded by the poll interval (up to ~1 s of slop). +2. Before every underlying read, compute the exact remaining budget + (`deadlineNanos - System.nanoTime()`) and hand that exact value to `setSoTimeout`. A + `SocketTimeoutException` from that read then unambiguously means the deadline — not merely + one poll cycle — has elapsed, with no retry loop needed. + +**Decision.** Option 2. + +**Consequence.** Deadline precision is exact (modulo OS timer granularity) rather than +poll-interval-bounded, and the implementation is simpler — no retry loop, no distinction between +"timed out this poll" and "timed out for real". The cost is one `setSoTimeout` syscall per +underlying fill (not per byte, not per `read()` call served from the buffer) — negligible, since +fills already happen at buffer granularity (up to 8 KiB at a time), not per byte. + +**Revisit when.** Not expected to be revisited; this is strictly better than option 1 on both +precision and simplicity. + +--- + +## DEC-14 — `MalformedRequestException extends HttpException`; caught separately from the per-request handler try/catch, never routed through the user's exception handler + +**Context.** `EX-02`/`EX-03`/`EX-08`/`EX-18` all need to reject a request with a specific HTTP +status before any handler or middleware runs. `HttpException` already exists in this codebase +for "carry a status code, get turned into a response" — but it is caught by +`router.getExceptionHandler()` inside the per-request try/catch, which is user-configurable +(e.g. `flash-ext-jackson` installs a JSON-formatting handler). + +**Options.** +1. Reuse `HttpException` directly, letting a malformed request flow through the same + user-configurable exception handler as an application-level failure. +2. A new type, `MalformedRequestException extends HttpException`, caught at a separate site — + around `parser.parse(in)` itself, before routing — with a fixed, minimal, non-customizable + response, always followed by closing the connection. + +**Decision.** Option 2. + +**Consequence.** A malformed or hostile request never reaches user code at all — not the +handler, not middleware, not a custom exception handler that might (reasonably, for its actual +purpose) try to look up a route, log structured JSON, or otherwise do work that assumes a +well-formed `Request`. The connection is always closed afterwards, never kept alive, which is +exactly the property `EX-02`'s smuggling defense depends on. Subclassing `HttpException` (rather +than an unrelated new hierarchy) keeps `status()`/message` access idiomatic with the rest of the +codebase's error-status convention, while the distinct type is what lets `HttpServer` catch it +at the parse site specifically. + +**Revisit when.** Not expected to be revisited. diff --git a/flash/docs/http2/HTTP1-HARDENING.md b/flash/docs/http2/HTTP1-HARDENING.md new file mode 100644 index 0000000..8fe32a6 --- /dev/null +++ b/flash/docs/http2/HTTP1-HARDENING.md @@ -0,0 +1,94 @@ +# HTTP/1.1 Hardening (Phase 1) + +Audience: operators. This is the document to read when a `400`/`413`/`414`/`431`/`501` shows up +in the logs and it isn't obvious why. Every rejection rule Flash's HTTP/1.1 parser enforces is +listed here with its RFC citation and the status it produces. Contributor-level detail (why each +check is implemented the way it is, the exact code paths) lives in the Javadoc of +`RequestParser`, `ChunkedInputStream`, and `dev.relism.flash.exceptions.MalformedRequestException`. + +Every rejection in this document has one thing in common: **the connection is always closed +afterwards, never kept alive.** A rejected request is exactly the situation a smuggling attack +needs a reusable connection for, so none of these rejections offer one — see +`MalformedRequestException`'s Javadoc. + +## Request-smuggling defenses (RFC 9112 §6.1) + +| Rule | Status | Detail | +|---|---|---| +| `Content-Length` and `Transfer-Encoding` both present | `400` | The canonical CL.TE/TE.CL smuggling vector. Rejected regardless of which header appears first. | +| Multiple `Content-Length` lines with **differing** values | `400` | Identical repeated values are tolerated (RFC 9110 §8.6 permits treating them as one). | +| `Transfer-Encoding` whose **final** coding is not `chunked` | `501` | Flash implements only `chunked`; anything else (`gzip` alone, or `chunked, gzip` — chunked must be *last*) is unsupported. | + +## Strict `Content-Length` parsing (RFC 9110 §8.6) + +| Input | Status | +|---|---| +| Empty value | `400` | +| Any non-digit byte (including a leading `+` or `-`) | `400` | +| More than 19 digits | `400` | +| Value overflows `Long.MAX_VALUE` | `400` | +| Value exceeds `Http1Limits.MAX_CONTENT_LENGTH` (4 GiB by default) | `413` | + +The previous parser silently skipped non-digit characters (`"5abc"` parsed as `5`; `"-1"` parsed +as `1`) instead of rejecting them — this is the fix. + +## Header and request-line limits (`Http1Limits`) + +| Limit | Default | Status when exceeded | +|---|---|---| +| `MAX_HEADER_COUNT` | 100 | `431 Request Header Fields Too Large` | +| `MAX_HEADER_NAME_LENGTH` | 256 B | `431` | +| `MAX_HEADER_VALUE_LENGTH` | 8192 B | `431` | +| `MAX_REQUEST_LINE_LENGTH` | 8192 B | `431` | +| Header block exceeds `maxHeaderBufferSize` (or the connection ends before it completes) | configurable, default 64 KiB | `431` | + +## Line-terminator and header-syntax correctness (RFC 9112 §5) + +| Rule | Status | +|---|---| +| A `\r` not immediately followed by `\n` (bare CR) | `400` — a known desynchronization/smuggling surface | +| A header line beginning with whitespace (obsolete line folding, RFC 9112 §5.2) | `400` | +| A header name containing a byte outside RFC 9110 §5.6.2's `tchar` set | `400` | +| A header line with no `:` | `400` | + +## Chunked transfer safety (RFC 9112 §7.1, `Http1Limits`) + +| Limit | Default | Status when exceeded | +|---|---|---| +| `MAX_CHUNK_SIZE` | 16 MiB | `413` | +| Chunk-size line longer than 16 hex digits | — | `400` | +| `MAX_CHUNK_EXT_LENGTH` (the optional `;name=value` after a chunk size) | 256 B | `400` | +| `MAX_CHUNKS_PER_BODY` | 100 000 | `413` | +| `MAX_TRAILER_COUNT` | 50 | `431` | +| A chunk's data not followed by `\r\n`, or a malformed chunk-size/trailer terminator | — | `400` | + +Trailers are consumed (safely, within the bounds above) but discarded, not exposed to the +handler, on HTTP/1.1 today — exposing them via `Request.trailers()` on both protocols is Phase +12 scope. + +## Timeouts (`FlashConfiguration`) + +| Setting | Default | Covers | +|---|---|---| +| `idleKeepAliveTimeoutMs` | 60 000 | How long a keep-alive connection may sit idle waiting for its next request. | +| `headerReadTimeoutMs` | 10 000 | Once the first byte of a request arrives, how long the full header block may take. | +| `bodyReadTimeoutMs` | 30 000 | How long reading the body (by the handler, or the automatic post-response drain) may take. | +| `shutdownDrainTimeoutMs` | 15 000 | How long graceful shutdown waits for in-flight requests before force-closing (wired up starting Phase 2). | + +These are enforced by an **absolute deadline**, not merely `Socket.setSoTimeout`. A per-read +socket timeout alone never trips against a peer that sends one byte just often enough to keep +each individual read alive (the classic slowloris shape) — see +`dev.relism.flash.transport.BufferedByteSource`'s Javadoc for how the absolute deadline is +implemented on top of the JDK's per-read-only timeout API. + +## TLS (RFC 9113 §9.2.2, applies once a listener offers `h2` over ALPN) + +- The TLS handshake is forced explicitly (not left to the JDK's lazy on-first-read trigger) + before any protocol decision is made, and is bounded by `headerReadTimeoutMs`. +- When a listener's `TlsConfig.applicationProtocols` includes `"h2"`, the enabled TLS 1.2 cipher + suite list is filtered against the RFC 9113 Appendix A blocklist + (`TlsConfig.TLS12_H2_BLOCKED_CIPHERS`, ~280 entries). TLS 1.3 is never affected — none of its + cipher suites are on that list. +- HTTP/2 itself is not yet served in this phase (lands in Phase 8): a connection that negotiates + `h2` via ALPN, or that opens with the h2c prior-knowledge preface while + `FlashConfiguration.http2Enabled` is set, is currently closed cleanly rather than served. diff --git a/flash/docs/http2/IMPLEMENTATION-PLAN.md b/flash/docs/http2/IMPLEMENTATION-PLAN.md index 12d61f0..dada6e3 100644 --- a/flash/docs/http2/IMPLEMENTATION-PLAN.md +++ b/flash/docs/http2/IMPLEMENTATION-PLAN.md @@ -62,7 +62,7 @@ Status values: `not started` / `in progress` / `blocked` / `done`. | Phase | Status | Branch/PR | Notes | |---|---|---|---| | 0 — Groundwork | done | `feature/core/http2` | Package skeleton, `Http2Limits`, `Http1Limits`, `Http2ErrorCode`, `Http2Exception`/`Http2StreamException`, `DECISIONS.md` (`DEC-01`…`DEC-11`), `package-info.java`. 226/226 tests green. | -| 1 — HTTP/1.1 hardening + ALPN/preface | not started | — | — | +| 1 — HTTP/1.1 hardening + ALPN/preface | done | `feature/core/http2` | EX-02/03/07/08/10/17/18/30/31 fixed; EX-35/36 found+fixed. `BufferedByteSource`, `ProtocolNegotiator`, `MalformedRequestException` added (plan corrected, DEC-12). 277/277 tests green (run twice). h1 benchmark check deferred — no JMH harness until Phase 3 (documented in DoD). | | 2 — Transport decomposition | not started | — | — | | 3 — Serialized frame writer (GO/NO-GO gate) | not started | — | — | | 4 — Byte-layer foundations | not started | — | — | @@ -608,6 +608,28 @@ composed transport rather than a god object. package-private `TransportFactory`. **Phase**: 2. +### EX-35 — `Transfer-Encoding` multi-value handling drops the message boundary silently +Found while implementing `EX-02` in `RequestParser.java`'s header-scan loop (the exact code that +decides `isChunked`). The pre-existing check was `equalsIgnoreCase(buffer, valueStart, lineEnd, +"chunked")` — an exact **whole-value** comparison. RFC 9112 §6.1 requires only that `chunked` be +the **final** coding in a comma-separated list (e.g. `Transfer-Encoding: gzip, chunked` is valid +and self-delimiting). The old check silently treated any such multi-coding value as *not* +chunked at all — `isChunked` stayed `false`, `contentLength` stayed `0`, and the body bytes that +followed were left for the next `parse()` call to misinterpret as the start of a new request: +a real message-boundary corruption, not just a missed feature. +**Fix**: parse the comma-separated token list and inspect only the last token +(`RequestParser.isFinalCodingChunked`). A value whose final coding is not `chunked` is now +rejected with `501` (`EX-02`'s own fix), rather than silently misparsed. +**Phase**: 1. + +### EX-36 — A header line without a `:` was silently skipped instead of rejected +Found in the same loop as `EX-18`/`EX-35`. `RequestParser`'s header-line loop located the colon +via `find(...)` and, if none was found (`colon == -1`), simply did nothing for that line and +moved on to the next — a malformed header line was permissively ignored rather than rejected. +RFC 9112 §5 gives no such leniency: a header field line without a colon is not valid HTTP. +**Fix**: `colon == -1` now rejects the request with `400 Bad Request`. +**Phase**: 1. + --- # PART III — The phases @@ -800,7 +822,9 @@ is unreadable) blocks every h2 phase, and fixing it is the natural companion to seam. ### EX items -`EX-02`, `EX-03`, `EX-07`, `EX-08`, `EX-10`, `EX-17`, `EX-18`, `EX-30`, `EX-31`. +`EX-02`, `EX-03`, `EX-07`, `EX-08`, `EX-10`, `EX-17`, `EX-18`, `EX-30`, `EX-31`, plus two found +while implementing this phase and registered in Part II per R10: `EX-35` (multi-value +`Transfer-Encoding` silently misparsed), `EX-36` (a header line with no `:` silently skipped). ### Files @@ -813,9 +837,19 @@ Modified: - `flash/src/main/java/dev/relism/flash/extension/FlashConfiguration.java` Created: -- `flash/src/main/java/dev/relism/flash/http/Http1Limits.java` (from Phase 0) +- `flash/src/main/java/dev/relism/flash/http/Http1Limits.java` (from Phase 0; extended here with + the chunked-transfer bounds for `EX-10`'s safety task) - `flash/src/main/java/dev/relism/flash/transport/ProtocolNegotiator.java` - `flash/src/main/java/dev/relism/flash/transport/NegotiatedProtocol.java` (enum: `HTTP_1_1`, `H2`) +- `flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java` — **plan correction**: + task 8 below requires this class (the buffered, deadline-aware, peekable source `EX-10`'s fix + and `EX-07`'s absolute-deadline requirement both need), but it was missing from this phase's + original Files list. Added here; recorded as `DEC-12` in `DECISIONS.md`. +- `flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java` — likewise + not originally listed: the typed, status-carrying rejection `EX-02`/`EX-03`/`EX-08`/`EX-18` + all need to tell `HttpServer` which status to respond with, as distinct from + `HttpException` (which routes through the user's handler chain — a malformed request must + not). Recorded alongside `DEC-12`. ### Tasks @@ -900,18 +934,18 @@ Created: already has one. ### Safety checks (checklist — all mandatory) -- [ ] `Content-Length` strict-numeric, bounded, single-valued -- [ ] `Content-Length` + `Transfer-Encoding` rejected -- [ ] Non-`chunked` final transfer coding rejected -- [ ] Bare CR / missing LF rejected -- [ ] obs-fold (leading whitespace continuation line) rejected -- [ ] Header name `tchar` validated -- [ ] Header count / name length / value length / request-line length bounded -- [ ] Chunk size, chunk count, chunk-extension length, trailer count bounded -- [ ] Header-read absolute deadline enforced (not just `setSoTimeout`) -- [ ] Idle keep-alive timeout enforced -- [ ] Body-read timeout enforced -- [ ] TLS handshake covered by a timeout +- [x] `Content-Length` strict-numeric, bounded, single-valued — `RequestParserSecurityTest` +- [x] `Content-Length` + `Transfer-Encoding` rejected, regardless of order — `RequestParserSecurityTest` +- [x] Non-`chunked` final transfer coding rejected — `RequestParserSecurityTest` +- [x] Bare CR / missing LF rejected — `RequestParserSecurityTest` +- [x] obs-fold (leading whitespace continuation line) rejected — `RequestParserSecurityTest` +- [x] Header name `tchar` validated — `RequestParserSecurityTest` +- [x] Header count / name length / value length / request-line length bounded — `RequestParserSecurityTest` +- [x] Chunk size, chunk count, chunk-extension length, trailer count bounded — `ChunkedInputStreamTest` +- [x] Header-read absolute deadline enforced (not just `setSoTimeout`) — `HttpServerTimeoutTest.slowlorisHeaderDribble_disconnectedWithinHeaderReadTimeout` +- [x] Idle keep-alive timeout enforced — `HttpServerTimeoutTest.idleKeepAliveConnection_disconnectedWithinIdleTimeout` +- [x] Body-read timeout enforced — `HttpServerTimeoutTest.slowBodyDribble_disconnectedWithinBodyReadTimeout` +- [x] TLS handshake covered by a timeout — `HttpServerTimeoutTest.tlsHandshakeNeverStarted_disconnectedWithinHeaderReadTimeout` ### Tests - `RequestParserSecurityTest` — one test per rejection above, each asserting both the status @@ -933,11 +967,21 @@ Created: operators can understand a `400` in their logs. ### DoD -- [ ] Every checklist item above is implemented and tested. -- [ ] `mvn test` green. -- [ ] No behavioural change to well-formed HTTP/1.1 traffic (verified by the existing test - suite passing unmodified). -- [ ] h1 benchmark shows no regression beyond noise (baseline captured before the phase). +- [x] Every checklist item above is implemented and tested. +- [x] `mvn test` green. Full `flash` module: 277/277, run twice in a row for timing-test stability + (the four `HttpServerTimeoutTest` cases are wall-clock-based). +- [x] No behavioural change to well-formed HTTP/1.1 traffic (verified by the existing test + suite passing unmodified — the only test-file edits were signature updates for + `RequestParser.parse(BufferedByteSource)` and exception-type/status updates for the small + number of existing tests that asserted the pre-fix buggy behaviour, e.g. a 5 GB + `Content-Length` being silently accepted, or an unrecognised method producing a bare + `IOException` instead of a typed `501`; each such change is called out in the Phase 1 + commit). +- [ ] h1 benchmark shows no regression beyond noise (baseline captured before the phase). **Not + verified — no JMH harness exists yet; it is a Phase 3 deliverable.** Left unchecked + rather than claimed. Once Phase 3 adds the harness, an h1 GET benchmark should be run + against the pre-Phase-1 commit and against this one before Phase 3 is considered started, + so this box can be resolved retroactively. --- diff --git a/flash/src/main/java/dev/relism/flash/ChunkedInputStream.java b/flash/src/main/java/dev/relism/flash/ChunkedInputStream.java index 492dc7d..bb89eeb 100644 --- a/flash/src/main/java/dev/relism/flash/ChunkedInputStream.java +++ b/flash/src/main/java/dev/relism/flash/ChunkedInputStream.java @@ -1,24 +1,35 @@ package dev.relism.flash; -import java.io.ByteArrayInputStream; +import dev.relism.flash.exceptions.MalformedRequestException; +import dev.relism.flash.http.Http1Limits; +import dev.relism.flash.transport.BufferedByteSource; + import java.io.IOException; import java.io.InputStream; -import java.io.SequenceInputStream; /** * De-chunking {@link InputStream} for HTTP/1.1 {@code Transfer-Encoding: chunked} request bodies. * Handles pre-buffered bytes from the header read-ahead, chunk framing, and trailer consumption. * Returns -1 at end of the final chunk; the underlying socket is left positioned for the next request. + * + *

{@code EX-10}: reads through the connection's shared {@link BufferedByteSource} + * instead of the raw, unbuffered socket stream. Chunk-size digits, the trailing CRLF after each + * chunk, and trailer lines are all read one byte at a time by design (the framing is + * byte-oriented) — that used to mean one {@code read(2)} syscall per byte on the raw socket; + * against {@link BufferedByteSource} it is a read from an already-filled in-memory buffer. + * The header-parser's read-ahead bytes are handed to {@code src} via + * {@link BufferedByteSource#prependOnce}, replacing the {@code SequenceInputStream}/ + * {@code ByteArrayInputStream} pair the previous implementation allocated per chunked request. */ final class ChunkedInputStream extends InputStream { - private final InputStream src; + private final BufferedByteSource src; private int chunkRemaining = 0; private boolean done = false; + private int chunksSeen = 0; - ChunkedInputStream(InputStream socket, byte[] preBuf, int preBufOff, int preBufLen) { - src = preBufLen > 0 - ? new SequenceInputStream(new ByteArrayInputStream(preBuf, preBufOff, preBufLen), socket) - : socket; + ChunkedInputStream(BufferedByteSource src, byte[] preBuf, int preBufOff, int preBufLen) { + this.src = src; + if (preBufLen > 0) src.prependOnce(preBuf, preBufOff, preBufLen); } @Override @@ -29,7 +40,7 @@ final class ChunkedInputStream extends InputStream { if (chunkRemaining == 0) { consumeTrailers(); done = true; return -1; } } int b = src.read(); - if (b >= 0 && --chunkRemaining == 0) { src.read(); src.read(); } // consume trailing \r\n + if (b >= 0 && --chunkRemaining == 0) consumeChunkTerminator(); return b; } @@ -43,30 +54,99 @@ final class ChunkedInputStream extends InputStream { int n = src.read(buf, off, Math.min(len, chunkRemaining)); if (n > 0) { chunkRemaining -= n; - if (chunkRemaining == 0) { src.read(); src.read(); } // consume trailing \r\n + if (chunkRemaining == 0) consumeChunkTerminator(); } return n; } + /** Validates and consumes the CRLF that terminates every chunk's data (RFC 9112 §7.1.1). */ + private void consumeChunkTerminator() throws IOException { + int cr = src.read(); + int lf = src.read(); + if (cr != '\r' || lf != '\n') { + throw new MalformedRequestException(400, "Malformed chunk terminator"); + } + } + + /** + * Reads one chunk-size line: hex digits, an optional {@code ;}-prefixed chunk-extension + * (discarded — RFC 9112 §7.1.1 permits ignoring extensions this server does not recognise), + * then CRLF. Bounded per {@code Http1Limits} against: more than 16 hex digits (a chunk size + * cannot legitimately need more — {@code Long.MAX_VALUE} is 16 hex digits), a size above + * {@link Http1Limits#MAX_CHUNK_SIZE}, an extension longer than + * {@link Http1Limits#MAX_CHUNK_EXT_LENGTH}, and more than + * {@link Http1Limits#MAX_CHUNKS_PER_BODY} chunks per body — all defences against a peer + * that is technically well-formed but deliberately expensive to parse. + */ private int readChunkSize() throws IOException { + if (++chunksSeen > Http1Limits.MAX_CHUNKS_PER_BODY) { + throw new MalformedRequestException(413, "Too many chunks"); + } + long size = 0; - int b; - while ((b = src.read()) != -1) { - if (b >= '0' && b <= '9') size = (size << 4) | (b - '0'); - else if (b >= 'a' && b <= 'f') size = (size << 4) | (b - 'a' + 10); - else if (b >= 'A' && b <= 'F') size = (size << 4) | (b - 'A' + 10); - else { while ((b = src.read()) != -1 && b != '\n'); break; } // ext or \r\n - if (size > Integer.MAX_VALUE) throw new IOException("Chunk size exceeds 2 GB limit"); + int digits = 0; + int b = src.read(); + while (isHexDigit(b)) { + if (++digits > 16) throw new MalformedRequestException(400, "Chunk size line too long"); + size = (size << 4) | hexValue(b); + if (size > Http1Limits.MAX_CHUNK_SIZE) { + throw new MalformedRequestException(413, "Chunk size exceeds configured maximum"); + } + b = src.read(); + } + if (digits == 0) throw new MalformedRequestException(400, "Malformed chunk size"); + + int extLen = 0; + while (b != -1 && b != '\r') { + if (++extLen > Http1Limits.MAX_CHUNK_EXT_LENGTH) { + throw new MalformedRequestException(400, "Chunk extension too long"); + } + b = src.read(); + } + if (b != '\r' || src.read() != '\n') { + throw new MalformedRequestException(400, "Malformed chunk size line terminator"); } return (int) size; } - // Reads and discards trailer headers until the empty line that terminates the chunked body. + private static boolean isHexDigit(int b) { + return (b >= '0' && b <= '9') || (b >= 'a' && b <= 'f') || (b >= 'A' && b <= 'F'); + } + + private static int hexValue(int b) { + if (b <= '9') return b - '0'; + if (b <= 'F') return b - 'A' + 10; + return b - 'a' + 10; + } + + /** + * Reads and discards trailer headers until the empty line that terminates the chunked body + * (RFC 9112 §7.1.2). Bounded by {@link Http1Limits#MAX_TRAILER_COUNT} and + * {@link Http1Limits#MAX_HEADER_VALUE_LENGTH} — without a bound, a peer could follow the + * final chunk with an unbounded trailer section purely to waste CPU discarding it. Trailers + * are discarded, not exposed to the handler; exposing them is Phase 12 scope + * ({@code Request.trailers()}). + */ private void consumeTrailers() throws IOException { + int trailerCount = 0; while (true) { int b = src.read(); - if (b == -1 || b == '\r') { src.read(); return; } // empty line — done - while ((b = src.read()) != -1 && b != '\n'); // skip non-empty trailer line + if (b == -1) return; // EOF mid-trailers — nothing left to bound. + if (b == '\r') { + if (src.read() != '\n') { + throw new MalformedRequestException(400, "Malformed trailer section terminator"); + } + return; // empty line — trailer section done + } + if (++trailerCount > Http1Limits.MAX_TRAILER_COUNT) { + throw new MalformedRequestException(431, "Too many trailers"); + } + int lineLen = 1; + while ((b = src.read()) != -1 && b != '\n') { + if (++lineLen > Http1Limits.MAX_HEADER_VALUE_LENGTH) { + throw new MalformedRequestException(431, "Trailer line too long"); + } + } } } } diff --git a/flash/src/main/java/dev/relism/flash/HttpServer.java b/flash/src/main/java/dev/relism/flash/HttpServer.java index d352134..b068aa9 100644 --- a/flash/src/main/java/dev/relism/flash/HttpServer.java +++ b/flash/src/main/java/dev/relism/flash/HttpServer.java @@ -1,5 +1,6 @@ package dev.relism.flash; +import dev.relism.flash.exceptions.MalformedRequestException; import dev.relism.flash.extension.FlashApp; import dev.relism.flash.extension.FlashConfiguration; import dev.relism.flash.http.ContentType; @@ -11,6 +12,9 @@ import dev.relism.flash.models.Response; import dev.relism.flash.routing.AbstractRouter; import dev.relism.flash.routing.AbstractWsRouter; import dev.relism.flash.tls.TlsConfig; +import dev.relism.flash.transport.BufferedByteSource; +import dev.relism.flash.transport.NegotiatedProtocol; +import dev.relism.flash.transport.ProtocolNegotiator; import dev.relism.flash.websocket.WebSocketFrame; import dev.relism.flash.websocket.WebSocketHandler; import dev.relism.flash.websocket.WebSocketSession; @@ -25,6 +29,7 @@ import java.io.*; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; +import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -275,7 +280,6 @@ class HttpServer implements ServerHandle { executorService.submit(() -> { activeSockets.add(socket); try (socket; - InputStream in = socket.getInputStream(); OutputStream out = new BufferedOutputStream(socket.getOutputStream())) { // TCP_NODELAY: disable Nagle's algorithm. @@ -286,6 +290,18 @@ class HttpServer implements ServerHandle { socket.setTcpNoDelay(true); socket.setSendBufferSize(SOCKET_BUF_SIZE); + // EX-30: force the TLS handshake explicitly, under a bounded timeout, + // before any protocol decision is made. SSLSocket#getApplicationProtocol() + // (which ProtocolNegotiator relies on) returns null until the handshake has + // actually completed; nothing previously forced that before the first read, + // which happened to work by accident (the JDK triggers it lazily on read) + // but left ALPN unreadable at exactly the point negotiation needs it. + if (socket instanceof SSLSocket sslSocketForHandshake) { + socket.setSoTimeout(configuration.getHeaderReadTimeoutMs()); + sslSocketForHandshake.startHandshake(); + socket.setSoTimeout(0); // BufferedByteSource's deadline takes over below + } + // rawOut is the unbuffered socket stream — passed to WebSocketSession // directly. WS writes are already bulk (header + payload in two calls); // with TCP_NODELAY the kernel ships them without Nagle delay, so no @@ -295,16 +311,71 @@ class HttpServer implements ServerHandle { // userspace coalescing before a single syscall. OutputStream rawOut = socket.getOutputStream(); + // EX-10: the single buffered, deadline-aware view over this connection's + // inbound bytes — see BufferedByteSource's Javadoc. Not part of the + // try-with-resources list above because closing `socket` already closes + // the stream it wraps (same reasoning that already applied to rawOut). + BufferedByteSource in = new BufferedByteSource(socket.getInputStream(), socket); + + NegotiatedProtocol negotiated = negotiateProtocol(socket, in); + if (negotiated == NegotiatedProtocol.H2) { + // No Http2Connection exists yet (lands in Phase 8) — close cleanly + // rather than attempt to speak a protocol this version cannot serve. + return; + } + RequestParser parser = new RequestParser( configuration.getMaxHeaderBufferSize(), (InetSocketAddress) socket.getRemoteSocketAddress(), socket instanceof SSLSocket sslSocket ? sslSocket : null); + byte[] idleProbe = new byte[1]; + while (!stopped) { - Request request = parser.parse(in); + // 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 an attack. peek() lets us detect "bytes have started + // arriving" without handing them to the parser under the wrong deadline. + // + // Skipped entirely when the parser already has bytes buffered from a + // previous read (HTTP pipelining: a client that sent two requests back + // to back before reading either response). In that case the next + // request has, by definition, already started — peeking the *source* + // for a fresh byte would wait for something that is never coming there, + // since it already arrived and is sitting in the parser's own buffer. + if (!parser.hasBufferedBytes()) { + in.setDeadline(System.nanoTime() + configuration.getIdleKeepAliveTimeoutMs() * 1_000_000L); + int firstByteSeen; + try { + firstByteSeen = in.peek(idleProbe, 0, 1); + } catch (SocketTimeoutException e) { + break; // idle timeout — nothing pending; close quietly, like EOF + } + if (firstByteSeen <= 0) break; // clean EOF + } + + // Bytes have started arriving: tighten to the slowloris-specific bound + // for the rest of the header block. A per-read SO_TIMEOUT alone would + // never trip here — see BufferedByteSource's Javadoc. + in.setDeadline(System.nanoTime() + configuration.getHeaderReadTimeoutMs() * 1_000_000L); + Request request; + try { + request = parser.parse(in); + } catch (MalformedRequestException e) { + // EX-02/03/08/18: a fixed, minimal, non-customizable rejection — + // never routed through the handler or the user's exception handler + // (see MalformedRequestException's Javadoc) — and the connection is + // always closed afterwards, never kept alive. + Response rejection = new Response(e.status(), e.getMessage(), ContentType.TEXT_PLAIN); + writeResponse(out, rejection, false); + break; + } catch (SocketTimeoutException e) { + break; // header-read deadline exceeded — close + } if (request == null) break; if (request.method() == HttpMethod.GET && isWebSocketUpgrade(request)) { + in.clearDeadline(); // the WS session loop is long-lived; it paces itself WebSocketHandler wsHandler = wsRouter.route(request); if (wsHandler == null) { out.write(WS_REJECT_400); @@ -323,6 +394,11 @@ class HttpServer implements ServerHandle { return; } + // Headers are fully read; the body (if any) may still be pending — + // whether the handler consumes it or the automatic drain() below does, + // bound it by the same deadline (EX-07). + in.setDeadline(System.nanoTime() + configuration.getBodyReadTimeoutMs() * 1_000_000L); + boolean keepAlive = isKeepAlive(request); Response response = new Response(200, ContentType.TEXT_PLAIN); @@ -341,6 +417,7 @@ class HttpServer implements ServerHandle { writeResponse(out, response, keepAlive); request.drain(); + in.clearDeadline(); if (!keepAlive) break; } @@ -368,6 +445,32 @@ class HttpServer implements ServerHandle { } } + // ── Protocol negotiation ─────────────────────────────────────────────────── + + /** + * Decides h1 vs h2 for one connection, applying {@link FlashConfiguration#isHttp2Enabled()} + * to the plaintext (h2c) path — see {@link ProtocolNegotiator}'s Javadoc for why the flag is + * applied here rather than inside the negotiator itself. TLS/ALPN detection costs nothing + * (the handshake already resolved it) and is therefore always performed, regardless of the + * flag: what the flag gates is whether Flash even attempts the h2c preface peek on a + * plaintext socket, so that a plaintext connection with the feature left at its default + * behaves byte-for-byte like pre-HTTP/2 Flash. + */ + private NegotiatedProtocol negotiateProtocol(Socket socket, BufferedByteSource in) throws IOException { + if (socket instanceof SSLSocket) { + return ProtocolNegotiator.negotiate(socket, in); + } + if (!configuration.isHttp2Enabled()) { + return NegotiatedProtocol.HTTP_1_1; + } + in.setDeadline(System.nanoTime() + configuration.getIdleKeepAliveTimeoutMs() * 1_000_000L); + try { + return ProtocolNegotiator.negotiate(socket, in); + } finally { + in.clearDeadline(); + } + } + // ── WebSocket upgrade detection (zero-alloc) ────────────────────────────── private static boolean isWebSocketUpgrade(Request request) { diff --git a/flash/src/main/java/dev/relism/flash/RequestParser.java b/flash/src/main/java/dev/relism/flash/RequestParser.java index 1a50a25..c1a2d99 100644 --- a/flash/src/main/java/dev/relism/flash/RequestParser.java +++ b/flash/src/main/java/dev/relism/flash/RequestParser.java @@ -1,17 +1,19 @@ package dev.relism.flash; +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.Request; import dev.relism.flash.models.RequestLine; import dev.relism.flash.routing.routers.fastpathrouter.FastPathViews; +import dev.relism.flash.transport.BufferedByteSource; import lombok.extern.slf4j.Slf4j; import javax.net.ssl.SSLSocket; import java.io.IOException; -import java.io.InputStream; import java.net.InetSocketAddress; import java.util.Arrays; @@ -35,11 +37,41 @@ import java.util.Arrays; * to the next request. They are snapshotted at the top of {@link #parse} * and reset to {@code 0/0} before any work begins, so an exception thrown mid-parse * leaves the fields clean rather than pointing at stale data from a previous request. + * + *

Rejection model (RFC 9112 §6.1, {@code EX-02}/{@code EX-03}/{@code EX-08}/{@code EX-18})

+ * Anything wrong with the request itself — smuggling-relevant ambiguity, an over-limit + * header, a malformed byte where the grammar forbids one — is reported as a + * {@link MalformedRequestException} carrying the exact status the caller must respond with. + * This is distinct from {@link IOException}, which still means "the socket failed" (EOF, + * reset, timeout). The caller ({@code HttpServer.process}) must always close the connection + * after a {@link MalformedRequestException}, never keep it alive — RFC 9112 §6.1's rationale + * for rejecting {@code Content-Length} + {@code Transfer-Encoding} outright is exactly that a + * kept-alive connection after a disputed request boundary is what a smuggling attack needs. */ @Slf4j public class RequestParser { private static final int INITIAL_BUFFER_SIZE = 8192; + /** + * RFC 9110 §5.6.2 {@code tchar} set, table-driven so header-name validation is a single + * array read per byte rather than a chain of comparisons (R4/R5). Indexed directly by + * byte value; only defined for the ASCII range a valid header name can ever occupy. + */ + private static final boolean[] TCHAR = new boolean[128]; + + static { + for (byte b : "!#$%&'*+-.^_`|~".getBytes(java.nio.charset.StandardCharsets.US_ASCII)) { + TCHAR[b] = true; + } + for (char c = '0'; c <= '9'; c++) TCHAR[c] = true; + for (char c = 'A'; c <= 'Z'; c++) TCHAR[c] = true; + for (char c = 'a'; c <= 'z'; c++) TCHAR[c] = true; + } + + private static boolean isTChar(byte b) { + return b >= 0 && b < 128 && TCHAR[b]; + } + private final int maxHeaderBufferSize; private final InetSocketAddress remoteAddress; private final SSLSocket sslSocket; @@ -65,6 +97,19 @@ public class RequestParser { this.buffer = new byte[Math.min(INITIAL_BUFFER_SIZE, maxHeaderBufferSize)]; } + /** + * Whether bytes from a previous {@link #parse} call are already buffered and ready to be + * consumed by the next call without reading anything further from the source — the HTTP + * pipelining case. The caller (the connection loop) uses this to decide whether it is safe + * to skip waiting for "the next request has started arriving": if bytes are already + * buffered, the next request has, by definition, already started (and may even be + * complete), so an idle-timeout wait on the underlying source would wait for bytes that + * were never going to arrive there — they are already here. + */ + boolean hasBufferedBytes() { + return bufLen > 0; + } + /** * Parses the next HTTP request from {@code in}. * @@ -75,9 +120,11 @@ public class RequestParser { * the same parser instance is reused after an error. * * @return the parsed {@link Request}, or {@code null} on clean EOF. - * @throws IOException on malformed headers or I/O failure. + * @throws MalformedRequestException if the request violates the HTTP/1.1 grammar or a + * configured safety limit — carries the exact status to respond with. + * @throws IOException on genuine I/O failure (socket reset, timeout). */ - public Request parse(InputStream in) throws IOException { + public Request parse(BufferedByteSource in) throws IOException { // Snapshot leftover bytes from the previous request, then reset immediately. // Any exception thrown below leaves bufBase/bufLen at 0 — safe state. int base = bufBase; @@ -94,7 +141,8 @@ public class RequestParser { System.arraycopy(buffer, base, buffer, 0, totalRead); base = 0; } else if (buffer.length >= maxHeaderBufferSize) { - throw new IOException("Request headers exceed " + maxHeaderBufferSize + " bytes"); + throw new MalformedRequestException(431, + "Request headers exceed " + maxHeaderBufferSize + " bytes"); } else { buffer = Arrays.copyOf(buffer, Math.min(buffer.length * 2, maxHeaderBufferSize)); } @@ -107,20 +155,22 @@ public class RequestParser { } if (totalRead <= 0) return null; if (headerEndIdx == -1) { - throw new IOException("Request headers exceed " + maxHeaderBufferSize + " bytes"); + throw new MalformedRequestException(431, + "Request headers exceed " + maxHeaderBufferSize + " bytes"); } // ── Request line ───────────────────────────────────────────────────── int methodEnd = find(buffer, base, headerEndIdx, (byte) ' '); - if (methodEnd == -1) throw new IOException("Invalid request line (method)"); + if (methodEnd == -1) throw new MalformedRequestException(400, "Invalid request line (method)"); + if (methodEnd == base) throw new MalformedRequestException(400, "Missing HTTP method"); HttpMethod method = HttpMethod.fromBytes(buffer, base, methodEnd - base); - if (method == null) throw new IOException("Unsupported HTTP method"); + if (method == null) throw new MalformedRequestException(501, "Unsupported HTTP method"); int pathStart = methodEnd + 1; int pathEnd = find(buffer, pathStart, headerEndIdx, (byte) ' '); - if (pathEnd == -1) throw new IOException("Invalid request line (path)"); + if (pathEnd == -1) throw new MalformedRequestException(400, "Invalid request line (path)"); int queryMark = find(buffer, pathStart, pathEnd, (byte) '?'); FastPathViews.RequestByteView pathView = new FastPathViews.RequestByteView(buffer, pathStart, @@ -131,7 +181,14 @@ public class RequestParser { int protocolStart = pathEnd + 1; int protocolEnd = find(buffer, protocolStart, headerEndIdx, (byte) '\r'); - if (protocolEnd == -1) throw new IOException("Invalid request line (protocol)"); + if (protocolEnd == -1) throw new MalformedRequestException(400, "Invalid request line (protocol)"); + + // EX-08: the request line itself (method SP target SP version) is bounded separately + // from the overall header-block size, so an oversized request line gets its own, + // specific rejection rather than being folded into the generic "headers too large" case. + if (protocolEnd - base > Http1Limits.MAX_REQUEST_LINE_LENGTH) { + throw new MalformedRequestException(431, "Request line exceeds " + Http1Limits.MAX_REQUEST_LINE_LENGTH + " bytes"); + } FastPathViews.RequestByteView protocolView = new FastPathViews.RequestByteView(buffer, protocolStart, protocolEnd - protocolStart); @@ -140,27 +197,98 @@ public class RequestParser { int sectionStart = find(buffer, protocolEnd, headerEndIdx, (byte) '\n') + 1; int current = sectionStart; - long contentLength = 0; - boolean isChunked = false; + long contentLength = -1; + boolean contentLengthSeen = false; + boolean transferEncodingSeen = false; + boolean transferEncodingChunked = false; + int headerCount = 0; while (current < headerEndIdx) { + // EX-18 (obs-fold): a header line MUST NOT begin with whitespace. RFC 9112 §5.2 + // deprecates line folding and treating a folded continuation as part of the + // previous header's value is a known request-smuggling vector. + byte first = buffer[current]; + if (first == ' ' || first == '\t') { + throw new MalformedRequestException(400, "Obsolete line folding is not supported"); + } + int lineEnd = find(buffer, current, headerEndIdx + 1, (byte) '\r'); if (lineEnd == -1 || lineEnd == current) break; - int colon = find(buffer, current, lineEnd, (byte) ':'); - if (colon != -1) { - int valueStart = colon + 1; - while (valueStart < lineEnd && buffer[valueStart] == ' ') valueStart++; + // EX-18: verify the '\r' is immediately followed by '\n' instead of blindly + // advancing past two bytes — a bare '\r' not followed by '\n' desynchronizes the + // parse and is a known bare-CR smuggling surface. Safe to read lineEnd+1: lineEnd + // is at most headerEndIdx, and findEndOfHeader already guaranteed 4 readable bytes + // (\r\n\r\n) starting at headerEndIdx. + if (buffer[lineEnd + 1] != '\n') { + throw new MalformedRequestException(400, "Malformed line terminator (bare CR)"); + } - if (equalsIgnoreCase(buffer, current, colon, "content-length")) { - contentLength = parseLong(buffer, valueStart, lineEnd); - } else if (equalsIgnoreCase(buffer, current, colon, "transfer-encoding")) { - isChunked = equalsIgnoreCase(buffer, valueStart, lineEnd, "chunked"); + if (++headerCount > Http1Limits.MAX_HEADER_COUNT) { + throw new MalformedRequestException(431, "Too many headers"); + } + + int colon = find(buffer, current, lineEnd, (byte) ':'); + if (colon == -1) { + throw new MalformedRequestException(400, "Header line missing ':'"); + } + if (colon - current > Http1Limits.MAX_HEADER_NAME_LENGTH) { + throw new MalformedRequestException(431, "Header name exceeds " + Http1Limits.MAX_HEADER_NAME_LENGTH + " bytes"); + } + for (int i = current; i < colon; i++) { + if (!isTChar(buffer[i])) { + throw new MalformedRequestException(400, "Invalid header name character"); } } + + int valueStart = colon + 1; + while (valueStart < lineEnd && buffer[valueStart] == ' ') valueStart++; + if (lineEnd - valueStart > Http1Limits.MAX_HEADER_VALUE_LENGTH) { + throw new MalformedRequestException(431, "Header value exceeds " + Http1Limits.MAX_HEADER_VALUE_LENGTH + " bytes"); + } + + if (equalsIgnoreCase(buffer, current, colon, "content-length")) { + // EX-03: strict, overflow-safe parsing — replaces the old digit-skipping + // parseLong, which silently accepted "5abc" as 5 and "-1" as 1. + long parsed = parseContentLengthStrict(buffer, valueStart, lineEnd); + // Multiple Content-Length lines with differing values is itself a smuggling + // primitive (EX-02); identical repeated values are tolerated (RFC 9110 §8.6 + // permits a recipient to treat that as one value). + if (contentLengthSeen && parsed != contentLength) { + throw new MalformedRequestException(400, "Conflicting Content-Length values"); + } + contentLength = parsed; + contentLengthSeen = true; + } else if (equalsIgnoreCase(buffer, current, colon, "transfer-encoding")) { + transferEncodingSeen = true; + // Correctness fix found while implementing EX-02 in this exact code path + // (registered as EX-35): the old check required the WHOLE value to equal + // "chunked", so "gzip, chunked" — valid per RFC 9112 §6.1, where chunked need + // only be the FINAL coding — was silently treated as not chunked at all, + // corrupting the message boundary. Fixed by inspecting only the last token. + transferEncodingChunked = isFinalCodingChunked(buffer, valueStart, lineEnd); + } current = lineEnd + 2; } + // EX-02 (RFC 9112 §6.1): a request with both Content-Length and Transfer-Encoding + // MUST be treated as an error by an origin server — this is the canonical CL.TE/TE.CL + // smuggling vector. Checked once both headers are known, regardless of the order they + // appeared in, so ordering games cannot bypass it. + if (contentLengthSeen && transferEncodingSeen) { + throw new MalformedRequestException(400, "Content-Length and Transfer-Encoding both present"); + } + boolean isChunked; + if (transferEncodingSeen) { + if (!transferEncodingChunked) { + throw new MalformedRequestException(501, "Unsupported Transfer-Encoding"); + } + isChunked = true; + } else { + isChunked = false; + if (!contentLengthSeen) contentLength = 0; + } + headerMap.reset(buffer, sectionStart, headerEndIdx); // ── Body / pipelining accounting ───────────────────────────────────── @@ -219,12 +347,54 @@ public class RequestParser { return true; } - private static long parseLong(byte[] buf, int start, int end) { + /** + * Strict, overflow-safe {@code Content-Length} parsing ({@code EX-03}). Rejects: an empty + * value, any non-digit byte (including a leading {@code +}/{@code -}, which are not + * digits), more than 19 digits (the longest possible {@code Long.MAX_VALUE}), arithmetic + * overflow past {@code Long.MAX_VALUE}, and a value above + * {@link Http1Limits#MAX_CONTENT_LENGTH}. The pre-existing {@code parseLong} silently + * skipped any non-digit character instead of rejecting it — {@code "5abc"} parsed as + * {@code 5} and {@code "-1"} parsed as {@code 1}. + */ + private static long parseContentLengthStrict(byte[] buf, int start, int end) throws MalformedRequestException { + int len = end - start; + if (len == 0) throw new MalformedRequestException(400, "Empty Content-Length value"); + if (len > 19) throw new MalformedRequestException(400, "Content-Length value too long"); long value = 0; for (int i = start; i < end; i++) { byte c = buf[i]; - if (c >= '0' && c <= '9') value = value * 10 + (c - '0'); + if (c < '0' || c > '9') { + throw new MalformedRequestException(400, "Malformed Content-Length value"); + } + int digit = c - '0'; + if (value > (Long.MAX_VALUE - digit) / 10) { + throw new MalformedRequestException(400, "Content-Length overflow"); + } + value = value * 10 + digit; + } + if (value > Http1Limits.MAX_CONTENT_LENGTH) { + throw new MalformedRequestException(413, "Content-Length exceeds configured maximum"); } return value; } -} \ No newline at end of file + + /** + * RFC 9112 §6.1: when {@code Transfer-Encoding} lists multiple codings + * ({@code "gzip, chunked"}), {@code chunked} MUST be the final one for the message to be + * self-delimiting. Returns whether the last comma-separated token in {@code [start, end)} + * is exactly {@code "chunked"} (case-insensitive), ignoring surrounding whitespace around + * that token. Registered as {@code EX-35}: the previous whole-value comparison silently + * misclassified any multi-coding value as non-chunked. + */ + private static boolean isFinalCodingChunked(byte[] buf, int start, int end) { + int e = end; + while (e > start && (buf[e - 1] == ' ' || buf[e - 1] == '\t')) e--; + int lastComma = start - 1; + for (int i = start; i < e; i++) { + if (buf[i] == ',') lastComma = i; + } + int tokenStart = lastComma + 1; + while (tokenStart < e && (buf[tokenStart] == ' ' || buf[tokenStart] == '\t')) tokenStart++; + return equalsIgnoreCase(buf, tokenStart, e, "chunked"); + } +} diff --git a/flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java b/flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java new file mode 100644 index 0000000..c15b67d --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/exceptions/MalformedRequestException.java @@ -0,0 +1,24 @@ +package dev.relism.flash.exceptions; + +/** + * Thrown by the HTTP/1.1 parser when a request violates a protocol rule that must be rejected + * outright — most importantly the request-smuggling defenses of RFC 9112 §6.1 (see + * {@code EX-02}/{@code EX-03} in {@code flash/docs/http2/IMPLEMENTATION-PLAN.md}) and the hard + * safety limits in {@code Http1Limits} (see {@code EX-08}). + * + *

Distinct from {@link HttpException}, which a handler throws to describe an + * application-level failure and which is routed through the user's configured exception + * handler ({@code AbstractRouter.getExceptionHandler()}). A malformed request never reaches a + * handler, or middleware, or the user's exception handler at all: it is rejected by the + * transport itself, with a fixed, minimal, non-customizable response, and the connection is + * always closed afterwards — never kept alive. Keeping a connection alive after a rejected + * request is exactly the situation a smuggling attempt exploits (a rejected first request + * hiding a crafted second one in the same TCP stream), so the transport never offers that + * choice to user code. + */ +public class MalformedRequestException extends HttpException { + + public MalformedRequestException(int status, String message) { + super(status, message); + } +} diff --git a/flash/src/main/java/dev/relism/flash/extension/FlashConfiguration.java b/flash/src/main/java/dev/relism/flash/extension/FlashConfiguration.java index 5a8745c..14cdb7e 100644 --- a/flash/src/main/java/dev/relism/flash/extension/FlashConfiguration.java +++ b/flash/src/main/java/dev/relism/flash/extension/FlashConfiguration.java @@ -57,6 +57,58 @@ public class FlashConfiguration { @Builder.Default int wsFrameBufferSize = 64 * 1024; + /** + * Maximum time, in milliseconds, allowed for a request's headers to be fully read once the + * first byte of it has arrived. Bounds the classic slowloris attack: a peer that trickles + * one header byte every few seconds forever. Enforced by an absolute deadline + * (see {@code dev.relism.flash.transport.BufferedByteSource}), not merely a per-read socket + * timeout — a per-read timeout alone never trips as long as each individual read succeeds + * within the window, no matter how long the overall header block takes. Default: 10 000 + * ({@code EX-07}). + */ + @Builder.Default + int headerReadTimeoutMs = 10_000; + + /** + * Maximum time, in milliseconds, a keep-alive connection may sit idle waiting for its next + * request before being closed. More generous than {@link #headerReadTimeoutMs} because an + * idle keep-alive connection is normal, expected behaviour, not an attack in progress — the + * tighter bound applies only once bytes have actually started arriving. Default: 60 000 + * ({@code EX-07}). + */ + @Builder.Default + int idleKeepAliveTimeoutMs = 60_000; + + /** + * Maximum time, in milliseconds, a request's body may take to be fully read (by the handler + * or by the automatic drain after it returns) once headers are parsed. Default: 30 000 + * ({@code EX-07}). + */ + @Builder.Default + int bodyReadTimeoutMs = 30_000; + + /** + * Maximum time, in milliseconds, {@link dev.relism.flash.ServerHandle#stop()} waits for + * in-flight requests to finish after it stops accepting new connections, before force- + * closing whatever remains. Default: 15 000 ({@code EX-32} — the graceful two-stage + * shutdown this bounds is wired up starting Phase 2). + */ + @Builder.Default + int shutdownDrainTimeoutMs = 15_000; + + /** + * Whether this server will ever negotiate HTTP/2. Default {@code false}: until the h2 + * connection state machine exists (Phase 8) there is nothing to negotiate into, so this + * flag currently only gates the h2c cleartext-preface detection + * ({@code dev.relism.flash.transport.ProtocolNegotiator}) — skipping it entirely keeps + * plaintext connections byte-for-byte identical to pre-HTTP/2 Flash when left at its + * default. TLS/ALPN connections are always detected accurately regardless of this flag + * (that costs nothing — see {@code ProtocolNegotiator}'s Javadoc) but are cleanly rejected + * rather than served until the phases that implement HTTP/2 land. + */ + @Builder.Default + boolean http2Enabled = false; + /** One bind target: a TCP port, an optional bind host (default: all interfaces), and optional TLS. */ public record Listener(int port, String host, TlsConfig tls) { public Listener(int port) { this(port, null, null); } 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 c01b21c..61fb98e 100644 --- a/flash/src/main/java/dev/relism/flash/http/Http1Limits.java +++ b/flash/src/main/java/dev/relism/flash/http/Http1Limits.java @@ -24,8 +24,15 @@ public final class Http1Limits { * resource-exhaustion vector for any code path that pre-sizes a buffer from it. Requests * declaring a length above this are rejected with {@code 413 Payload Too Large} before any * body byte is read. + * + *

4 GiB — generous enough for legitimate large uploads (Flash is a general-purpose + * server, not an API-only framework with a tiny default), while still bounding a hostile + * peer to a finite, known-in-advance number rather than the effectively unbounded + * {@code Long.MAX_VALUE} the parser accepted before this limit existed. Comfortably above + * {@code Integer.MAX_VALUE} (~2.1 billion) so legitimate very-large declared lengths are + * not confused with the int-overflow bug this same fix (EX-03) also closes. */ - public static final long MAX_CONTENT_LENGTH = 100L * 1024 * 1024; + public static final long MAX_CONTENT_LENGTH = 4L * 1024 * 1024 * 1024; /** * Maximum number of header lines accepted in a single request. Without this bound, a @@ -55,4 +62,38 @@ public final class Http1Limits { * into the generic header-block-too-large case. */ public static final int MAX_REQUEST_LINE_LENGTH = 8_192; + + /** + * Maximum size, in bytes, of a single {@code Transfer-Encoding: chunked} chunk. + * {@code ChunkedInputStream.readChunkSize} previously accepted any value up to 2 GiB before + * rejecting it; a hostile peer can advertise a huge chunk size and then trickle bytes, + * forcing the connection to stay open far longer than any legitimate chunk would need + * (bounded separately by {@code bodyReadTimeoutMs}, but this limit catches the size claim + * itself before that timeout would). + */ + public static final long MAX_CHUNK_SIZE = 16L * 1024 * 1024; + + /** + * Maximum length, in bytes, of the chunk-extension section (the optional + * {@code ;name=value} data after a chunk size and before its CRLF, RFC 9112 §7.1.1). Flash + * does not interpret chunk extensions; without a bound, a peer could send an arbitrarily + * long extension on every chunk purely to waste CPU discarding it. + */ + public static final int MAX_CHUNK_EXT_LENGTH = 256; + + /** + * Maximum number of chunks accepted in a single request body. Without this bound, a peer + * can send an unbounded number of minimal (or zero-length) chunks, each cheap individually + * but collectively forcing unbounded per-chunk framing work — a "death by a thousand + * chunks" variant of a slow-body attack. + */ + public static final int MAX_CHUNKS_PER_BODY = 100_000; + + /** + * Maximum number of trailer header lines accepted after the final chunk of a chunked body + * (RFC 9112 §7.1.2). Bounded for the same reason {@link #MAX_HEADER_COUNT} bounds the + * regular header section; trailer values are separately bounded by + * {@link #MAX_HEADER_VALUE_LENGTH}. + */ + public static final int MAX_TRAILER_COUNT = 50; } diff --git a/flash/src/main/java/dev/relism/flash/http/HttpStatus.java b/flash/src/main/java/dev/relism/flash/http/HttpStatus.java index 58bafba..bb502c4 100644 --- a/flash/src/main/java/dev/relism/flash/http/HttpStatus.java +++ b/flash/src/main/java/dev/relism/flash/http/HttpStatus.java @@ -37,24 +37,42 @@ public enum HttpStatus { CONFLICT (409, "Conflict"), GONE (410, "Gone"), LENGTH_REQUIRED (411, "Length Required"), + PRECONDITION_FAILED (412, "Precondition Failed"), PAYLOAD_TOO_LARGE (413, "Payload Too Large"), URI_TOO_LONG (414, "URI Too Long"), UNSUPPORTED_MEDIA_TYPE (415, "Unsupported Media Type"), + RANGE_NOT_SATISFIABLE (416, "Range Not Satisfiable"), + EXPECTATION_FAILED (417, "Expectation Failed"), + MISDIRECTED_REQUEST (421, "Misdirected Request"), UNPROCESSABLE_ENTITY (422, "Unprocessable Entity"), TOO_MANY_REQUESTS (429, "Too Many Requests"), + REQUEST_HEADER_FIELDS_TOO_LARGE (431, "Request Header Fields Too Large"), // 5xx INTERNAL_SERVER_ERROR (500, "Internal Server Error"), NOT_IMPLEMENTED (501, "Not Implemented"), BAD_GATEWAY (502, "Bad Gateway"), SERVICE_UNAVAILABLE (503, "Service Unavailable"), - GATEWAY_TIMEOUT (504, "Gateway Timeout"); + GATEWAY_TIMEOUT (504, "Gateway Timeout"), + HTTP_VERSION_NOT_SUPPORTED (505, "HTTP Version Not Supported"), + INSUFFICIENT_STORAGE (507, "Insufficient Storage"), + NETWORK_AUTHENTICATION_REQUIRED (511, "Network Authentication Required"); - private static final int MAX_STATUS_CODE = 504; - private static final byte[][] INDEX = new byte[MAX_STATUS_CODE + 1][]; - private static final String[] REASONS = new String[MAX_STATUS_CODE + 1]; + // EX-17: the bound used to be the hand-maintained constant 504, which silently threw + // ArrayIndexOutOfBoundsException from this static initializer the moment any constant + // above it (421, 431, 505, 507, 511 — several of which HTTP/2 needs, see MISDIRECTED_REQUEST + // and REQUEST_HEADER_FIELDS_TOO_LARGE above) was added. Computed from values() instead, so + // adding a status code can never silently break class loading again. + private static final int MAX_STATUS_CODE; + private static final byte[][] INDEX; + private static final String[] REASONS; static { + int max = 0; + for (HttpStatus s : values()) max = Math.max(max, s.code); + MAX_STATUS_CODE = max; + INDEX = new byte[MAX_STATUS_CODE + 1][]; + REASONS = new String[MAX_STATUS_CODE + 1]; for (HttpStatus s : values()) { INDEX[s.code] = s.bytes; REASONS[s.code] = s.reason; diff --git a/flash/src/main/java/dev/relism/flash/tls/TlsConfig.java b/flash/src/main/java/dev/relism/flash/tls/TlsConfig.java index 16935c2..5006019 100644 --- a/flash/src/main/java/dev/relism/flash/tls/TlsConfig.java +++ b/flash/src/main/java/dev/relism/flash/tls/TlsConfig.java @@ -14,6 +14,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.security.GeneralSecurityException; import java.security.KeyStore; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; /** * Declarative TLS configuration for a {@link dev.relism.flash.extension.FlashConfiguration.Listener}. @@ -50,6 +53,309 @@ public final class TlsConfig { private static final String[] SECURE_PROTOCOLS = { "TLSv1.3", "TLSv1.2" }; + /** + * {@code EX-31}: RFC 9113 §9.2.2 requires that an HTTP/2 endpoint MUST NOT use any of these + * cipher suites over TLS 1.2 (the list is unchanged from RFC 7540 Appendix A, which 9113 + * carries forward verbatim), and that it MUST support at least + * {@code TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}. TLS 1.3 is unaffected: none of its cipher + * suites (the {@code TLS_AES_*}/{@code TLS_CHACHA20_*} identifiers) appear here, since + * TLS 1.3 removed static/non-ephemeral key exchange and CBC-mode ciphers entirely — the + * exact property this blocklist exists to enforce for TLS 1.2. + * + *

Transcribed from Go's {@code golang.org/x/net/http2} {@code isBadCipher} table + * (BSD-licensed, itself an implementation of this exact RFC 9113 requirement, cross-checked + * against the IANA TLS Cipher Suite registry) rather than by hand from the RFC text, for the + * same reason Appendix D of {@code flash/docs/http2/IMPLEMENTATION-PLAN.md} insists the HPACK + * static table be transcribed from the RFC directly and verified: a transcription error in a + * ~280-entry list is easy to make and easy to miss, and here the failure mode is silently + * permitting a cipher suite RFC 9113 requires rejecting. Built once, in a static + * initializer (R4) — never reconstructed per connection. + */ + private static final Set TLS12_H2_BLOCKED_CIPHERS = Set.of( + "TLS_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA", + "TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA", + "TLS_DHE_DSS_WITH_AES_128_CBC_SHA", + "TLS_DHE_DSS_WITH_AES_128_CBC_SHA256", + "TLS_DHE_DSS_WITH_AES_256_CBC_SHA", + "TLS_DHE_DSS_WITH_AES_256_CBC_SHA256", + "TLS_DHE_DSS_WITH_ARIA_128_CBC_SHA256", + "TLS_DHE_DSS_WITH_ARIA_256_CBC_SHA384", + "TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DHE_DSS_WITH_DES_CBC_SHA", + "TLS_DHE_DSS_WITH_SEED_CBC_SHA", + "TLS_DHE_PSK_WITH_3DES_EDE_CBC_SHA", + "TLS_DHE_PSK_WITH_AES_128_CBC_SHA", + "TLS_DHE_PSK_WITH_AES_128_CBC_SHA256", + "TLS_DHE_PSK_WITH_AES_256_CBC_SHA", + "TLS_DHE_PSK_WITH_AES_256_CBC_SHA384", + "TLS_DHE_PSK_WITH_ARIA_128_CBC_SHA256", + "TLS_DHE_PSK_WITH_ARIA_256_CBC_SHA384", + "TLS_DHE_PSK_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_PSK_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_DHE_PSK_WITH_NULL_SHA", + "TLS_DHE_PSK_WITH_NULL_SHA256", + "TLS_DHE_PSK_WITH_NULL_SHA384", + "TLS_DHE_PSK_WITH_RC4_128_SHA", + "TLS_DHE_RSA_EXPORT_WITH_DES40_CBC_SHA", + "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256", + "TLS_DHE_RSA_WITH_ARIA_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_ARIA_256_CBC_SHA384", + "TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DHE_RSA_WITH_DES_CBC_SHA", + "TLS_DHE_RSA_WITH_SEED_CBC_SHA", + "TLS_DH_DSS_EXPORT_WITH_DES40_CBC_SHA", + "TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA", + "TLS_DH_DSS_WITH_AES_128_CBC_SHA", + "TLS_DH_DSS_WITH_AES_128_CBC_SHA256", + "TLS_DH_DSS_WITH_AES_128_GCM_SHA256", + "TLS_DH_DSS_WITH_AES_256_CBC_SHA", + "TLS_DH_DSS_WITH_AES_256_CBC_SHA256", + "TLS_DH_DSS_WITH_AES_256_GCM_SHA384", + "TLS_DH_DSS_WITH_ARIA_128_CBC_SHA256", + "TLS_DH_DSS_WITH_ARIA_128_GCM_SHA256", + "TLS_DH_DSS_WITH_ARIA_256_CBC_SHA384", + "TLS_DH_DSS_WITH_ARIA_256_GCM_SHA384", + "TLS_DH_DSS_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DH_DSS_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DH_DSS_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_DH_DSS_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DH_DSS_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DH_DSS_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_DH_DSS_WITH_DES_CBC_SHA", + "TLS_DH_DSS_WITH_SEED_CBC_SHA", + "TLS_DH_RSA_EXPORT_WITH_DES40_CBC_SHA", + "TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_DH_RSA_WITH_AES_128_CBC_SHA", + "TLS_DH_RSA_WITH_AES_128_CBC_SHA256", + "TLS_DH_RSA_WITH_AES_128_GCM_SHA256", + "TLS_DH_RSA_WITH_AES_256_CBC_SHA", + "TLS_DH_RSA_WITH_AES_256_CBC_SHA256", + "TLS_DH_RSA_WITH_AES_256_GCM_SHA384", + "TLS_DH_RSA_WITH_ARIA_128_CBC_SHA256", + "TLS_DH_RSA_WITH_ARIA_128_GCM_SHA256", + "TLS_DH_RSA_WITH_ARIA_256_CBC_SHA384", + "TLS_DH_RSA_WITH_ARIA_256_GCM_SHA384", + "TLS_DH_RSA_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DH_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DH_RSA_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_DH_RSA_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DH_RSA_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DH_RSA_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_DH_RSA_WITH_DES_CBC_SHA", + "TLS_DH_RSA_WITH_SEED_CBC_SHA", + "TLS_DH_anon_EXPORT_WITH_DES40_CBC_SHA", + "TLS_DH_anon_EXPORT_WITH_RC4_40_MD5", + "TLS_DH_anon_WITH_3DES_EDE_CBC_SHA", + "TLS_DH_anon_WITH_AES_128_CBC_SHA", + "TLS_DH_anon_WITH_AES_128_CBC_SHA256", + "TLS_DH_anon_WITH_AES_128_GCM_SHA256", + "TLS_DH_anon_WITH_AES_256_CBC_SHA", + "TLS_DH_anon_WITH_AES_256_CBC_SHA256", + "TLS_DH_anon_WITH_AES_256_GCM_SHA384", + "TLS_DH_anon_WITH_ARIA_128_CBC_SHA256", + "TLS_DH_anon_WITH_ARIA_128_GCM_SHA256", + "TLS_DH_anon_WITH_ARIA_256_CBC_SHA384", + "TLS_DH_anon_WITH_ARIA_256_GCM_SHA384", + "TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DH_anon_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_DH_anon_WITH_DES_CBC_SHA", + "TLS_DH_anon_WITH_RC4_128_MD5", + "TLS_DH_anon_WITH_SEED_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_ARIA_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_ARIA_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_NULL_SHA", + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", + "TLS_ECDHE_PSK_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_PSK_WITH_ARIA_128_CBC_SHA256", + "TLS_ECDHE_PSK_WITH_ARIA_256_CBC_SHA384", + "TLS_ECDHE_PSK_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDHE_PSK_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDHE_PSK_WITH_NULL_SHA", + "TLS_ECDHE_PSK_WITH_NULL_SHA256", + "TLS_ECDHE_PSK_WITH_NULL_SHA384", + "TLS_ECDHE_PSK_WITH_RC4_128_SHA", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_ARIA_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_ARIA_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_NULL_SHA", + "TLS_ECDHE_RSA_WITH_RC4_128_SHA", + "TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDH_ECDSA_WITH_ARIA_128_CBC_SHA256", + "TLS_ECDH_ECDSA_WITH_ARIA_128_GCM_SHA256", + "TLS_ECDH_ECDSA_WITH_ARIA_256_CBC_SHA384", + "TLS_ECDH_ECDSA_WITH_ARIA_256_GCM_SHA384", + "TLS_ECDH_ECDSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDH_ECDSA_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_ECDH_ECDSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDH_ECDSA_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_ECDH_ECDSA_WITH_NULL_SHA", + "TLS_ECDH_ECDSA_WITH_RC4_128_SHA", + "TLS_ECDH_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDH_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDH_RSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDH_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDH_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDH_RSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDH_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDH_RSA_WITH_ARIA_128_CBC_SHA256", + "TLS_ECDH_RSA_WITH_ARIA_128_GCM_SHA256", + "TLS_ECDH_RSA_WITH_ARIA_256_CBC_SHA384", + "TLS_ECDH_RSA_WITH_ARIA_256_GCM_SHA384", + "TLS_ECDH_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDH_RSA_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_ECDH_RSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDH_RSA_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_ECDH_RSA_WITH_NULL_SHA", + "TLS_ECDH_RSA_WITH_RC4_128_SHA", + "TLS_ECDH_anon_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDH_anon_WITH_AES_128_CBC_SHA", + "TLS_ECDH_anon_WITH_AES_256_CBC_SHA", + "TLS_ECDH_anon_WITH_NULL_SHA", + "TLS_ECDH_anon_WITH_RC4_128_SHA", + "TLS_EMPTY_RENEGOTIATION_INFO_SCSV", + "TLS_KRB5_EXPORT_WITH_DES_CBC_40_MD5", + "TLS_KRB5_EXPORT_WITH_DES_CBC_40_SHA", + "TLS_KRB5_EXPORT_WITH_RC2_CBC_40_MD5", + "TLS_KRB5_EXPORT_WITH_RC2_CBC_40_SHA", + "TLS_KRB5_EXPORT_WITH_RC4_40_MD5", + "TLS_KRB5_EXPORT_WITH_RC4_40_SHA", + "TLS_KRB5_WITH_3DES_EDE_CBC_MD5", + "TLS_KRB5_WITH_3DES_EDE_CBC_SHA", + "TLS_KRB5_WITH_DES_CBC_MD5", + "TLS_KRB5_WITH_DES_CBC_SHA", + "TLS_KRB5_WITH_IDEA_CBC_MD5", + "TLS_KRB5_WITH_IDEA_CBC_SHA", + "TLS_KRB5_WITH_RC4_128_MD5", + "TLS_KRB5_WITH_RC4_128_SHA", + "TLS_NULL_WITH_NULL_NULL", + "TLS_PSK_WITH_3DES_EDE_CBC_SHA", + "TLS_PSK_WITH_AES_128_CBC_SHA", + "TLS_PSK_WITH_AES_128_CBC_SHA256", + "TLS_PSK_WITH_AES_128_CCM", + "TLS_PSK_WITH_AES_128_CCM_8", + "TLS_PSK_WITH_AES_128_GCM_SHA256", + "TLS_PSK_WITH_AES_256_CBC_SHA", + "TLS_PSK_WITH_AES_256_CBC_SHA384", + "TLS_PSK_WITH_AES_256_CCM", + "TLS_PSK_WITH_AES_256_CCM_8", + "TLS_PSK_WITH_AES_256_GCM_SHA384", + "TLS_PSK_WITH_ARIA_128_CBC_SHA256", + "TLS_PSK_WITH_ARIA_128_GCM_SHA256", + "TLS_PSK_WITH_ARIA_256_CBC_SHA384", + "TLS_PSK_WITH_ARIA_256_GCM_SHA384", + "TLS_PSK_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_PSK_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_PSK_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_PSK_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_PSK_WITH_NULL_SHA", + "TLS_PSK_WITH_NULL_SHA256", + "TLS_PSK_WITH_NULL_SHA384", + "TLS_PSK_WITH_RC4_128_SHA", + "TLS_RSA_EXPORT_WITH_DES40_CBC_SHA", + "TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5", + "TLS_RSA_EXPORT_WITH_RC4_40_MD5", + "TLS_RSA_PSK_WITH_3DES_EDE_CBC_SHA", + "TLS_RSA_PSK_WITH_AES_128_CBC_SHA", + "TLS_RSA_PSK_WITH_AES_128_CBC_SHA256", + "TLS_RSA_PSK_WITH_AES_128_GCM_SHA256", + "TLS_RSA_PSK_WITH_AES_256_CBC_SHA", + "TLS_RSA_PSK_WITH_AES_256_CBC_SHA384", + "TLS_RSA_PSK_WITH_AES_256_GCM_SHA384", + "TLS_RSA_PSK_WITH_ARIA_128_CBC_SHA256", + "TLS_RSA_PSK_WITH_ARIA_128_GCM_SHA256", + "TLS_RSA_PSK_WITH_ARIA_256_CBC_SHA384", + "TLS_RSA_PSK_WITH_ARIA_256_GCM_SHA384", + "TLS_RSA_PSK_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_RSA_PSK_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_RSA_PSK_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_RSA_PSK_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_RSA_PSK_WITH_NULL_SHA", + "TLS_RSA_PSK_WITH_NULL_SHA256", + "TLS_RSA_PSK_WITH_NULL_SHA384", + "TLS_RSA_PSK_WITH_RC4_128_SHA", + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_RSA_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_AES_128_CBC_SHA256", + "TLS_RSA_WITH_AES_128_CCM", + "TLS_RSA_WITH_AES_128_CCM_8", + "TLS_RSA_WITH_AES_128_GCM_SHA256", + "TLS_RSA_WITH_AES_256_CBC_SHA", + "TLS_RSA_WITH_AES_256_CBC_SHA256", + "TLS_RSA_WITH_AES_256_CCM", + "TLS_RSA_WITH_AES_256_CCM_8", + "TLS_RSA_WITH_AES_256_GCM_SHA384", + "TLS_RSA_WITH_ARIA_128_CBC_SHA256", + "TLS_RSA_WITH_ARIA_128_GCM_SHA256", + "TLS_RSA_WITH_ARIA_256_CBC_SHA384", + "TLS_RSA_WITH_ARIA_256_GCM_SHA384", + "TLS_RSA_WITH_CAMELLIA_128_CBC_SHA", + "TLS_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_RSA_WITH_CAMELLIA_128_GCM_SHA256", + "TLS_RSA_WITH_CAMELLIA_256_CBC_SHA", + "TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_RSA_WITH_CAMELLIA_256_GCM_SHA384", + "TLS_RSA_WITH_DES_CBC_SHA", + "TLS_RSA_WITH_IDEA_CBC_SHA", + "TLS_RSA_WITH_NULL_MD5", + "TLS_RSA_WITH_NULL_SHA", + "TLS_RSA_WITH_NULL_SHA256", + "TLS_RSA_WITH_RC4_128_MD5", + "TLS_RSA_WITH_RC4_128_SHA", + "TLS_RSA_WITH_SEED_CBC_SHA", + "TLS_SRP_SHA_DSS_WITH_3DES_EDE_CBC_SHA", + "TLS_SRP_SHA_DSS_WITH_AES_128_CBC_SHA", + "TLS_SRP_SHA_DSS_WITH_AES_256_CBC_SHA", + "TLS_SRP_SHA_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_SRP_SHA_RSA_WITH_AES_128_CBC_SHA", + "TLS_SRP_SHA_RSA_WITH_AES_256_CBC_SHA", + "TLS_SRP_SHA_WITH_3DES_EDE_CBC_SHA", + "TLS_SRP_SHA_WITH_AES_128_CBC_SHA", + "TLS_SRP_SHA_WITH_AES_256_CBC_SHA" + ); + + /** RFC 9113 §9.2.2: an h2 endpoint MUST support this cipher suite. Not enforced (Flash + * cannot force a peer to offer it), but documented here as the fact {@link #applyTo}'s + * filtering relies on: filtering the blocklist above out of the JDK's default enabled set + * never removes this one, because it was never in the blocklist to begin with. */ + static final String REQUIRED_H2_CIPHER_SUITE = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"; + private final SSLContext context; private final boolean hardenDefaults; private final ClientAuth clientAuth; @@ -128,5 +434,32 @@ public final class TlsConfig { } if (clientAuth == ClientAuth.REQUIRE) socket.setNeedClientAuth(true); else if (clientAuth == ClientAuth.OPTIONAL) socket.setWantClientAuth(true); + + // EX-31: RFC 9113 §9.2.2 — when this listener can negotiate h2, the enabled cipher + // suite list must exclude every suite on the TLS 1.2 blocklist. TLS 1.3 suites are + // never in that list (see TLS12_H2_BLOCKED_CIPHERS's Javadoc) so this only narrows + // which TLS 1.2 suites remain available; TLS 1.3 is unaffected either way. + if (negotiatesH2()) { + String[] enabled = socket.getEnabledCipherSuites(); + List filtered = new ArrayList<>(enabled.length); + for (String suite : enabled) { + if (!TLS12_H2_BLOCKED_CIPHERS.contains(suite)) filtered.add(suite); + } + socket.setEnabledCipherSuites(filtered.toArray(new String[0])); + } + } + + /** + * Whether this listener's configured ALPN protocol list ({@link #applicationProtocols}) + * includes {@code "h2"}. Lets a caller (the connection runner, for logging; {@link #applyTo} + * itself, for {@code EX-31}'s cipher filtering) know a listener's h2 capability without + * duplicating the offered-protocols check. + */ + public boolean negotiatesH2() { + if (applicationProtocols == null) return false; + for (String protocol : applicationProtocols) { + if ("h2".equals(protocol)) return true; + } + return false; } } diff --git a/flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java b/flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java new file mode 100644 index 0000000..1396ace --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/transport/BufferedByteSource.java @@ -0,0 +1,264 @@ +package dev.relism.flash.transport; + +import java.io.IOException; +import java.io.InputStream; +import java.net.Socket; +import java.net.SocketTimeoutException; + +/** + * The single buffered view over one connection's inbound bytes, for the whole lifetime of the + * connection. Fixes {@code EX-10} (one syscall per byte in {@code ChunkedInputStream}) and + * gives {@link dev.relism.flash.transport.ProtocolNegotiator} a way to inspect the first bytes + * of a plaintext connection (the h2c preface) without consuming them. + * + *

Why this exists instead of {@link java.io.BufferedInputStream}

+ * A generic buffered stream would already fix the per-byte-syscall problem, but it cannot + * "un-consume" bytes without a fragile {@code mark()}/{@code reset()} dance, and it has no way + * to bound an individual read by an absolute wall-clock deadline (see below). This class is + * purpose-built for exactly the two things this connection loop needs beyond plain buffering: + * {@link #peek(byte[], int, int)} (look-ahead without consuming — used once, at connection + * start, for h2c prior-knowledge detection) and {@link #prependOnce(byte[], int, int)} + * (zero-allocation, zero-copy re-insertion of bytes the caller already read into its own + * buffer — used by {@code ChunkedInputStream} to hand back the header-parser's read-ahead + * bytes instead of the {@code SequenceInputStream}/{@code ByteArrayInputStream} wrapping this + * replaces). + * + *

Deadline, not {@code SO_TIMEOUT} alone

+ * {@link Socket#setSoTimeout(int)} bounds a single {@code read()} call, not a sequence of them — + * a peer that trickles one byte every 9 seconds never trips a 10-second {@code SO_TIMEOUT}, since + * each individual read succeeds within the window. {@link #setDeadline(long)} instead records an + * absolute {@link System#nanoTime()} deadline; every underlying socket read computes the + * remaining budget and hands exactly that to {@code setSoTimeout} before reading, so a + * {@link SocketTimeoutException} from an underlying read unambiguously means the deadline — + * not just one read — has been exceeded. This is what {@code EX-07} requires: "implement that + * deadline, do not rely on {@code setSoTimeout} alone." + * + *

Thread-safety

+ * Not thread-safe, by design — exactly one virtual thread ever owns a connection's inbound + * bytes at a time (the same invariant {@code RequestParser} and {@code ChunkedInputStream} + * already assume). + */ +public final class BufferedByteSource extends InputStream { + + /** + * Default internal buffer size. Matches the relay-buffer convention already used elsewhere + * in this codebase (the 8 KB {@code STREAM_RELAY_BUFFER} in {@code HttpServer}) rather than + * introducing a new tuning constant nothing has calibrated yet. + */ + public static final int DEFAULT_BUFFER_SIZE = 8192; + + private final InputStream in; + private final Socket socket; + private final byte[] buf; + private int pos; + private int limit; + + // One-shot prepend window (prependOnce) — consumed before buf and before any underlying + // read. References the caller's own array; never copies it. + private byte[] prefixBuf; + private int prefixPos; + private int prefixLen; + + private boolean deadlineActive; + private long deadlineNanos; + + public BufferedByteSource(InputStream in, Socket socket) { + this(in, socket, DEFAULT_BUFFER_SIZE); + } + + public BufferedByteSource(InputStream in, Socket socket, int bufferSize) { + this.in = in; + this.socket = socket; + this.buf = new byte[bufferSize]; + } + + // ── Deadline ───────────────────────────────────────────────────────────── + + /** + * Every underlying socket read performed after this call is bounded so that it cannot + * still be blocking past {@code deadlineNanoTime} (an absolute value comparable to + * {@link System#nanoTime()}). A read that would exceed the deadline throws + * {@link SocketTimeoutException} instead of blocking further. Bytes already sitting in the + * internal buffer or the prepend window are served immediately regardless of the deadline — + * only reads that would otherwise block on the network are bounded. + */ + public void setDeadline(long deadlineNanoTime) { + this.deadlineActive = true; + this.deadlineNanos = deadlineNanoTime; + } + + /** + * Removes the deadline and restores the socket to blocking indefinitely + * ({@code SO_TIMEOUT = 0}). Must be called before any read the caller wants to be + * unbounded (e.g. handing the connection off to a long-lived WebSocket session loop). + */ + public void clearDeadline() throws IOException { + this.deadlineActive = false; + socket.setSoTimeout(0); + } + + // ── InputStream ────────────────────────────────────────────────────────── + + @Override + public int read() throws IOException { + if (prefixLen > 0) { + prefixLen--; + return prefixBuf[prefixPos++] & 0xFF; + } + if (pos >= limit) { + int n = fillFromUnderlying(buf, 0, buf.length); + if (n <= 0) return -1; + pos = 0; + limit = n; + } + return buf[pos++] & 0xFF; + } + + @Override + public int read(byte[] dst, int off, int len) throws IOException { + if (len == 0) return 0; + if (prefixLen > 0) { + int n = Math.min(len, prefixLen); + System.arraycopy(prefixBuf, prefixPos, dst, off, n); + prefixPos += n; + prefixLen -= n; + return n; + } + if (pos < limit) { + int n = Math.min(len, limit - pos); + System.arraycopy(buf, pos, dst, off, n); + pos += n; + return n; + } + // Buffer empty. A large request (this is the path RequestParser's own bulk + // header-buffer fill takes) bypasses the internal buffer entirely — copying it through + // `buf` first would cost a full extra memcpy for no benefit, since the caller's own + // array is at least as large as what we would have buffered. + if (len >= buf.length) { + return fillFromUnderlying(dst, off, len); + } + int n = fillFromUnderlying(buf, 0, buf.length); + if (n <= 0) return n; + pos = 0; + limit = n; + int c = Math.min(len, limit); + System.arraycopy(buf, 0, dst, off, c); + pos = c; + return c; + } + + @Override + public long skip(long n) throws IOException { + if (n <= 0) return 0; + long remaining = n; + if (prefixLen > 0) { + int s = (int) Math.min(remaining, prefixLen); + prefixPos += s; + prefixLen -= s; + remaining -= s; + } + if (remaining > 0 && pos < limit) { + int s = (int) Math.min(remaining, limit - pos); + pos += s; + remaining -= s; + } + if (remaining > 0) { + remaining -= Math.max(0, in.skip(remaining)); + } + return n - remaining; + } + + @Override + public int available() { + return prefixLen + (limit - pos); + } + + @Override + public void close() throws IOException { + in.close(); + } + + // ── Peek and prepend — the two operations beyond InputStream's contract ──── + + /** + * Ensures up to {@code len} bytes are buffered and copies them into {@code dst} without + * advancing the read position — a subsequent {@code read()} still returns the same + * bytes. Blocks (bounded by the active deadline, if any) until {@code len} bytes are + * available or the underlying stream reaches EOF. Returns the number of bytes actually made + * available, which is less than {@code len} only at EOF. + * + *

Only valid before anything has been {@link #prependOnce prepended} — in practice this + * means it is only ever called once, by {@code ProtocolNegotiator}, at the very start of a + * connection before any other read. + * + * @throws IllegalArgumentException if {@code len} exceeds the internal buffer's capacity — + * this class cannot peek further ahead than it buffers. + */ + public int peek(byte[] dst, int off, int len) throws IOException { + if (len > buf.length) { + throw new IllegalArgumentException( + "peek length " + len + " exceeds buffer capacity " + buf.length); + } + if (prefixLen > 0) { + throw new IllegalStateException( + "peek() is only valid before any bytes have been prepended to this source"); + } + while (limit - pos < len) { + if (pos > 0) { + System.arraycopy(buf, pos, buf, 0, limit - pos); + limit -= pos; + pos = 0; + } + int n = fillFromUnderlying(buf, limit, buf.length - limit); + if (n <= 0) break; + limit += n; + } + int available = Math.min(len, limit - pos); + System.arraycopy(buf, pos, dst, off, available); + return available; + } + + /** + * Queues {@code len} bytes, starting at {@code off} in the caller-owned array {@code src}, + * to be served by the next reads before anything else — zero allocation and zero + * copy, since {@code src} is referenced directly, not duplicated. The caller must not + * mutate {@code src[off..off+len)} until the prefix is fully consumed. + * + *

Exactly one prefix may be pending at a time. This is intentional: it exists solely to + * hand {@code RequestParser}'s header-buffer read-ahead bytes to a fresh + * {@code ChunkedInputStream} at the start of a chunked body, a single well-defined moment + * per request — it is not a general-purpose pushback stack. + * + * @throws IllegalStateException if a prefix is already pending + */ + public void prependOnce(byte[] src, int off, int len) { + if (prefixLen > 0) { + throw new IllegalStateException("a prefix is already pending on this source"); + } + this.prefixBuf = src; + this.prefixPos = off; + this.prefixLen = len; + } + + // ── Internal fill ──────────────────────────────────────────────────────── + + /** + * The only place this class ever touches the underlying socket stream. When a deadline is + * 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. + */ + private int fillFromUnderlying(byte[] dst, int off, int len) throws IOException { + if (!deadlineActive) { + return in.read(dst, off, len); + } + long remainingNanos = deadlineNanos - System.nanoTime(); + 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); + return in.read(dst, off, len); + } +} diff --git a/flash/src/main/java/dev/relism/flash/transport/NegotiatedProtocol.java b/flash/src/main/java/dev/relism/flash/transport/NegotiatedProtocol.java new file mode 100644 index 0000000..7d477a4 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/transport/NegotiatedProtocol.java @@ -0,0 +1,10 @@ +package dev.relism.flash.transport; + +/** + * The result of {@link ProtocolNegotiator#negotiate}: which protocol a connection will speak, + * decided once, immediately after ALPN or the h2c preface is inspected, per R1. + */ +public enum NegotiatedProtocol { + HTTP_1_1, + H2 +} diff --git a/flash/src/main/java/dev/relism/flash/transport/ProtocolNegotiator.java b/flash/src/main/java/dev/relism/flash/transport/ProtocolNegotiator.java new file mode 100644 index 0000000..ecc96f9 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/transport/ProtocolNegotiator.java @@ -0,0 +1,67 @@ +package dev.relism.flash.transport; + +import javax.net.ssl.SSLSocket; + +import java.io.IOException; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +/** + * Decides, once per connection and before any request is parsed, whether the connection speaks + * HTTP/1.1 or HTTP/2 — the single seam R1 requires ("the protocol decision is made once, + * immediately after ALPN/preface detection"). + * + *

Two independent signals, in order: + *

    + *
  1. ALPN (TLS connections). If the socket is an {@link SSLSocket} and the TLS + * handshake already resolved {@code "h2"} as the application protocol, this connection is + * {@link NegotiatedProtocol#H2}. Anything else negotiated — {@code "http/1.1"}, no + * protocol at all (a peer that doesn't speak ALPN), or an empty string — is + * {@link NegotiatedProtocol#HTTP_1_1}. This costs nothing beyond a field read: ALPN is + * resolved during the handshake, which must already have completed (see + * {@code TlsConfig}'s Javadoc on why {@code startHandshake()} must be called explicitly + * before this method runs — {@code EX-30}).
    2. + *
  2. h2c prior knowledge (plaintext connections, RFC 9113 §3.4). The first 24 bytes of + * the connection are compared, without being consumed, against the client connection + * preface {@code "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"}. A match is + * {@link NegotiatedProtocol#H2}; anything else — including a partial match followed by + * EOF, or a preface look-alike that diverges partway through — is + * {@link NegotiatedProtocol#HTTP_1_1}. This is why {@link BufferedByteSource#peek} exists: + * the bytes must remain available for {@code RequestParser} if they turn out not to be an + * h2 preface after all.
    3. + *
+ * + *

This method reports the protocol accurately and unconditionally — it does not consult + * {@code FlashConfiguration.http2Enabled}. Gating whether an {@link NegotiatedProtocol#H2} + * result is honoured (versus cleanly rejected, which is all Phase 1 can do — there is no + * {@code Http2Connection} yet) and whether the h2c peek is even attempted for plaintext + * connections are both the caller's responsibility, so that this class stays a pure, + * directly-testable detector (see {@code ProtocolNegotiatorTest}). + */ +public final class ProtocolNegotiator { + + /** + * The HTTP/2 client connection preface (RFC 9113 §3.4) — precompiled once (R4), never + * reconstructed per connection. + */ + private static final byte[] H2C_PREFACE = + "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(StandardCharsets.US_ASCII); + + private ProtocolNegotiator() { + } + + public static NegotiatedProtocol negotiate(Socket socket, BufferedByteSource source) throws IOException { + if (socket instanceof SSLSocket ssl) { + String applicationProtocol = ssl.getApplicationProtocol(); + return "h2".equals(applicationProtocol) ? NegotiatedProtocol.H2 : NegotiatedProtocol.HTTP_1_1; + } + + byte[] probe = new byte[H2C_PREFACE.length]; + int n = source.peek(probe, 0, probe.length); + if (n == H2C_PREFACE.length && Arrays.equals(probe, H2C_PREFACE)) { + return NegotiatedProtocol.H2; + } + return NegotiatedProtocol.HTTP_1_1; + } +} diff --git a/flash/src/test/java/dev/relism/flash/ChunkedInputStreamTest.java b/flash/src/test/java/dev/relism/flash/ChunkedInputStreamTest.java index 632e4ae..d906590 100644 --- a/flash/src/test/java/dev/relism/flash/ChunkedInputStreamTest.java +++ b/flash/src/test/java/dev/relism/flash/ChunkedInputStreamTest.java @@ -1,5 +1,8 @@ package dev.relism.flash; +import dev.relism.flash.exceptions.MalformedRequestException; +import dev.relism.flash.http.Http1Limits; +import dev.relism.flash.transport.BufferedByteSource; import org.junit.jupiter.api.Test; import java.io.ByteArrayInputStream; @@ -10,9 +13,15 @@ import static org.junit.jupiter.api.Assertions.*; class ChunkedInputStreamTest { + // BufferedByteSource's Socket reference is only touched when a deadline is set — none of + // these tests set one, so `null` is safe here. + private static BufferedByteSource source(byte[] bytes) { + return new BufferedByteSource(new ByteArrayInputStream(bytes), null); + } + private static ChunkedInputStream wrap(String chunkedEncoded) { byte[] bytes = chunkedEncoded.getBytes(StandardCharsets.UTF_8); - return new ChunkedInputStream(new ByteArrayInputStream(bytes), null, 0, 0); + return new ChunkedInputStream(source(bytes), null, 0, 0); } private static String readAll(ChunkedInputStream in) throws IOException { @@ -105,7 +114,7 @@ class ChunkedInputStreamTest { // "5\r\nhello" in preBuf, "\r\n0\r\n\r\n" in socket byte[] preBuf = "5\r\nhello".getBytes(StandardCharsets.UTF_8); byte[] socket = "\r\n0\r\n\r\n".getBytes(StandardCharsets.UTF_8); - ChunkedInputStream in = new ChunkedInputStream(new ByteArrayInputStream(socket), preBuf, 0, preBuf.length); + ChunkedInputStream in = new ChunkedInputStream(source(socket), preBuf, 0, preBuf.length); assertEquals("hello", new String(in.readAllBytes(), StandardCharsets.UTF_8)); } @@ -114,7 +123,103 @@ class ChunkedInputStreamTest { byte[] preBuf = "XX2\r\nhi\r\n0\r\n\r\n".getBytes(StandardCharsets.UTF_8); // offset=2, len=preBuf.length-2 — skip "XX" ChunkedInputStream in = new ChunkedInputStream( - new ByteArrayInputStream(new byte[0]), preBuf, 2, preBuf.length - 2); + source(new byte[0]), preBuf, 2, preBuf.length - 2); assertEquals("hi", new String(in.readAllBytes(), StandardCharsets.UTF_8)); } + + // --- EX-10: no per-byte syscalls against the underlying stream ------------ + + /** Counts every {@code read} call that reaches the wrapped stream — i.e. every syscall. */ + private static final class CountingInputStream extends ByteArrayInputStream { + int reads = 0; + CountingInputStream(byte[] buf) { super(buf); } + @Override public synchronized int read() { reads++; return super.read(); } + @Override public synchronized int read(byte[] b, int off, int len) { reads++; return super.read(b, off, len); } + } + + @Test + void byteByByteRead_doesNotSyscallPerByte() throws IOException { + // 100 one-byte chunks — the pre-fix implementation would have issued one read() call + // per payload byte PLUS one per chunk-size digit PLUS two per chunk terminator PLUS + // two for the final trailer-section terminator: hundreds of underlying reads for 100 + // bytes of payload. Buffered, this must collapse to a small, buffer-size-bound count. + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 100; i++) sb.append("1\r\nx\r\n"); + sb.append("0\r\n\r\n"); + CountingInputStream counting = new CountingInputStream(sb.toString().getBytes(StandardCharsets.UTF_8)); + BufferedByteSource src = new BufferedByteSource(counting, null); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + + int total = 0; + while (in.read() != -1) total++; + + assertEquals(100, total); + // The whole message (700 bytes) fits in BufferedByteSource's default 8 KB buffer, so + // this must be exactly one underlying read — nowhere near "one per byte". + assertEquals(1, counting.reads); + } + + // --- EX-02/09 chunk safety limits ------------------------------------------ + + @Test + void chunkSizeAboveLimit_rejected() { + // MAX_CHUNK_SIZE is 16 MiB (0x1000000); one hex digit past that overflows the bound. + BufferedByteSource src = source("10000000\r\n".getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + assertThrows(MalformedRequestException.class, in::read); + } + + @Test + void tooManyHexDigits_rejected() { + BufferedByteSource src = source("00000000000000001\r\n".getBytes(StandardCharsets.UTF_8)); // 17 digits + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + assertThrows(MalformedRequestException.class, in::read); + } + + @Test + void chunkExtensionTooLong_rejected() { + String ext = ";" + "a".repeat(300); + BufferedByteSource src = source(("5" + ext + "\r\nhello\r\n0\r\n\r\n").getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + assertThrows(MalformedRequestException.class, in::read); + } + + @Test + void malformedChunkSize_rejected() { + BufferedByteSource src = source(";novalue\r\n".getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + assertThrows(MalformedRequestException.class, in::read); + } + + @Test + void bareChunkTerminator_rejected() { + // Declares 5 bytes but the terminator after them is not CRLF. + BufferedByteSource src = source("5\r\nhelloXX0\r\n\r\n".getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + assertThrows(MalformedRequestException.class, () -> in.readAllBytes()); + } + + @Test + void tooManyChunks_rejected413() { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < Http1Limits.MAX_CHUNKS_PER_BODY + 5; i++) sb.append("1\r\nx\r\n"); + sb.append("0\r\n\r\n"); + BufferedByteSource src = source(sb.toString().getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + MalformedRequestException e = assertThrows(MalformedRequestException.class, () -> { + while (in.read() != -1) { /* drain */ } + }); + assertEquals(413, e.status()); + } + + @Test + void tooManyTrailers_rejected431() { + StringBuilder sb = new StringBuilder("2\r\nhi\r\n0\r\n"); + for (int i = 0; i < Http1Limits.MAX_TRAILER_COUNT + 5; i++) sb.append("X-").append(i).append(": v\r\n"); + sb.append("\r\n"); + BufferedByteSource src = source(sb.toString().getBytes(StandardCharsets.UTF_8)); + ChunkedInputStream in = new ChunkedInputStream(src, null, 0, 0); + MalformedRequestException e = assertThrows(MalformedRequestException.class, in::readAllBytes); + assertEquals(431, e.status()); + } } diff --git a/flash/src/test/java/dev/relism/flash/HttpServerTimeoutTest.java b/flash/src/test/java/dev/relism/flash/HttpServerTimeoutTest.java new file mode 100644 index 0000000..f471736 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/HttpServerTimeoutTest.java @@ -0,0 +1,185 @@ +package dev.relism.flash; + +import dev.relism.flash.extension.FlashApp; +import dev.relism.flash.extension.FlashConfiguration; +import dev.relism.flash.tls.TestKeystores; +import dev.relism.flash.tls.TlsConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-07}: a socket-level {@code SO_TIMEOUT} alone never trips against a peer that keeps + * trickling bytes slower than the timeout window — each individual read still succeeds. These + * tests prove the absolute deadline in {@code dev.relism.flash.transport.BufferedByteSource} + * actually bounds the total time, not just each read. + */ +class HttpServerTimeoutTest { + + private FlashApp app; + + @AfterEach + void tearDown() { + if (app != null) app.stop(); + } + + private int freePort() throws Exception { + try (ServerSocket s = new ServerSocket(0)) { + return s.getLocalPort(); + } + } + + @Test + void slowlorisHeaderDribble_disconnectedWithinHeaderReadTimeout() throws Exception { + int headerTimeoutMs = 300; + int port = freePort(); + app = FlashApp.create(FlashConfiguration.builder() + .port(port).host("127.0.0.1") + .headerReadTimeoutMs(headerTimeoutMs) + .idleKeepAliveTimeoutMs(60_000) + .build()); + app.get("/", (req, res) -> "ok"); + app.start(); + + long start = System.nanoTime(); + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.setSoTimeout(5_000); + OutputStream out = socket.getOutputStream(); + // One byte of a request line, then silence — never completes the header block. + out.write('G'); + out.flush(); + + // The server must close its side within headerReadTimeoutMs (+ generous slack for + // scheduling). Detected as EOF (-1) or a reset when the client tries to read. + int result = socket.getInputStream().read(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + + assertEquals(-1, result, "server must close, not hang, after the header deadline"); + assertTrue(elapsedMs < 5_000, "must not have fallen back to the client's own SO_TIMEOUT"); + assertTrue(elapsedMs >= headerTimeoutMs - 50, + "must not close before the configured deadline (was " + elapsedMs + "ms)"); + } + } + + @Test + void idleKeepAliveConnection_disconnectedWithinIdleTimeout() throws Exception { + int idleTimeoutMs = 300; + int port = freePort(); + app = FlashApp.create(FlashConfiguration.builder() + .port(port).host("127.0.0.1") + .headerReadTimeoutMs(10_000) + .idleKeepAliveTimeoutMs(idleTimeoutMs) + .build()); + app.get("/", (req, res) -> "ok"); + app.start(); + + long start = System.nanoTime(); + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.setSoTimeout(5_000); + // Send nothing at all — a connection accepted and then left idle. + int result = socket.getInputStream().read(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + + assertEquals(-1, result); + assertTrue(elapsedMs < 5_000); + assertTrue(elapsedMs >= idleTimeoutMs - 50, "was " + elapsedMs + "ms"); + } + } + + @Test + void slowBodyDribble_disconnectedWithinBodyReadTimeout() throws Exception { + int bodyTimeoutMs = 300; + int port = freePort(); + app = FlashApp.create(FlashConfiguration.builder() + .port(port).host("127.0.0.1") + .headerReadTimeoutMs(10_000) + .idleKeepAliveTimeoutMs(10_000) + .bodyReadTimeoutMs(bodyTimeoutMs) + .build()); + app.post("/echo", (req, res) -> req.body().bytes()); + app.start(); + + long start = System.nanoTime(); + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.setSoTimeout(5_000); + OutputStream out = socket.getOutputStream(); + out.write(("POST /echo HTTP/1.1\r\nHost: localhost\r\nContent-Length: 100\r\n\r\n" + "x".repeat(5)) + .getBytes(StandardCharsets.UTF_8)); + out.flush(); + // Only 5 of the declared 100 bytes were sent; the remaining 95 never arrive. Whether + // the server responds with an error before closing or simply closes, *something* + // must happen within the body deadline rather than a hang until the client's own + // (much longer) timeout. + socket.getInputStream().read(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + + assertTrue(elapsedMs < 5_000, "must not have fallen back to the client's own SO_TIMEOUT"); + assertTrue(elapsedMs >= bodyTimeoutMs - 50, "was " + elapsedMs + "ms"); + } + } + + @Test + void tlsHandshakeNeverStarted_disconnectedWithinHeaderReadTimeout(@TempDir Path dir) throws Exception { + int headerTimeoutMs = 300; + Path ks = TestKeystores.build(dir, "timeout.p12", "changeit", + TestKeystores.Entry.of("only", "timeout.test")); + int port = freePort(); + app = FlashApp.create(FlashConfiguration.builder() + .port(port).host("127.0.0.1") + .tls(TlsConfig.keystore(ks, "changeit")) + .headerReadTimeoutMs(headerTimeoutMs) + .build()); + app.get("/", (req, res) -> "ok"); + app.start(); + + long start = System.nanoTime(); + // A plain socket that never speaks TLS at all — the server's explicit + // startHandshake() (EX-30) blocks waiting for a ClientHello that is never coming, + // and must be bounded by headerReadTimeoutMs rather than hanging forever. Whether the + // JSSE implementation sends a TLS alert record before closing or just closes outright + // is a JSSE implementation detail, not something this test should pin down — the + // property under test is purely the bound on wall-clock time. + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.setSoTimeout(5_000); + socket.getInputStream().read(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + + assertTrue(elapsedMs < 5_000, "must not have fallen back to the client's own SO_TIMEOUT"); + assertTrue(elapsedMs >= headerTimeoutMs - 50, "was " + elapsedMs + "ms"); + } + } + + @Test + void wellBehavedRequest_wellWithinTimeouts_unaffected() throws Exception { + int port = freePort(); + app = FlashApp.create(FlashConfiguration.builder() + .port(port).host("127.0.0.1") + .headerReadTimeoutMs(300) + .idleKeepAliveTimeoutMs(300) + .bodyReadTimeoutMs(300) + .build()); + app.get("/ping", (req, res) -> "pong"); + app.start(); + + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.setSoTimeout(5_000); + socket.getOutputStream().write( + "GET /ping HTTP/1.1\r\nHost: localhost\r\n\r\n".getBytes(StandardCharsets.UTF_8)); + socket.getOutputStream().flush(); + byte[] buf = new byte[4096]; + int n = socket.getInputStream().read(buf); + assertTrue(n > 0); + String response = new String(buf, 0, n, StandardCharsets.UTF_8); + assertTrue(response.startsWith("HTTP/1.1 200 OK")); + assertTrue(response.contains("pong")); + } + } +} diff --git a/flash/src/test/java/dev/relism/flash/RequestParserSecurityTest.java b/flash/src/test/java/dev/relism/flash/RequestParserSecurityTest.java new file mode 100644 index 0000000..4410a8d --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/RequestParserSecurityTest.java @@ -0,0 +1,194 @@ +package dev.relism.flash; + +import dev.relism.flash.exceptions.MalformedRequestException; +import dev.relism.flash.http.Http1Limits; +import dev.relism.flash.models.Request; +import dev.relism.flash.transport.BufferedByteSource; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * One test per rejection rule added in HTTP/2 plan Phase 1 (EX-02, EX-03, EX-08, EX-18, EX-35, + * EX-36), each asserting the specific status code {@link MalformedRequestException} carries — + * not merely that some exception was thrown. {@code HttpServer} always closes the connection + * after any of these (never keep-alive); that behaviour is exercised at the integration level + * by {@code HttpServerTest}. + */ +class RequestParserSecurityTest { + + private static BufferedByteSource source(byte[] bytes) { + return new BufferedByteSource(new ByteArrayInputStream(bytes), null); + } + + private static Request parse(String raw) throws IOException { + byte[] bytes = raw.replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8); + return new RequestParser().parse(source(bytes)); + } + + private static MalformedRequestException expect(String raw) { + return assertThrows(MalformedRequestException.class, () -> parse(raw)); + } + + // --- EX-02: Content-Length + Transfer-Encoding smuggling ------------------- + + @Test + void contentLengthAndTransferEncodingBothPresent_rejected400() { + MalformedRequestException e = expect( + "POST / HTTP/1.1\nHost: h\nContent-Length: 5\nTransfer-Encoding: chunked\n\nhello"); + assertEquals(400, e.status()); + } + + @Test + void contentLengthAndTransferEncodingBothPresent_rejectedRegardlessOfOrder() { + // The check must not be bypassable by which header appears first. + MalformedRequestException e = expect( + "POST / HTTP/1.1\nHost: h\nTransfer-Encoding: chunked\nContent-Length: 5\n\nhello"); + assertEquals(400, e.status()); + } + + @Test + void duplicateContentLength_conflictingValues_rejected400() { + MalformedRequestException e = expect( + "POST / HTTP/1.1\nHost: h\nContent-Length: 5\nContent-Length: 6\n\nhello!"); + assertEquals(400, e.status()); + } + + @Test + void duplicateContentLength_identicalValues_accepted() throws IOException { + Request r = parse("POST / HTTP/1.1\nHost: h\nContent-Length: 5\nContent-Length: 5\n\nhello"); + assertEquals(5L, r.body().contentLength()); + } + + @Test + void transferEncoding_finalCodingNotChunked_rejected501() { + MalformedRequestException e = expect( + "POST / HTTP/1.1\nHost: h\nTransfer-Encoding: gzip\n\n"); + assertEquals(501, e.status()); + } + + @Test + void transferEncoding_chunkedNotFinal_rejected501() { + // "chunked, gzip" — chunked must be the LAST coding (RFC 9112 §6.1). + MalformedRequestException e = expect( + "POST / HTTP/1.1\nHost: h\nTransfer-Encoding: chunked, gzip\n\n"); + assertEquals(501, e.status()); + } + + // --- EX-03: strict Content-Length parsing ----------------------------------- + + @Test + void contentLength_nonDigitSuffix_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nContent-Length: 5abc\n\n").status()); + } + + @Test + void contentLength_leadingPlus_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nContent-Length: +5\n\n").status()); + } + + @Test + void contentLength_leadingMinus_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nContent-Length: -1\n\n").status()); + } + + @Test + void contentLength_empty_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nContent-Length: \n\n").status()); + } + + @Test + void contentLength_overflowsLong_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nContent-Length: 99999999999999999999\n\n").status()); + } + + @Test + void contentLength_aboveConfiguredMax_rejected413() { + long tooLarge = Http1Limits.MAX_CONTENT_LENGTH + 1; + assertEquals(413, expect("POST / HTTP/1.1\nHost: h\nContent-Length: " + tooLarge + "\n\n").status()); + } + + // --- EX-08: header/request-line limits -------------------------------------- + + @Test + void tooManyHeaders_rejected431() { + StringBuilder sb = new StringBuilder("GET / HTTP/1.1\nHost: h\n"); + for (int i = 0; i < Http1Limits.MAX_HEADER_COUNT + 5; i++) sb.append("X-").append(i).append(": v\n"); + sb.append("\n"); + assertEquals(431, expect(sb.toString()).status()); + } + + @Test + void headerNameTooLong_rejected431() { + String name = "X-" + "a".repeat(Http1Limits.MAX_HEADER_NAME_LENGTH + 1); + assertEquals(431, expect("GET / HTTP/1.1\nHost: h\n" + name + ": v\n\n").status()); + } + + @Test + void headerValueTooLong_rejected431() { + String value = "a".repeat(Http1Limits.MAX_HEADER_VALUE_LENGTH + 1); + assertEquals(431, expect("GET / HTTP/1.1\nHost: h\nX-Big: " + value + "\n\n").status()); + } + + @Test + void requestLineTooLong_rejected431() { + String path = "/" + "a".repeat(Http1Limits.MAX_REQUEST_LINE_LENGTH + 1); + assertEquals(431, expect("GET " + path + " HTTP/1.1\nHost: h\n\n").status()); + } + + // --- EX-18: bare CR / obs-fold ----------------------------------------------- + + @Test + void bareLfInsteadOfCrlf_headerLine_rejected() { + // A '\r' not immediately followed by '\n' desynchronizes the parse. + byte[] raw = "GET / HTTP/1.1\r\nHost: h\r\r\n\r\n".getBytes(StandardCharsets.UTF_8); + assertThrows(MalformedRequestException.class, () -> new RequestParser().parse(source(raw))); + } + + @Test + void obsFold_leadingWhitespaceContinuation_rejected400() { + MalformedRequestException e = assertThrows(MalformedRequestException.class, () -> + new RequestParser().parse(source( + "GET / HTTP/1.1\r\nHost: h\r\n Folded: continuation\r\n\r\n" + .getBytes(StandardCharsets.UTF_8)))); + assertEquals(400, e.status()); + } + + // --- header name tchar validation -------------------------------------------- + + @Test + void headerNameWithSpace_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nBad Name: v\n\n").status()); + } + + @Test + void headerNameWithControlChar_rejected400() { + String prefix = "GET / HTTP/1.1\r\nHost: h\r\nBad"; + String suffix = "Name: v\r\n\r\n"; + byte[] prefixBytes = prefix.getBytes(StandardCharsets.ISO_8859_1); + byte[] suffixBytes = suffix.getBytes(StandardCharsets.ISO_8859_1); + byte[] raw = new byte[prefixBytes.length + 1 + suffixBytes.length]; + System.arraycopy(prefixBytes, 0, raw, 0, prefixBytes.length); + raw[prefixBytes.length] = 0x01; // control character -- not a valid tchar + System.arraycopy(suffixBytes, 0, raw, prefixBytes.length + 1, suffixBytes.length); + assertThrows(MalformedRequestException.class, () -> new RequestParser().parse(source(raw))); + } + + // --- EX-36: header line missing ':' ------------------------------------------- + + @Test + void headerLineMissingColon_rejected400() { + assertEquals(400, expect("GET / HTTP/1.1\nHost: h\nNotAHeader\n\n").status()); + } + + // --- request-line rejections still carry the right status -------------------- + + @Test + void emptyMethod_rejected400() { + assertEquals(400, expect(" / HTTP/1.1\nHost: h\n\n").status()); + } +} diff --git a/flash/src/test/java/dev/relism/flash/RequestParserTest.java b/flash/src/test/java/dev/relism/flash/RequestParserTest.java index 2c8dd79..5bb3df5 100644 --- a/flash/src/test/java/dev/relism/flash/RequestParserTest.java +++ b/flash/src/test/java/dev/relism/flash/RequestParserTest.java @@ -1,6 +1,8 @@ package dev.relism.flash; +import dev.relism.flash.exceptions.MalformedRequestException; import dev.relism.flash.models.Request; +import dev.relism.flash.transport.BufferedByteSource; import org.junit.jupiter.api.Test; import java.io.ByteArrayInputStream; @@ -14,9 +16,15 @@ class RequestParserTest { // --- helpers --- + // BufferedByteSource's Socket reference is only touched when a deadline is set — none of + // these tests set one, so `null` is safe here. + private static BufferedByteSource source(byte[] bytes) { + return new BufferedByteSource(new ByteArrayInputStream(bytes), null); + } + private static Request parse(String raw) throws IOException { byte[] bytes = raw.replace("\n", "\r\n").getBytes(StandardCharsets.UTF_8); - return new RequestParser().parse(new ByteArrayInputStream(bytes)); + return new RequestParser().parse(source(bytes)); } private static String req(String requestLine, String... headers) { @@ -70,7 +78,7 @@ class RequestParserTest { void body_parsed() throws IOException { String body = "hello body"; String raw = "POST / HTTP/1.1\r\nContent-Length: " + body.length() + "\r\n\r\n" + body; - Request r = new RequestParser().parse(new ByteArrayInputStream(raw.getBytes(StandardCharsets.UTF_8))); + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); assertNotNull(r); assertEquals(body, new String(r.body().bytes(), StandardCharsets.UTF_8)); } @@ -79,7 +87,7 @@ class RequestParserTest { void body_parsed_forQueryMethod() throws IOException { String body = "{\"filter\":\"active\"}"; String raw = "QUERY / HTTP/1.1\r\nContent-Type: application/json\r\nContent-Length: " + body.length() + "\r\n\r\n" + body; - Request r = new RequestParser().parse(new ByteArrayInputStream(raw.getBytes(StandardCharsets.UTF_8))); + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); assertNotNull(r); assertEquals(dev.relism.flash.http.HttpMethod.QUERY, r.method()); assertEquals(body, new String(r.body().bytes(), StandardCharsets.UTF_8)); @@ -95,34 +103,43 @@ class RequestParserTest { @Test void emptyInputStream_returnsNull() throws IOException { - assertNull(new RequestParser().parse(new ByteArrayInputStream(new byte[0]))); + assertNull(new RequestParser().parse(source(new byte[0]))); } @Test - void missingHeaderTerminator_throwsIOException() { - // Valid request line but stream ends before \r\n\r\n + void missingHeaderTerminator_throwsMalformedRequestException() { + // Valid request line but stream ends before \r\n\r\n. Previously a generic IOException; + // now the same typed rejection EX-08's over-limit case uses, since both mean "the + // header block could never be completed within the allowed buffer" (EX-08). byte[] raw = "GET / HTTP/1.1\r\nHost: localhost\r\n".getBytes(StandardCharsets.UTF_8); - assertThrows(IOException.class, () -> new RequestParser().parse(new ByteArrayInputStream(raw))); + assertThrows(MalformedRequestException.class, () -> new RequestParser().parse(source(raw))); } @Test - void unknownHttpMethod_throwsIOException() { - assertThrows(IOException.class, () -> parse(req("BREW /coffee HTTP/1.1", "Host: localhost"))); + void unknownHttpMethod_throwsMalformedRequestException() { + // RFC 9110 §9.1 SHOULD 501 an unrecognised method. + MalformedRequestException e = assertThrows(MalformedRequestException.class, + () -> parse(req("BREW /coffee HTTP/1.1", "Host: localhost"))); + assertEquals(501, e.status()); } @Test - void requestLine_noProtocol_throwsIOException() { + void requestLine_noProtocol_throwsMalformedRequestException() { // No space after path, parser cannot find protocol boundary - assertThrows(IOException.class, () -> parse(req("GET /noproto"))); + MalformedRequestException e = assertThrows(MalformedRequestException.class, + () -> parse(req("GET /noproto"))); + assertEquals(400, e.status()); } @Test - void headers_exceedingMaxBufferSize_throwsIOException() { - // Feed more bytes than the configured cap with no \r\n\r\n : must throw + void headers_exceedingMaxBufferSize_throwsMalformedRequestException() { + // Feed more bytes than the configured cap with no \r\n\r\n : must throw 431 (EX-08). int cap = 16 * 1024; byte[] giant = new byte[cap + 1]; Arrays.fill(giant, (byte) 'A'); - assertThrows(IOException.class, () -> new RequestParser(cap).parse(new ByteArrayInputStream(giant))); + MalformedRequestException e = assertThrows(MalformedRequestException.class, + () -> new RequestParser(cap).parse(source(giant))); + assertEquals(431, e.status()); } @Test @@ -132,22 +149,38 @@ class RequestParserTest { "Transfer-Encoding: chunked\r\n" + "\r\n" + "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\n"; - Request r = new RequestParser().parse(new ByteArrayInputStream(raw.getBytes(StandardCharsets.UTF_8))); + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); assertNotNull(r); assertEquals(-1L, r.body().contentLength()); // -1 = chunked assertArrayEquals("hello world".getBytes(StandardCharsets.UTF_8), r.body().bytes()); } @Test - void contentLength_parsedAsLong() throws IOException { - // 5 GB — too large to materialize, but contentLength must be a long + void transferEncoding_multiValueEndingInChunked_recognised() throws IOException { + // EX-35: "gzip, chunked" — chunked need only be the FINAL coding (RFC 9112 §6.1). The + // old whole-value comparison misclassified this as not chunked at all. String raw = "POST / HTTP/1.1\r\n" + "Host: localhost\r\n" + - "Content-Length: 5000000000\r\n" + - "\r\n"; - Request r = new RequestParser().parse(new ByteArrayInputStream(raw.getBytes(StandardCharsets.UTF_8))); + "Transfer-Encoding: gzip, chunked\r\n" + + "\r\n" + + "2\r\nhi\r\n0\r\n\r\n"; + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); assertNotNull(r); - assertEquals(5_000_000_000L, r.body().contentLength()); + assertEquals(-1L, r.body().contentLength()); + assertArrayEquals("hi".getBytes(StandardCharsets.UTF_8), r.body().bytes()); + } + + @Test + void contentLength_parsedAsLong() throws IOException { + // ~3 GB — comfortably above Integer.MAX_VALUE (proving the value is a genuine long, not + // silently truncated) while staying within Http1Limits.MAX_CONTENT_LENGTH (4 GiB). + String raw = "POST / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: 3000000000\r\n" + + "\r\n"; + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); + assertNotNull(r); + assertEquals(3_000_000_000L, r.body().contentLength()); assertThrows(IllegalStateException.class, r.body()::bytes); } @@ -156,7 +189,7 @@ class RequestParserTest { // Content-Length claims 50 but stream ends after 5 bytes String body = "hello"; String raw = "POST / HTTP/1.1\r\nContent-Length: 50\r\n\r\n" + body; - Request r = new RequestParser().parse(new ByteArrayInputStream(raw.getBytes(StandardCharsets.UTF_8))); + Request r = new RequestParser().parse(source(raw.getBytes(StandardCharsets.UTF_8))); assertNotNull(r); assertEquals(50, r.body().bytes().length); assertEquals(body, new String(r.body().bytes(), 0, body.length(), StandardCharsets.UTF_8)); diff --git a/flash/src/test/java/dev/relism/flash/http/HttpStatusTest.java b/flash/src/test/java/dev/relism/flash/http/HttpStatusTest.java index 9256ce0..efe9bdf 100644 --- a/flash/src/test/java/dev/relism/flash/http/HttpStatusTest.java +++ b/flash/src/test/java/dev/relism/flash/http/HttpStatusTest.java @@ -40,4 +40,26 @@ class HttpStatusTest { assertNull(HttpStatus.reasonForCode(999)); assertNull(HttpStatus.reasonForCode(0)); } + + // --- EX-17: bound computed from values(), not a hand-maintained constant ----- + + @Test + void statusesAboveThePreviousHandMaintainedBound_workCorrectly() { + // The bound used to be hardcoded at 504; any of these (all >504, all needed by h1 + // hardening or h2) used to throw ArrayIndexOutOfBoundsException from the static + // initializer at class-load time. + assertArrayEquals("421 Misdirected Request".getBytes(StandardCharsets.UTF_8), HttpStatus.bytesForCode(421)); + assertArrayEquals("431 Request Header Fields Too Large".getBytes(StandardCharsets.UTF_8), HttpStatus.bytesForCode(431)); + assertArrayEquals("505 HTTP Version Not Supported".getBytes(StandardCharsets.UTF_8), HttpStatus.bytesForCode(505)); + assertArrayEquals("507 Insufficient Storage".getBytes(StandardCharsets.UTF_8), HttpStatus.bytesForCode(507)); + assertArrayEquals("511 Network Authentication Required".getBytes(StandardCharsets.UTF_8), HttpStatus.bytesForCode(511)); + } + + @Test + void everyEnumConstant_hasAWorkingBytesForCodeEntry() { + for (HttpStatus s : HttpStatus.values()) { + assertNotNull(HttpStatus.bytesForCode(s.code()), s.name()); + assertNotNull(HttpStatus.reasonForCode(s.code()), s.name()); + } + } } diff --git a/flash/src/test/java/dev/relism/flash/tls/TlsConfigTest.java b/flash/src/test/java/dev/relism/flash/tls/TlsConfigTest.java index 9fbd56a..d5b5eec 100644 --- a/flash/src/test/java/dev/relism/flash/tls/TlsConfigTest.java +++ b/flash/src/test/java/dev/relism/flash/tls/TlsConfigTest.java @@ -120,4 +120,73 @@ class TlsConfigTest { assertFalse(socket.getNeedClientAuth()); } } + + // --- EX-31: cipher suite filtering when h2 is offered ----------------------- + + @Test + void negotiatesH2_trueOnlyWhenH2IsInTheOfferedList() throws Exception { + SSLContext ctx = TestKeystores.trustAllClientContext(); + assertTrue(TlsConfig.ofContext(ctx).applicationProtocols("h2", "http/1.1").negotiatesH2()); + assertTrue(TlsConfig.ofContext(ctx).applicationProtocols("h2").negotiatesH2()); + assertFalse(TlsConfig.ofContext(ctx).applicationProtocols("http/1.1").negotiatesH2()); + assertFalse(TlsConfig.ofContext(ctx).negotiatesH2()); // no applicationProtocols call at all + } + + @Test + void applyTo_withH2Offered_removesBlockedTls12CipherSuites() throws Exception { + SSLContext ctx = TestKeystores.trustAllClientContext(); + TlsConfig tls = TlsConfig.ofContext(ctx).applicationProtocols("h2", "http/1.1"); + + try (SSLServerSocket socket = unboundSocket(tls)) { + tls.applyTo(socket); + List enabled = Arrays.asList(socket.getEnabledCipherSuites()); + // Spot-check a handful of RFC 9113 Appendix A entries across different families + // (RSA key exchange, 3DES, plain ECDHE-CBC) rather than the full ~280-entry list — + // TLS12_H2_BLOCKED_CIPHERS itself is the source of truth for the complete set. + assertFalse(enabled.contains("TLS_RSA_WITH_AES_128_CBC_SHA")); + assertFalse(enabled.contains("TLS_RSA_WITH_3DES_EDE_CBC_SHA")); + assertFalse(enabled.contains("TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA")); + assertFalse(enabled.contains("TLS_NULL_WITH_NULL_NULL")); + } + } + + @Test + void applyTo_withH2Offered_keepsTheRequiredCipherSuiteWhenTheJdkEnabledIt() throws Exception { + SSLContext ctx = TestKeystores.trustAllClientContext(); + TlsConfig tls = TlsConfig.ofContext(ctx).applicationProtocols("h2"); + + try (SSLServerSocket socket = unboundSocket(tls)) { + boolean jdkEnabledItByDefault = + Arrays.asList(socket.getEnabledCipherSuites()).contains(TlsConfig.REQUIRED_H2_CIPHER_SUITE); + tls.applyTo(socket); + if (jdkEnabledItByDefault) { + assertTrue(Arrays.asList(socket.getEnabledCipherSuites()).contains(TlsConfig.REQUIRED_H2_CIPHER_SUITE), + "RFC 9113 §9.2.2 requires supporting this suite — filtering must never remove it"); + } + } + } + + @Test + void applyTo_withoutH2Offered_leavesCipherSuitesUntouched() throws Exception { + SSLContext ctx = TestKeystores.trustAllClientContext(); + TlsConfig tls = TlsConfig.ofContext(ctx).applicationProtocols("http/1.1"); + + try (SSLServerSocket socket = unboundSocket(tls)) { + List before = Arrays.asList(socket.getEnabledCipherSuites()); + tls.applyTo(socket); + assertEquals(before, Arrays.asList(socket.getEnabledCipherSuites())); + } + } + + @Test + void applyTo_noApplicationProtocolsAtAll_leavesCipherSuitesUntouched() throws Exception { + SSLContext ctx = TestKeystores.trustAllClientContext(); + TlsConfig tls = TlsConfig.ofContext(ctx); + + try (SSLServerSocket socket = unboundSocket(tls)) { + List before = Arrays.asList(socket.getEnabledCipherSuites()); + tls.applyTo(socket); + assertEquals(before, Arrays.asList(socket.getEnabledCipherSuites())); + } + } } diff --git a/flash/src/test/java/dev/relism/flash/transport/ProtocolNegotiatorTest.java b/flash/src/test/java/dev/relism/flash/transport/ProtocolNegotiatorTest.java new file mode 100644 index 0000000..4f9762b --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/transport/ProtocolNegotiatorTest.java @@ -0,0 +1,154 @@ +package dev.relism.flash.transport; + +import dev.relism.flash.tls.TestKeystores; +import dev.relism.flash.tls.TlsConfig; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.concurrent.CompletableFuture; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@link ProtocolNegotiator#negotiate} is a pure, directly-testable detector (see its Javadoc + * for why it does not itself consult {@code FlashConfiguration.http2Enabled}) — every case here + * calls it directly rather than through {@code HttpServer}. + */ +class ProtocolNegotiatorTest { + + // ── Plaintext (h2c prior knowledge) — no real networking needed ──────────── + + private static BufferedByteSource plaintextSource(String bytes) { + return new BufferedByteSource( + new ByteArrayInputStream(bytes.getBytes(StandardCharsets.US_ASCII)), new Socket()); + } + + @Test + void h2cPrefaceExact_negotiatesH2() throws IOException { + BufferedByteSource src = plaintextSource("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"); + assertEquals(NegotiatedProtocol.H2, ProtocolNegotiator.negotiate(new Socket(), src)); + } + + @Test + void h2cPrefaceFollowedByMoreData_stillNegotiatesH2_andDoesNotConsume() throws IOException { + BufferedByteSource src = plaintextSource("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\nEXTRA"); + assertEquals(NegotiatedProtocol.H2, ProtocolNegotiator.negotiate(new Socket(), src)); + // peek() must not have consumed anything — the full 24-byte preface is still there for + // whatever reads next (Http2Connection, once it exists). + byte[] readBack = new byte[24]; + assertEquals(24, src.read(readBack, 0, 24)); + assertEquals("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", + new String(readBack, StandardCharsets.US_ASCII)); + } + + @Test + void partialPreface_thenEof_negotiatesHttp1_notConsumed() throws IOException { + // Fewer than 24 bytes total, then EOF — not a match, and RequestParser must still see + // every byte that was actually sent. + BufferedByteSource src = plaintextSource("PRI * HTTP/2.0\r\n"); + assertEquals(NegotiatedProtocol.HTTP_1_1, ProtocolNegotiator.negotiate(new Socket(), src)); + byte[] readBack = new byte[16]; + assertEquals(16, src.read(readBack, 0, 16)); + assertEquals("PRI * HTTP/2.0\r\n", new String(readBack, StandardCharsets.US_ASCII)); + } + + @Test + void prefaceLookalike_divergesPartway_negotiatesHttp1() throws IOException { + // "PRI " matches, then diverges — must not be misdetected as h2c. + BufferedByteSource src = plaintextSource("PRI * HTTP/9.9\r\n\r\nXX\r\n\r\n"); + assertEquals(NegotiatedProtocol.HTTP_1_1, ProtocolNegotiator.negotiate(new Socket(), src)); + } + + @Test + void plainGetRequest_negotiatesHttp1() throws IOException { + BufferedByteSource src = plaintextSource("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"); + assertEquals(NegotiatedProtocol.HTTP_1_1, ProtocolNegotiator.negotiate(new Socket(), src)); + byte[] readBack = new byte[15]; + assertEquals(15, src.read(readBack, 0, 15)); + assertEquals("GET / HTTP/1.1\r", new String(readBack, StandardCharsets.US_ASCII)); + } + + // ── TLS/ALPN — a real loopback handshake, since ALPN is resolved during it ─ + + private interface ThrowingConsumer { void accept(T t) throws Exception; } + + /** + * Binds a real TLS listener offering {@code serverAlpn}, connects a client offering + * {@code clientAlpn}, forces the handshake on both sides (mirroring {@code HttpServer}'s + * EX-30 fix), and hands the accepted server-side socket to {@code assertion}. + */ + private static void withNegotiatedAlpn(Path dir, String[] serverAlpn, String[] clientAlpn, + ThrowingConsumer assertion) throws Exception { + Path ks = TestKeystores.build(dir, "negotiator.p12", "changeit", + TestKeystores.Entry.of("only", "negotiator.test")); + TlsConfig serverTls = TlsConfig.keystore(ks, "changeit"); + if (serverAlpn != null) serverTls = serverTls.applicationProtocols(serverAlpn); + + try (SSLServerSocket serverSocket = (SSLServerSocket) serverTls.serverSocketFactory().createServerSocket()) { + serverTls.applyTo(serverSocket); + serverSocket.bind(new InetSocketAddress("127.0.0.1", 0)); + int port = serverSocket.getLocalPort(); + + CompletableFuture accepted = CompletableFuture.supplyAsync(() -> { + try { + SSLSocket s = (SSLSocket) serverSocket.accept(); + s.startHandshake(); + return s; + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + + SSLSocketFactory clientFactory = TestKeystores.trustAllClientContext().getSocketFactory(); + try (SSLSocket client = (SSLSocket) clientFactory.createSocket("127.0.0.1", port)) { + if (clientAlpn != null) { + javax.net.ssl.SSLParameters params = client.getSSLParameters(); + params.setApplicationProtocols(clientAlpn); + client.setSSLParameters(params); + } + client.startHandshake(); + + try (SSLSocket server = accepted.get()) { + assertion.accept(server); + } + } + } + } + + @Test + void alpnH2_negotiatesH2(@TempDir Path dir) throws Exception { + withNegotiatedAlpn(dir, new String[]{"h2", "http/1.1"}, new String[]{"h2", "http/1.1"}, server -> { + assertEquals("h2", server.getApplicationProtocol()); + assertEquals(NegotiatedProtocol.H2, + ProtocolNegotiator.negotiate(server, new BufferedByteSource(server.getInputStream(), server))); + }); + } + + @Test + void alpnHttp11_negotiatesHttp1(@TempDir Path dir) throws Exception { + withNegotiatedAlpn(dir, new String[]{"h2", "http/1.1"}, new String[]{"http/1.1"}, server -> { + assertEquals("http/1.1", server.getApplicationProtocol()); + assertEquals(NegotiatedProtocol.HTTP_1_1, + ProtocolNegotiator.negotiate(server, new BufferedByteSource(server.getInputStream(), server))); + }); + } + + @Test + void alpnAbsent_negotiatesHttp1(@TempDir Path dir) throws Exception { + // Neither side offers ALPN at all — the common case today. + withNegotiatedAlpn(dir, null, null, server -> { + assertEquals(NegotiatedProtocol.HTTP_1_1, + ProtocolNegotiator.negotiate(server, new BufferedByteSource(server.getInputStream(), server))); + }); + } +}