From 31aad7f573fe58f2db026176bdf72d0925e9e3d6 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:37:43 +0200 Subject: [PATCH 01/16] feat: Decompress gzip request bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requests carrying `Content-Encoding: gzip` are now inflated before OpenAPI body validation runs, so the validator, the type mappers and handlers all see plain bytes. Inflation runs through a counting loop under a hard cap (10 MiB) so a small compressed payload cannot expand into an OOM. Exceeding the cap yields 413, an unsupported coding yields 415, and a malformed or truncated gzip stream yields 400 — all as problem+json. Only ZipException and EOFException are converted; a genuine socket failure stays an IOException and still renders 500. Once inflated, the body no longer matches the request headers that described it, so the handler's header view hides `Content-Encoding` and reports the inflated `Content-Length`. --- docs/plans/dynamic-discovering-piglet.md | 368 ++++++++++++++++++ .../com/retailsvc/http/OpenApiServer.java | 43 +- .../http/internal/AcceptEncodingHeader.java | 73 ++++ .../http/internal/ContentEncodingHeader.java | 50 +++ .../retailsvc/http/internal/ExtrasRouter.java | 12 +- .../http/internal/ProblemDetail.java | 34 +- .../http/internal/RequestBodyReader.java | 112 ++++++ .../internal/RequestPreparationFilter.java | 11 +- .../internal/AcceptEncodingHeaderTest.java | 94 +++++ .../internal/ContentEncodingHeaderTest.java | 67 ++++ .../http/internal/ExtrasRouterTest.java | 43 +- .../http/internal/ProblemDetailTest.java | 15 + .../http/internal/RequestBodyReaderTest.java | 173 ++++++++ .../RequestPreparationFilterTest.java | 84 +++- 14 files changed, 1136 insertions(+), 43 deletions(-) create mode 100644 docs/plans/dynamic-discovering-piglet.md create mode 100644 src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java create mode 100644 src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java create mode 100644 src/main/java/com/retailsvc/http/internal/RequestBodyReader.java create mode 100644 src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java create mode 100644 src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java create mode 100644 src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java diff --git a/docs/plans/dynamic-discovering-piglet.md b/docs/plans/dynamic-discovering-piglet.md new file mode 100644 index 00000000..a23af35c --- /dev/null +++ b/docs/plans/dynamic-discovering-piglet.md @@ -0,0 +1,368 @@ +# gzip content encoding + +## Context + +The library has no notion of `Content-Encoding` or `Accept-Encoding` today — a repo-wide grep +returns zero hits. Every request body is read verbatim with `exchange.getRequestBody().readAllBytes()` +and every response body is written verbatim by `ResponseRenderer`. A client that gzips its payload +gets a schema-validation failure on binary garbage, and JSON responses always go out uncompressed +even when the client advertises gzip support. + +This adds transparent gzip in both directions: + +- **Requests** — a body sent with `Content-Encoding: gzip` is inflated before OpenAPI body + validation runs, so the validator, the `TypeMapper`s and handlers see plain bytes and need no + changes. Inflation is bounded so a small compressed payload cannot expand into an OOM (zip bomb). +- **Responses** — a body is gzipped when the client sends `Accept-Encoding: gzip`, the media type is + text-shaped, and the payload clears a size threshold. + +Both directions are on by default; the two numeric knobs are builder options. + +## Design decisions + +| Decision | Choice | +|---|---| +| Enablement | On by default, no on/off flag. Raising `minimumGzipResponseBytes` is the escape hatch. | +| Codings | `gzip` (and the legacy `x-gzip` alias) only. `identity` is a legal no-op, not an error. | +| Unknown request coding (`br`, `deflate`, two stacked codings) | `415 Unsupported Media Type` | +| Malformed / truncated gzip stream | `400 Bad Request`, problem+json, `ZipException` as cause | +| Over the inflation cap | `413 Content Too Large` | +| `Accept-Encoding` absent, or `gzip;q=0` | No compression. `*` with q>0 does compress. | +| Response policy | Compressible media type **and** body ≥ threshold (default 1 KiB) | +| Inflation cap | Default 10 MiB, `maxDecompressedRequestBytes(long)` | +| Uncompressed request bodies | Stay unbounded, as today — pre-existing gap, out of scope | +| Streaming responses | Compressed too; `BodyWriter.Sized` degrades to chunked | +| `Request.header("Content-Encoding")` | Hidden after decoding; `Content-Length` reports the *inflated* length | +| `SecurityFilter` 401/403 | Left uncompressed — bypasses the renderer, bodies are ~120 bytes | + +## Branch + +`feat/gzip-content-encoding`, cut from `master`. A plain branch, not a worktree — the Java LSP and +the SonarLint MCP server are both blind to `.claude/worktrees/`. + +--- + +## Architecture + +Two shared, immutable collaborators built once in `OpenApiServer` and threaded into the filters, +exactly as `ResponseRenderer` is threaded today from `OpenApiServer.java:95`: + +- `RequestBodyReader` (new) — inbound decode, owns the inflation cap. +- `ResponseRenderer` (existing) — outbound encode, gains the size threshold. + +Putting the encode step inside `ResponseRenderer.render` covers five of the six response paths: +normal dispatch, handler exceptions, pre-request 404/405/400, extras routes (`/health`, spec +serving, CORS preflight) and the catch-all `/` 404. The sixth — `SecurityFilter.renderRejection` +(`SecurityFilter.java:111-137`) — writes to the exchange directly and stays uncompressed by design. + +A `ResponseDecorator` would have been the wrong hook: decorators run only in `DispatchHandler` +(`DispatchHandler.java:47-49`), so every error body, the health endpoint and all 404s would be missed. + +## New files + +### `internal/AcceptEncodingHeader.java` + +```java +/** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */ +public final class AcceptEncodingHeader { + public static boolean acceptsGzip(String header); +} +``` + +Split on `,`; per token strip `;` params; lowercase. `q` defaults to `1.0`, and an unparsable `q` is +treated as `1.0` (lenient — no security consequence). Track `gzipQ` (from `gzip` or `x-gzip`) and +`starQ`; return `gzipQ != null ? gzipQ > 0 : starQ != null && starQ > 0`. An explicit `gzip;q=0` is a +refusal and beats a positive `*`. + +### `internal/ContentEncodingHeader.java` + +```java +/** Classifies a request {@code Content-Encoding} into the codings the server can decode. */ +public final class ContentEncodingHeader { + public enum Coding { NONE, GZIP, UNSUPPORTED } + public static Coding parse(String header); +} +``` + +`null`/blank/`identity` → `NONE`; a single `gzip`/`x-gzip` (optionally with `identity`) → `GZIP`; +anything else, including two real stacked codings → `UNSUPPORTED`. + +### `internal/RequestBodyReader.java` + +```java +/** Reads the request body, transparently inflating gzip under a hard cap on decompressed size. */ +public final class RequestBodyReader { + public static final long DEFAULT_MAX_DECOMPRESSED_BYTES = 10L * 1024 * 1024; + + public RequestBodyReader(long maxDecompressedBytes); // IllegalArgumentException if <= 0 + public Body read(HttpExchange exchange) throws IOException; + + public record Body(byte[] bytes, boolean decoded) { + public UnaryOperator headerLookup(Headers headers); + } +} +``` + +Read the raw bytes first, then inflate from a `ByteArrayInputStream`. Inflating the exchange stream +directly would make `new GZIPInputStream(...)` throw `EOFException` on an empty body, forcing a peek +to tell "no body" from "truncated body". Peak memory is compressed + inflated, and the plain path +already buffers the whole body. + +Inflate in a loop with a running counter; trip the cap **mid-inflate** rather than after, so a bomb +never materialises. Catch **only `ZipException | EOFException`** and rethrow as +`BadRequestException(HTTP_BAD_REQUEST, "malformed gzip request body", e)` — a genuine socket +`IOException` must stay an `IOException` and render 500, not 400. + +`headerLookup` returns `headers::getFirst` untouched when nothing was decoded. After inflating it +hides `Content-Encoding` and reports the inflated `Content-Length`: the stored value is the +*compressed* size, which would be a lie sitting next to `bytes()`. + +`BadRequestException` already enforces a 4xx status (`BadRequestException.java:47`) and +`Handlers.defaultExceptionHandler()` (`Handlers.java:67-77`) renders it as problem+json with the +supplied status and logs the cause at DEBUG — 413 and 415 need no new error plumbing. + +### `internal/ResponseCompression.java` + +```java +/** Response content-coding policy and the gzip primitives the renderer uses. */ +public final class ResponseCompression { + public static boolean isCompressible(String contentType); + public static byte[] gzip(byte[] body) throws IOException; + public static OutputStream gzipStream(OutputStream out) throws IOException; +} +``` + +`text/*` **except `text/event-stream`** (gzip buffers through a `Deflater`; wrapping SSE destroys +per-event flushing and hangs the client), the `+json` / `+xml` / `+yaml` structured suffixes, plus +`application/json`, `application/xml`, `application/yaml`, `application/x-yaml`, +`application/javascript`, `application/x-ndjson`. `application/problem+json` and `image/svg+xml` fall +out of the suffix rules; `application/octet-stream` — the `byte[]` default — is excluded. + +**Null guard is load-bearing:** `ContentTypeHeader.mediaType(null)` returns `application/json`, so a +null content type must be rejected *before* that call, or every untyped stream looks like JSON. + +## Modified files + +### `internal/ResponseRenderer.java` — the bulk of the work + +Two-arg constructor `(Map, long minimumGzipBytes)` plus a retained 1-arg +overload delegating to it with `DEFAULT_MINIMUM_GZIP_BYTES = 1024`, so the three existing test +call sites compile untouched. + +`render` gains a third branch so the null-body case can still do header work: +`renderEmpty` / `renderStream` / `renderBytes`. + +Three private helpers: + +- `bodyAllowed(int status)` — false for 1xx, 204, 205, 206, 304. Those must never carry a coding. +- `wouldCompress(exchange, headers, length)` — no existing `Content-Encoding`, `length >= threshold`, + and `AcceptEncodingHeader.acceptsGzip(...)`. +- `addVary(headers)` — appends `Accept-Encoding` to any existing `Vary` (the CORS preflight handler + already emits `Vary: Origin`) as one merged field line, skipping if `*` or `Accept-Encoding` is + already listed. + +`renderBytes` inserts one `maybeCompress` call between the Content-Type default and +`sendResponseHeaders`; `Content-Length` stays correct for free because `:71` derives it from +`bytes.length`. If gzip comes out no smaller than the input, keep the original bytes and set no +header. + +`renderStream` threshold-checks `BodyWriter.Sized.length()`; a `Chunked` body has no knowable length +and maps to "always over threshold" — buffering to measure would defeat streaming. When compressing, +send `sendResponseHeaders(status, 0)` (chunked), because `Sized.length()` is the *uncompressed* +length and must not go on the wire. + +`renderEmpty` is where the HEAD fix lands: when a GET would have been compressed, drop the +hand-declared `Content-Length` (RFC 9110 §9.3.2 permits omitting fields "determined only while +generating the content"). Never set `Content-Encoding` on a bodiless response — it would promise an +encoding for a body the client may later fetch with a different `Accept-Encoding`. + +**Ordering constraint:** check `isCompressible(contentType) && bodyAllowed(status)` before reading +`exchange.getRequestHeaders()`. Cheap checks first anyway — but see the test-stub note in Task 7. + +### `internal/RequestPreparationFilter.java` + +New field + 8th constructor parameter `RequestBodyReader bodyReader` (the constructor already carries +`@SuppressWarnings("java:S107")`). At `:97`, `bodyReader.read(exchange)`; at `:124`, +`body.headerLookup(headers)` in place of `headers::getFirst`. The read stays before routing, so +404/405 behaviour is unchanged. No exception wiring: `BadRequestException` is a `RuntimeException` +and `doFilter`'s catch at `:72-76` already renders it. + +### `internal/ExtrasRouter.java` + +Same substitution at `:58` and `:68`, plus a 3rd constructor parameter. + +### `internal/ProblemDetail.java` + +`TITLES` has no 413 entry, so a 413 would render `"title": "Bad Request"`. Add +`413 -> "Content Too Large"`. **`Map.of` caps at 10 pairs and `TITLES` has exactly 10** — the 11th +forces a rewrite to `Map.ofEntries(entry(...), ...)`. + +### `Handlers.java` + +**No change.** The HEAD fix lives in `renderEmpty`, which fixes *any* handler that hand-sets +`Content-Length` on a null body, not just `resourceHandler`. + +### `OpenApiServer.java` + +`HandlerConfig` (`:61-67`) gains `long maxDecompressedRequestBytes, long minimumGzipResponseBytes`. +Builder fields default from the two internal constants; two fluent setters modelled on +`shutdownTimeoutSeconds` (`:403-410`) — `maxDecompressedRequestBytes` requires `> 0`, +`minimumGzipResponseBytes` requires `>= 0`. At `:95`, build both collaborators and thread +`bodyReader` through `wireBindings` → `wireBinding` and `wireExtras`, parallel to `renderer`. + +--- + +## Tasks + +Test-first throughout: write the named tests, watch them fail, then implement. Check off each step +as it completes. + +### Task 1 — header parsing + +- [x] **Step 1** `internal/AcceptEncodingHeaderTest` — `nullHeaderIsNotAccepted`, + `blankHeaderIsNotAccepted`, `plainGzipIsAccepted`, `gzipAmongOtherCodingsIsAccepted`, + `caseInsensitiveGzipIsAccepted`, `xGzipIsAccepted`, `explicitZeroQValueIsRefused`, + `positiveQValueIsAccepted`, `wildcardIsAccepted`, `wildcardWithZeroQValueIsRefused`, + `explicitGzipBeatsWildcardRefusal`, `identityOnlyIsNotAccepted`, + `malformedQValueIsTreatedAsAccepted`. Then implement. +- [x] **Step 2** `internal/ContentEncodingHeaderTest` — `nullHeaderIsNone`, `emptyHeaderIsNone`, + `identityIsNone`, `gzipIsGzip`, `xGzipIsGzip`, `mixedCaseGzipIsGzip`, `gzipWithIdentityIsGzip`, + `brotliIsUnsupported`, `deflateIsUnsupported`, `stackedCodingsAreUnsupported`. Then implement. + +### Task 2 — bounded request inflation + +- [x] **Step 3** `internal/RequestBodyReaderTest` (mocked `HttpExchange`, per the + `ExtrasRouterTest` pattern) — `plainBodyIsReturnedUnchanged`, `gzipBodyIsInflated`, + `emptyGzipBodyIsReturnedEmpty`, `unsupportedCodingThrows415`, `oversizedInflatedBodyThrows413`, + `malformedGzipThrows400WithCause`, `truncatedGzipThrows400`, + `decodedBodyHidesContentEncodingHeader`, `decodedBodyReportsInflatedContentLength`, + `plainBodyKeepsOriginalHeaderLookup`, `constructorRejectsNonPositiveCap`. Then implement. +- [x] **Step 4** `internal/ProblemDetailTest` — `contentTooLargeHasItsOwnTitle`. Then convert + `TITLES` to `Map.ofEntries` and add the 413 row. +- [x] **Step 5** Wire into `RequestPreparationFilter` and `ExtrasRouter`: + `gzipRequestBodyIsInflatedBeforeValidation`, `unsupportedRequestCodingIsRejectedBeforeRouting`, + `gzipRequestBodyIsInflatedForExtraRoutes`. Their test factories gain a `RequestBodyReader` + argument — the compile break is the expected first failure. + +### Task 3 — response compression + +- [ ] **Step 6** `internal/ResponseCompressionTest` — `nullContentTypeIsNotCompressible`, + `jsonIsCompressible`, `problemJsonIsCompressible`, `yamlIsCompressible`, + `textPlainWithCharsetIsCompressible`, `xmlSuffixIsCompressible`, + `octetStreamIsNotCompressible`, `imagePngIsNotCompressible`, `eventStreamIsNotCompressible`, + `gzipRoundTripsBytes`, `gzipStreamRoundTripsBytes`. Then implement. +- [ ] **Step 7** `internal/ResponseRendererTest` — the repo's **first direct renderer test**, so it + starts with baseline coverage of behaviour it is about to change + (`writesBytesWithContentLength`, `writesNullBodyWithMinusOne`, `writesSizedStreamWithLength`), + then: `compressesJsonBodyOverThreshold`, `setsContentEncodingGzipWhenCompressed`, + `sentContentLengthMatchesCompressedPayload`, `skipsCompressionBelowThreshold`, + `skipsCompressionForOctetStream`, `skipsCompressionWithoutAcceptEncoding`, + `skipsCompressionWhenGzipRefusedByQValue`, + `skipsCompressionWhenHandlerAlreadySetContentEncoding`, `addsVaryEvenWhenNotCompressed`, + `appendsVaryToExistingValue`, `doesNotDuplicateVary`, `neverCompressesNoContentResponses`, + `fallsBackToPlainBytesWhenGzipIsLarger`, `leavesUncompressedBodyByteIdentical`. + Then implement `bodyAllowed`, `wouldCompress`, `addVary`, `maybeCompress`. + + Add `when(exchange.getRequestHeaders()).thenReturn(new Headers())` to + `DispatchHandlerTest.stubExchange()` — it stubs only `getResponseHeaders()` today, so an + unstubbed `getRequestHeaders()` returns null. Fixing the stub is honest and removes a fragile + dependency on evaluation order in main code. +- [ ] **Step 8** Streaming and null bodies, same test class: + `compressesChunkedStreamWhenAcceptEncodingPresent`, + `degradesSizedStreamToChunkedWhenCompressed`, `skipsCompressionForSizedStreamBelowThreshold`, + `skipsCompressionForNullContentTypeStream`, + `stripsContentLengthOnNullBodyWhenGetWouldCompress`, + `keepsContentLengthOnNullBodyWhenClientDoesNotAcceptGzip`, + `keepsContentLengthOnNullBodyForNonCompressibleType`. + Then implement `renderStream` and `renderEmpty`. + +### Task 4 — builder + +- [ ] **Step 9** `OpenApiServerBuilderTest` — `maxDecompressedRequestBytesRejectsZero`, + `maxDecompressedRequestBytesRejectsNegative`, `minimumGzipResponseBytesRejectsNegative`. + Then add the two setters, the `HandlerConfig` fields and the `build()` wiring. + +### Task 5 — end to end + +- [ ] **Step 10** `GzipIT` extending `ServerBaseTest`. Reuses the existing `/openapi.json` fixture + with runtime handler overrides — **no new spec files**. `text-echo` (`POST /text-echo`, + `text/plain`, schema `{"type":"string"}`, no `maxLength`) echoes the body, so one call + exercises both directions. Private `gzip(byte[])` / `gunzip(byte[])` helpers; requests built + manually as in `NonJsonBodyIT`; responses read with `BodyHandlers.ofByteArray()`. Tests: + `largeJsonResponseIsGzippedWhenClientAcceptsGzip`, `smallJsonResponseIsNotGzipped`, + `responseIsNotGzippedWithoutAcceptEncoding`, `streamedSpecResourceIsGzippedAndChunked`, + `headOmitsContentLengthWhenGetWouldBeCompressed`, + `headKeepsContentLengthWithoutAcceptEncoding`, `gzippedRequestBodyIsDecompressed`, + `handlerDoesNotSeeContentEncodingHeader`, `unsupportedRequestEncodingReturns415`, + `malformedGzipRequestReturns400`, `oversizedGzipRequestReturns413` (server built with + `maxDecompressedRequestBytes(1024)`, posting ~4 KiB gzipped — keeps it fast and covers the + builder option). + + `java.net.http.HttpClient` neither sends `Accept-Encoding` nor auto-decompresses, so + `responseIsNotGzippedWithoutAcceptEncoding` is the regression guard for the whole existing + IT suite. + +### Task 6 — docs + +- [ ] **Step 11** README: `### Request decompression` and `### Response compression` under + `## Server configuration`, a TOC entry, a `## Highlights` bullet, and a **"Not in this + release"** list matching the HTTPS section's convention — brotli/deflate/zstd, + `Accept-Encoding` on 415 responses (RFC 9110 §15.5.16 SHOULD; `BadRequestException` carries no + headers), compression of `SecurityFilter` 401/403 bodies, per-route opt-out. Plus `Caveats` + bullets: the cap bounds *inflated* bytes only and is not a request size limit; a strong `ETag` + set by a handler now spans two byte streams; a handler that throws mid-stream yields a valid + gzip trailer over truncated content rather than a framing error. +- [ ] **Step 12** Correct the stale request-flow description in `CLAUDE.md` — it describes three + filters including `ExceptionFilter` on the spec context, exchange-attribute body stashing and + a `Request.bytes(exchange)` static helper, none of which match the current code, and it + mentions neither `SecurityFilter` nor `ExtrasRouter`. + +--- + +## Verification + +```bash +mvn test +mvn test -Dtest=ResponseRendererTest +mvn verify # Failsafe runs GzipIT; JaCoCo at target/site/jacoco/ +mvn verify -Dit.test=GzipIT +``` + +What to look for: + +- **Zero regressions in the pre-existing ITs.** This is the load-bearing signal that on-by-default + compression is invisible to `java.net.http.HttpClient`. If `OpenApiServerIT`, `SecurityIT`, + `NonJsonBodyIT` or `ExtraHandlersIT` change behaviour, the gate is wrong. +- No JUL warning `sendResponseHeaders: being invoked with a content length for a HEAD request` in + the Failsafe output — that string means a HEAD request reached `renderBytes`. +- `target/site/jacoco/` — the four new internal classes near-fully covered, and `ResponseRenderer` + coverage sharply up now that it has a direct test. + +Manually against the example server: + +```bash +mvn test-compile exec:java -Dexec.mainClass=com.retailsvc.http.start.ServerLauncher \ + -Dexec.classpathScope=test + +# small body: Vary only, no Content-Encoding +curl -sD- -o /dev/null -H 'Accept-Encoding: gzip' http://localhost:8080/api/v1/data + +# request decompression — expect the echoed text back +printf 'hello gzip' | gzip | curl -s --data-binary @- \ + -H 'Content-Type: text/plain' -H 'Content-Encoding: gzip' \ + http://localhost:8080/api/v1/text-echo + +# unsupported coding — expect 415 problem+json +curl -si -H 'Content-Encoding: br' -H 'Content-Type: text/plain' \ + --data-binary 'x' http://localhost:8080/api/v1/text-echo +``` + +`k6 run acceptance/k6/script.js` should pass unchanged: it asserts +`r.headers['Content-Type'] === 'application/json'`, which compression never touches, and every +payload it exercises is far under 1 KiB. + +`mvn sortpom:sort` is not needed — no new dependencies, `java.util.zip` is in `java.base`. + +Before pushing: analyse every touched file with the SonarLint MCP server and fix any new issue in +the same branch. Watch for `java:S6218` on the `Body` record (array component) and `java:S107` on the +widened `HandlerConfig`. diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index e1823b83..ef2855af 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -9,6 +9,7 @@ import com.retailsvc.http.internal.ExtrasRouter; import com.retailsvc.http.internal.FormTypeMapper; import com.retailsvc.http.internal.PemSslContext; +import com.retailsvc.http.internal.RequestBodyReader; import com.retailsvc.http.internal.RequestPreparationFilter; import com.retailsvc.http.internal.ResponseRenderer; import com.retailsvc.http.internal.SecurityFilter; @@ -93,9 +94,24 @@ record HandlerConfig( httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); ResponseRenderer renderer = new ResponseRenderer(bodyMappers); + RequestBodyReader bodyReader = + new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES); boolean anyBindingAtRoot = - wireBindings(httpServer, bindings, bodyMappers, handlerConfig, exceptionHandler, renderer); - wireExtras(httpServer, anyBindingAtRoot, handlerConfig.extras(), exceptionHandler, renderer); + wireBindings( + httpServer, + bindings, + bodyMappers, + handlerConfig, + exceptionHandler, + renderer, + bodyReader); + wireExtras( + httpServer, + anyBindingAtRoot, + handlerConfig.extras(), + exceptionHandler, + renderer, + bodyReader); httpServer.start(); this.shutdownTimeoutSeconds = shutdownTimeoutSeconds; @@ -119,13 +135,21 @@ private static boolean wireBindings( Map bodyMappers, HandlerConfig handlerConfig, ExceptionHandler exceptionHandler, - ResponseRenderer renderer) { + ResponseRenderer renderer, + RequestBodyReader bodyReader) { boolean anyBindingAtRoot = false; for (SpecBinding binding : bindings) { String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/"); anyBindingAtRoot |= "/".equals(basePath); wireBinding( - httpServer, basePath, binding, bodyMappers, handlerConfig, exceptionHandler, renderer); + httpServer, + basePath, + binding, + bodyMappers, + handlerConfig, + exceptionHandler, + renderer, + bodyReader); } return anyBindingAtRoot; } @@ -138,7 +162,8 @@ private static void wireBinding( Map bodyMappers, HandlerConfig handlerConfig, ExceptionHandler exceptionHandler, - ResponseRenderer renderer) { + ResponseRenderer renderer, + RequestBodyReader bodyReader) { Map operationsById = binding.spec().operations().stream() .collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op)); @@ -152,7 +177,8 @@ private static void wireBinding( bodyMappers, exceptionHandler, renderer, - handlerConfig.afterHooks())); + handlerConfig.afterHooks(), + bodyReader)); ctx.getFilters() .add( new SecurityFilter( @@ -174,7 +200,8 @@ private static void wireExtras( boolean anyBindingAtRoot, Map extras, ExceptionHandler exceptionHandler, - ResponseRenderer renderer) { + ResponseRenderer renderer, + RequestBodyReader bodyReader) { if (anyBindingAtRoot) { if (!extras.isEmpty()) { throw new IllegalStateException( @@ -182,7 +209,7 @@ private static void wireExtras( } return; } - ExtrasRouter extrasRouter = new ExtrasRouter(extras, renderer); + ExtrasRouter extrasRouter = new ExtrasRouter(extras, renderer, bodyReader); HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); } diff --git a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java new file mode 100644 index 00000000..faec2167 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java @@ -0,0 +1,73 @@ +package com.retailsvc.http.internal; + +import java.util.Locale; + +/** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */ +public final class AcceptEncodingHeader { + + private static final String GZIP = "gzip"; + private static final String X_GZIP = "x-gzip"; + private static final String WILDCARD = "*"; + private static final String QUALITY = "q"; + private static final double DEFAULT_QUALITY = 1.0; + + private AcceptEncodingHeader() {} + + /** + * Whether the client accepts a gzip-coded response. A {@code null}, blank, or unrelated header + * yields {@code false}. An explicit {@code gzip;q=0} is a refusal and outranks a positive + * wildcard; a wildcard applies only when gzip is not listed in its own right. + */ + public static boolean acceptsGzip(String header) { + if (header == null) { + return false; + } + Boolean gzipAccepted = null; + Boolean wildcardAccepted = null; + for (String token : header.split(",")) { + String trimmed = token.trim(); + if (trimmed.isEmpty()) { + continue; + } + int semi = trimmed.indexOf(';'); + String coding = + (semi < 0 ? trimmed : trimmed.substring(0, semi)).trim().toLowerCase(Locale.ROOT); + boolean accepted = quality(semi < 0 ? null : trimmed.substring(semi + 1)) > 0; + if (GZIP.equals(coding) || X_GZIP.equals(coding)) { + gzipAccepted = gzipAccepted == null ? accepted : gzipAccepted || accepted; + } else if (WILDCARD.equals(coding)) { + wildcardAccepted = wildcardAccepted == null ? accepted : wildcardAccepted || accepted; + } + } + if (gzipAccepted != null) { + return gzipAccepted; + } + return wildcardAccepted != null && wildcardAccepted; + } + + /** + * Reads the {@code q} weight from a token's parameter list. An absent or unparsable weight is + * read as the default 1.0 — a malformed header should not silently disable compression. + */ + private static double quality(String parameters) { + if (parameters == null) { + return DEFAULT_QUALITY; + } + for (String parameter : parameters.split(";")) { + String trimmed = parameter.trim(); + int equals = trimmed.indexOf('='); + if (equals <= 0) { + continue; + } + String name = trimmed.substring(0, equals).trim().toLowerCase(Locale.ROOT); + if (QUALITY.equals(name)) { + try { + return Double.parseDouble(trimmed.substring(equals + 1).trim()); + } catch (NumberFormatException e) { + return DEFAULT_QUALITY; + } + } + } + return DEFAULT_QUALITY; + } +} diff --git a/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java new file mode 100644 index 00000000..4c9e2cd3 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java @@ -0,0 +1,50 @@ +package com.retailsvc.http.internal; + +import java.util.Locale; + +/** Classifies a request {@code Content-Encoding} into the codings the server can decode. */ +public final class ContentEncodingHeader { + + private static final String IDENTITY_CODING = "identity"; + private static final String GZIP_CODING = "gzip"; + private static final String X_GZIP_CODING = "x-gzip"; + + private ContentEncodingHeader() {} + + /** The content coding applied to a request body. */ + public enum Coding { + /** No coding, or the explicit {@code identity} no-op. */ + NONE, + /** A single gzip coding. */ + GZIP, + /** A coding this server cannot decode; the caller renders 415. */ + UNSUPPORTED + } + + /** + * Classifies the header value. {@code null}, blank and {@code identity} are all {@link + * Coding#NONE}; a single gzip coding — optionally alongside {@code identity} — is {@link + * Coding#GZIP}. Anything else, including two stacked codings, is {@link Coding#UNSUPPORTED}. + */ + public static Coding parse(String header) { + if (header == null) { + return Coding.NONE; + } + Coding result = Coding.NONE; + for (String token : header.split(",")) { + String coding = token.trim().toLowerCase(Locale.ROOT); + if (coding.isEmpty() || IDENTITY_CODING.equals(coding)) { + continue; + } + if (result != Coding.NONE) { + return Coding.UNSUPPORTED; + } + if (GZIP_CODING.equals(coding) || X_GZIP_CODING.equals(coding)) { + result = Coding.GZIP; + } else { + return Coding.UNSUPPORTED; + } + } + return result; + } +} diff --git a/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java b/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java index 62e06a41..fbe0d5cc 100644 --- a/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java +++ b/src/main/java/com/retailsvc/http/internal/ExtrasRouter.java @@ -21,9 +21,12 @@ private record Entry(PathPattern pattern, RequestHandler handler) {} private final Map exact; private final List wildcards; private final ResponseRenderer renderer; + private final RequestBodyReader bodyReader; - public ExtrasRouter(Map extras, ResponseRenderer renderer) { + public ExtrasRouter( + Map extras, ResponseRenderer renderer, RequestBodyReader bodyReader) { this.renderer = renderer; + this.bodyReader = bodyReader; Map exactBuilder = new LinkedHashMap<>(); List wildcardBuilder = new ArrayList<>(); for (Map.Entry e : extras.entrySet()) { @@ -55,18 +58,17 @@ public void handle(HttpExchange exchange) throws IOException { throw new NotFoundException(exchange.getRequestMethod() + " " + decoded); } - byte[] body = exchange.getRequestBody().readAllBytes(); + RequestBodyReader.Body body = bodyReader.read(exchange); HttpMethod method = HttpMethod.parse(exchange.getRequestMethod()); - var headers = exchange.getRequestHeaders(); Request request = new Request( - body, + body.bytes(), null, null, null, Map.of(), exchange.getRequestURI().getRawQuery(), - headers::getFirst, + body.headerLookup(), Map.of(), method); Response response = hit.handle(request); diff --git a/src/main/java/com/retailsvc/http/internal/ProblemDetail.java b/src/main/java/com/retailsvc/http/internal/ProblemDetail.java index a6888063..9edaf6d5 100644 --- a/src/main/java/com/retailsvc/http/internal/ProblemDetail.java +++ b/src/main/java/com/retailsvc/http/internal/ProblemDetail.java @@ -1,6 +1,7 @@ package com.retailsvc.http.internal; import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; import com.retailsvc.http.BadRequestException; import com.retailsvc.http.validate.ValidationError; @@ -79,27 +80,18 @@ private static int depth(String pointer) { } private static final Map TITLES = - Map.of( - HTTP_BAD_REQUEST, - BAD_REQUEST, - 401, - "Unauthorized", - 403, - "Forbidden", - 404, - "Not Found", - 405, - "Method Not Allowed", - 409, - "Conflict", - 410, - "Gone", - 412, - "Precondition Failed", - 415, - "Unsupported Media Type", - 422, - "Unprocessable Content"); + Map.ofEntries( + Map.entry(HTTP_BAD_REQUEST, BAD_REQUEST), + Map.entry(401, "Unauthorized"), + Map.entry(403, "Forbidden"), + Map.entry(404, "Not Found"), + Map.entry(405, "Method Not Allowed"), + Map.entry(409, "Conflict"), + Map.entry(410, "Gone"), + Map.entry(412, "Precondition Failed"), + Map.entry(HTTP_ENTITY_TOO_LARGE, "Content Too Large"), + Map.entry(415, "Unsupported Media Type"), + Map.entry(422, "Unprocessable Content")); private static String titleFor(int status) { return TITLES.getOrDefault(status, BAD_REQUEST); diff --git a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java new file mode 100644 index 00000000..3526d1ba --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java @@ -0,0 +1,112 @@ +package com.retailsvc.http.internal; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; + +import com.retailsvc.http.BadRequestException; +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpExchange; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.EOFException; +import java.io.IOException; +import java.util.function.UnaryOperator; +import java.util.zip.GZIPInputStream; +import java.util.zip.ZipException; + +/** + * Reads the raw request body, transparently inflating a gzip {@code Content-Encoding} under a hard + * cap on the decompressed size. Immutable and shared across requests. + */ +public final class RequestBodyReader { + + /** Default ceiling on the inflated size of a gzip request body: 10 MiB. */ + public static final long DEFAULT_MAX_DECOMPRESSED_BYTES = 10L * 1024 * 1024; + + private static final String CONTENT_ENCODING = "Content-Encoding"; + private static final String CONTENT_LENGTH = "Content-Length"; + private static final int BUFFER_SIZE = 8192; + + private final long maxDecompressedBytes; + + public RequestBodyReader(long maxDecompressedBytes) { + if (maxDecompressedBytes <= 0) { + throw new IllegalArgumentException( + "maxDecompressedBytes must be positive, got " + maxDecompressedBytes); + } + this.maxDecompressedBytes = maxDecompressedBytes; + } + + /** + * Reads and decodes the request body. + * + * @throws BadRequestException 415 when the coding is not one this server decodes, 413 when the + * inflated body exceeds the cap, 400 when the gzip stream is malformed or truncated + */ + public Body read(HttpExchange exchange) throws IOException { + Headers headers = exchange.getRequestHeaders(); + String header = headers.getFirst(CONTENT_ENCODING); + byte[] raw = exchange.getRequestBody().readAllBytes(); + return switch (ContentEncodingHeader.parse(header)) { + case NONE -> new Body(raw, headers::getFirst); + case GZIP -> decoded(inflate(raw), headers); + case UNSUPPORTED -> + throw new BadRequestException( + HTTP_UNSUPPORTED_TYPE, "unsupported Content-Encoding: " + header); + }; + } + + /** + * Inflates a complete gzip member. The body is buffered before inflating so that an empty body + * stays an empty body — inflating the exchange stream directly would fail at construction and be + * indistinguishable from a truncated stream. + */ + private byte[] inflate(byte[] raw) throws IOException { + if (raw.length == 0) { + return raw; + } + try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(raw), BUFFER_SIZE)) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[BUFFER_SIZE]; + long total = 0; + int read; + while ((read = in.read(buffer)) != -1) { + total += read; + if (total > maxDecompressedBytes) { + throw new BadRequestException( + HTTP_ENTITY_TOO_LARGE, + "decompressed request body exceeds " + maxDecompressedBytes + " bytes"); + } + out.write(buffer, 0, read); + } + return out.toByteArray(); + } catch (ZipException | EOFException e) { + throw new BadRequestException(HTTP_BAD_REQUEST, "malformed gzip request body", e); + } + } + + /** + * Presents the inflated body as if it had arrived uncoded: the coding is gone, so reporting it + * alongside the decoded bytes would misdescribe them, and the stored length measures the + * compressed payload rather than what the handler can read. + */ + private static Body decoded(byte[] bytes, Headers headers) { + String inflatedLength = Integer.toString(bytes.length); + return new Body( + bytes, + name -> { + if (CONTENT_ENCODING.equalsIgnoreCase(name)) { + return null; + } + if (CONTENT_LENGTH.equalsIgnoreCase(name)) { + return inflatedLength; + } + return headers.getFirst(name); + }); + } + + /** A decoded request body and the header view a handler should see alongside it. */ + @SuppressWarnings("java:S6218") + public record Body(byte[] bytes, UnaryOperator headerLookup) {} +} diff --git a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java index 3365d250..8d773d58 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java +++ b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java @@ -40,6 +40,7 @@ public final class RequestPreparationFilter extends Filter { private final ExceptionHandler exceptionHandler; private final ResponseRenderer renderer; private final List afterHooks; + private final RequestBodyReader bodyReader; @SuppressWarnings("java:S107") public RequestPreparationFilter( @@ -49,7 +50,8 @@ public RequestPreparationFilter( Map bodyMappers, ExceptionHandler exceptionHandler, ResponseRenderer renderer, - List afterHooks) { + List afterHooks, + RequestBodyReader bodyReader) { this.spec = spec; this.router = router; this.validator = validator; @@ -57,6 +59,7 @@ public RequestPreparationFilter( this.exceptionHandler = exceptionHandler; this.renderer = renderer; this.afterHooks = List.copyOf(afterHooks); + this.bodyReader = bodyReader; } @Override @@ -94,7 +97,8 @@ public void doFilter(HttpExchange exchange, Chain chain) throws IOException { } private Request buildRequest(HttpExchange exchange) throws IOException { - byte[] body = exchange.getRequestBody().readAllBytes(); + RequestBodyReader.Body decoded = bodyReader.read(exchange); + byte[] body = decoded.bytes(); HttpMethod method = HttpMethod.parse(exchange.getRequestMethod()); String path = stripBasePath(exchange.getRequestURI().getPath()); @@ -113,7 +117,6 @@ private Request buildRequest(HttpExchange exchange) throws IOException { validateParameters(exchange, op, match.pathParameters()); ParsedBody parsedBody = validateAndParseBody(exchange, op, body); - var headers = exchange.getRequestHeaders(); return new Request( body, parsedBody.value(), @@ -121,7 +124,7 @@ private Request buildRequest(HttpExchange exchange) throws IOException { op.operationId(), match.pathParameters(), exchange.getRequestURI().getRawQuery(), - headers::getFirst, + decoded.headerLookup(), Map.of(), method); } diff --git a/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java new file mode 100644 index 00000000..4574674c --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java @@ -0,0 +1,94 @@ +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class AcceptEncodingHeaderTest { + + @Test + void nullHeaderIsNotAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip(null)).isFalse(); + } + + @Test + void blankHeaderIsNotAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip(" ")).isFalse(); + } + + @Test + void plainGzipIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip")).isTrue(); + } + + @Test + void gzipAmongOtherCodingsIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("br, deflate, gzip")).isTrue(); + } + + @Test + void caseInsensitiveGzipIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("GZip")).isTrue(); + } + + @Test + void xGzipIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("x-gzip")).isTrue(); + } + + @Test + void explicitZeroQValueIsRefused() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0")).isFalse(); + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0.0")).isFalse(); + } + + @Test + void positiveQValueIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0.5")).isTrue(); + } + + @Test + void wildcardIsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("*")).isTrue(); + } + + @Test + void wildcardWithZeroQValueIsRefused() { + assertThat(AcceptEncodingHeader.acceptsGzip("*;q=0")).isFalse(); + } + + @Test + void explicitGzipBeatsWildcardRefusal() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip, *;q=0")).isTrue(); + } + + @Test + void explicitGzipRefusalBeatsWildcard() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0, *")).isFalse(); + } + + @Test + void identityOnlyIsNotAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("identity")).isFalse(); + } + + @Test + void deflateOnlyIsNotAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("deflate, br")).isFalse(); + } + + @Test + void surroundingWhitespaceIsTolerated() { + assertThat(AcceptEncodingHeader.acceptsGzip(" deflate , gzip ; q=0.8 ")).isTrue(); + } + + @Test + void malformedQValueIsTreatedAsAccepted() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=bogus")).isTrue(); + } + + @Test + void emptyTokensAreIgnored() { + assertThat(AcceptEncodingHeader.acceptsGzip("deflate,,gzip")).isTrue(); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java b/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java new file mode 100644 index 00000000..67019ac8 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java @@ -0,0 +1,67 @@ +package com.retailsvc.http.internal; + +import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.GZIP; +import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.NONE; +import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.UNSUPPORTED; +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class ContentEncodingHeaderTest { + + @Test + void nullHeaderIsNone() { + assertThat(ContentEncodingHeader.parse(null)).isEqualTo(NONE); + } + + @Test + void emptyHeaderIsNone() { + assertThat(ContentEncodingHeader.parse(" ")).isEqualTo(NONE); + } + + @Test + void identityIsNone() { + assertThat(ContentEncodingHeader.parse("identity")).isEqualTo(NONE); + } + + @Test + void gzipIsGzip() { + assertThat(ContentEncodingHeader.parse("gzip")).isEqualTo(GZIP); + } + + @Test + void xGzipIsGzip() { + assertThat(ContentEncodingHeader.parse("x-gzip")).isEqualTo(GZIP); + } + + @Test + void mixedCaseGzipIsGzip() { + assertThat(ContentEncodingHeader.parse("GZip")).isEqualTo(GZIP); + } + + @Test + void gzipWithIdentityIsGzip() { + assertThat(ContentEncodingHeader.parse("identity, gzip")).isEqualTo(GZIP); + } + + @Test + void surroundingWhitespaceIsTolerated() { + assertThat(ContentEncodingHeader.parse(" gzip ")).isEqualTo(GZIP); + } + + @Test + void brotliIsUnsupported() { + assertThat(ContentEncodingHeader.parse("br")).isEqualTo(UNSUPPORTED); + } + + @Test + void deflateIsUnsupported() { + assertThat(ContentEncodingHeader.parse("deflate")).isEqualTo(UNSUPPORTED); + } + + @Test + void stackedCodingsAreUnsupported() { + assertThat(ContentEncodingHeader.parse("gzip, gzip")).isEqualTo(UNSUPPORTED); + assertThat(ContentEncodingHeader.parse("gzip, br")).isEqualTo(UNSUPPORTED); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java index b8c17e7c..0475d66a 100644 --- a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java @@ -15,10 +15,13 @@ import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.Test; class ExtrasRouterTest { @@ -120,19 +123,53 @@ void traversalRejected() { .isInstanceOf(BadRequestException.class); } + @Test + void gzipRequestBodyIsInflatedForExtraRoutes() throws Exception { + AtomicReference seen = new AtomicReference<>(); + Map extras = new LinkedHashMap<>(); + extras.put( + "/echo", + req -> { + seen.set(new String(req.bytes(), StandardCharsets.UTF_8)); + return Response.empty(); + }); + Headers headers = new Headers(); + headers.add("Content-Encoding", "gzip"); + + invoke(newRouter(extras), "/echo", gzip("hello".getBytes(StandardCharsets.UTF_8)), headers); + + assertThat(seen.get()).isEqualTo("hello"); + } + private static ExtrasRouter newRouter(Map extras) { Map mappers = Map.of("application/json", new GsonTypeMapper()); - return new ExtrasRouter(extras, new ResponseRenderer(mappers)); + return new ExtrasRouter( + extras, + new ResponseRenderer(mappers), + new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } private static void invoke(ExtrasRouter router, String path) throws Exception { + invoke(router, path, new byte[0], new Headers()); + } + + private static void invoke(ExtrasRouter router, String path, byte[] body, Headers headers) + throws Exception { HttpExchange ex = mock(HttpExchange.class); when(ex.getRequestMethod()).thenReturn("GET"); when(ex.getRequestURI()).thenReturn(URI.create(path)); - when(ex.getRequestHeaders()).thenReturn(new Headers()); - when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(new byte[0])); + when(ex.getRequestHeaders()).thenReturn(headers); + when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body)); when(ex.getResponseHeaders()).thenReturn(new Headers()); when(ex.getResponseBody()).thenReturn(new ByteArrayOutputStream()); router.handle(ex); } + + private static byte[] gzip(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { + gzip.write(data); + } + return out.toByteArray(); + } } diff --git a/src/test/java/com/retailsvc/http/internal/ProblemDetailTest.java b/src/test/java/com/retailsvc/http/internal/ProblemDetailTest.java index f00aba5e..346f705f 100644 --- a/src/test/java/com/retailsvc/http/internal/ProblemDetailTest.java +++ b/src/test/java/com/retailsvc/http/internal/ProblemDetailTest.java @@ -1,5 +1,7 @@ package com.retailsvc.http.internal; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import static org.assertj.core.api.Assertions.assertThat; import com.retailsvc.http.BadRequestException; @@ -106,4 +108,17 @@ void badRequestWithPointerBecomesSingleEntry() { var pd = ProblemDetail.forBadRequest(new BadRequestException(409, "taken", "/email", "unique")); assertThat(pd.errors()).containsExactly(new Entry("#/email", "unique", "taken")); } + + @Test + void contentTooLargeHasItsOwnTitle() { + var pd = ProblemDetail.forBadRequest(new BadRequestException(HTTP_ENTITY_TOO_LARGE, "too big")); + assertThat(pd.title()).isEqualTo("Content Too Large"); + assertThat(pd.status()).isEqualTo(HTTP_ENTITY_TOO_LARGE); + } + + @Test + void unsupportedMediaTypeKeepsItsTitle() { + var pd = ProblemDetail.forBadRequest(new BadRequestException(HTTP_UNSUPPORTED_TYPE, "nope")); + assertThat(pd.title()).isEqualTo("Unsupported Media Type"); + } } diff --git a/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java new file mode 100644 index 00000000..09d0d297 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java @@ -0,0 +1,173 @@ +package com.retailsvc.http.internal; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.retailsvc.http.BadRequestException; +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpExchange; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Arrays; +import java.util.zip.GZIPOutputStream; +import java.util.zip.ZipException; +import org.junit.jupiter.api.Test; + +class RequestBodyReaderTest { + + private static final long CAP = 1024; + private final RequestBodyReader reader = new RequestBodyReader(CAP); + + @Test + void constructorRejectsNonPositiveCap() { + assertThatThrownBy(() -> new RequestBodyReader(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("maxDecompressedBytes"); + assertThatThrownBy(() -> new RequestBodyReader(-1)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void plainBodyIsReturnedUnchanged() throws IOException { + byte[] raw = "hello".getBytes(UTF_8); + + RequestBodyReader.Body body = reader.read(exchange(raw, null)); + + assertThat(body.bytes()).isEqualTo(raw); + } + + @Test + void identityCodedBodyIsReturnedUnchanged() throws IOException { + byte[] raw = "hello".getBytes(UTF_8); + + RequestBodyReader.Body body = reader.read(exchange(raw, "identity")); + + assertThat(body.bytes()).isEqualTo(raw); + } + + @Test + void gzipBodyIsInflated() throws IOException { + byte[] plain = "hello gzip".getBytes(UTF_8); + + RequestBodyReader.Body body = reader.read(exchange(gzip(plain), "gzip")); + + assertThat(body.bytes()).isEqualTo(plain); + } + + @Test + void emptyGzipBodyIsReturnedEmpty() throws IOException { + RequestBodyReader.Body body = reader.read(exchange(new byte[0], "gzip")); + + assertThat(body.bytes()).isEmpty(); + } + + @Test + void unsupportedCodingThrows415() { + assertThatThrownBy(() -> reader.read(exchange("x".getBytes(UTF_8), "br"))) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> assertThat(e.status()).isEqualTo(HTTP_UNSUPPORTED_TYPE)); + } + + @Test + void oversizedInflatedBodyThrows413() throws IOException { + byte[] bomb = new byte[(int) CAP * 4]; + + assertThatThrownBy(() -> reader.read(exchange(gzip(bomb), "gzip"))) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> assertThat(e.status()).isEqualTo(HTTP_ENTITY_TOO_LARGE)); + } + + @Test + void bodyExactlyAtCapIsAccepted() throws IOException { + byte[] atLimit = new byte[(int) CAP]; + + RequestBodyReader.Body body = reader.read(exchange(gzip(atLimit), "gzip")); + + assertThat(body.bytes()).hasSize((int) CAP); + } + + @Test + void malformedGzipThrows400WithCause() { + byte[] garbage = "not gzip at all".getBytes(UTF_8); + + assertThatThrownBy(() -> reader.read(exchange(garbage, "gzip"))) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> { + assertThat(e.status()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(e.getCause()).isInstanceOf(ZipException.class); + }); + } + + @Test + void truncatedGzipThrows400() throws IOException { + byte[] complete = gzip("some reasonably long payload to truncate".getBytes(UTF_8)); + byte[] truncated = Arrays.copyOf(complete, complete.length - 6); + + assertThatThrownBy(() -> reader.read(exchange(truncated, "gzip"))) + .isInstanceOfSatisfying( + BadRequestException.class, e -> assertThat(e.status()).isEqualTo(HTTP_BAD_REQUEST)); + } + + @Test + void decodedBodyHidesContentEncodingHeader() throws IOException { + RequestBodyReader.Body body = reader.read(exchange(gzip("hi".getBytes(UTF_8)), "gzip")); + + assertThat(body.headerLookup().apply("Content-Encoding")).isNull(); + assertThat(body.headerLookup().apply("content-encoding")).isNull(); + } + + @Test + void decodedBodyReportsInflatedContentLength() throws IOException { + byte[] plain = "hello gzip".getBytes(UTF_8); + + RequestBodyReader.Body body = reader.read(exchange(gzip(plain), "gzip")); + + assertThat(body.headerLookup().apply("Content-Length")).isEqualTo(String.valueOf(plain.length)); + } + + @Test + void decodedBodyLeavesOtherHeadersVisible() throws IOException { + RequestBodyReader.Body body = reader.read(exchange(gzip("hi".getBytes(UTF_8)), "gzip")); + + assertThat(body.headerLookup().apply("X-Custom")).isEqualTo("value"); + } + + @Test + void plainBodyKeepsOriginalHeaderLookup() throws IOException { + RequestBodyReader.Body body = reader.read(exchange("hi".getBytes(UTF_8), "identity")); + + assertThat(body.headerLookup().apply("Content-Encoding")).isEqualTo("identity"); + assertThat(body.headerLookup().apply("X-Custom")).isEqualTo("value"); + } + + private static HttpExchange exchange(byte[] body, String contentEncoding) { + Headers headers = new Headers(); + if (contentEncoding != null) { + headers.add("Content-Encoding", contentEncoding); + } + headers.add("Content-Length", String.valueOf(body.length)); + headers.add("X-Custom", "value"); + HttpExchange exchange = mock(HttpExchange.class); + when(exchange.getRequestHeaders()).thenReturn(headers); + when(exchange.getRequestBody()).thenReturn(new ByteArrayInputStream(body)); + return exchange; + } + + private static byte[] gzip(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { + gzip.write(data); + } + return out.toByteArray(); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index d37ee9c5..65b3fb54 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -1,9 +1,11 @@ package com.retailsvc.http.internal; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; +import com.retailsvc.http.BadRequestException; import com.retailsvc.http.ExceptionHandler; import com.retailsvc.http.MethodNotAllowedException; import com.retailsvc.http.NotFoundException; @@ -27,6 +29,8 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.List; @@ -34,20 +38,39 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.Test; import org.mockito.Mockito; class RequestPreparationFilterTest { private HttpExchange exchange(String method, String path, byte[] body) { + return exchange(method, path, body, new Headers()); + } + + private HttpExchange exchange(String method, String path, byte[] body, Headers headers) { HttpExchange ex = mock(HttpExchange.class); Mockito.when(ex.getRequestMethod()).thenReturn(method); Mockito.when(ex.getRequestURI()).thenReturn(URI.create(path)); - Mockito.when(ex.getRequestHeaders()).thenReturn(new Headers()); + Mockito.when(ex.getRequestHeaders()).thenReturn(headers); Mockito.when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body)); return ex; } + private static Headers headers(String name, String value) { + Headers headers = new Headers(); + headers.add(name, value); + return headers; + } + + private static byte[] gzip(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { + gzip.write(data); + } + return out.toByteArray(); + } + private Spec specWith(Operation... ops) { return new Spec( "3.1.0", @@ -92,7 +115,8 @@ public byte[] writeTo(Object value) { mappers, rethrow, new ResponseRenderer(mappers), - List.of()); + List.of(), + new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } @Test @@ -338,4 +362,60 @@ void booleanQueryParamRejectsNonBooleanString() { .extracting(t -> ((ValidationException) t).error().keyword()) .isEqualTo("type"); } + + @Test + void gzipRequestBodyIsInflatedBeforeValidation() throws Exception { + var op = + new Operation( + "get-x", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(), + Map.of(), + Map.of(), + Optional.empty()); + Filter f = newFilter(specWith(op)); + byte[] plain = "hello gzip".getBytes(StandardCharsets.UTF_8); + HttpExchange ex = exchange("GET", "/x", gzip(plain), headers("Content-Encoding", "gzip")); + + AtomicReference seenBody = new AtomicReference<>(); + AtomicReference seenEncoding = new AtomicReference<>("still here"); + Filter.Chain chain = mock(Filter.Chain.class); + Mockito.doAnswer( + inv -> { + Request req = DispatchHandler.CURRENT.get(); + seenBody.set(req.bytes()); + seenEncoding.set(req.header("Content-Encoding").orElse(null)); + return null; + }) + .when(chain) + .doFilter(Mockito.any()); + + f.doFilter(ex, chain); + + assertThat(seenBody.get()).isEqualTo(plain); + assertThat(seenEncoding.get()).isNull(); + } + + @Test + void unsupportedRequestCodingIsRejectedBeforeRouting() { + var op = + new Operation( + "get-x", + HttpMethod.GET, + PathTemplate.compile("/x"), + Optional.empty(), + List.of(), + Map.of(), + Map.of(), + Optional.empty()); + Filter f = newFilter(specWith(op)); + HttpExchange ex = exchange("GET", "/missing", new byte[0], headers("Content-Encoding", "br")); + + assertThatThrownBy(() -> f.doFilter(ex, mock(Filter.Chain.class))) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> assertThat(e.status()).isEqualTo(HTTP_UNSUPPORTED_TYPE)); + } } From efe2362807c734f93276029c03dd9641eb3623ab Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:42:46 +0200 Subject: [PATCH 02/16] feat: Compress responses when the client accepts gzip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Response bodies are now gzipped when the client sends `Accept-Encoding: gzip`, the media type is text-shaped, and the payload clears a 1 KiB threshold. Compressing tiny payloads costs more than it saves, and already-compressed media gains nothing. The step lives in ResponseRenderer, the one point every response flows through, so problem+json errors, the health endpoint, served specs and 404s are all covered. A handler that coded the body itself is left alone, as is a payload gzip fails to shrink. Statuses that carry no content never get a coding. Streamed bodies are deflated as they are written. A sized body's declared length measures the uncoded form, so a coded stream degrades to chunked; a body of unknown length is coded regardless of the threshold, since measuring it would defeat streaming it. `Vary: Accept-Encoding` is announced whenever a body could have been coded, not only when it was, and is merged into any Vary the handler already set rather than added as a second field line. Two fixes fall out of routing bodiless responses through the same path: they now carry the Content-Type the handler declared, and they drop a hand-declared Content-Length when the matching GET would have been compressed — HEAD must not advertise a length the coded body will not match. --- docs/plans/dynamic-discovering-piglet.md | 6 +- .../http/internal/ResponseCompression.java | 68 ++++ .../http/internal/ResponseRenderer.java | 154 +++++++- .../http/internal/DispatchHandlerTest.java | 1 + .../internal/ResponseCompressionTest.java | 94 +++++ .../http/internal/ResponseRendererTest.java | 362 ++++++++++++++++++ 6 files changed, 675 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/retailsvc/http/internal/ResponseCompression.java create mode 100644 src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java create mode 100644 src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java diff --git a/docs/plans/dynamic-discovering-piglet.md b/docs/plans/dynamic-discovering-piglet.md index a23af35c..c1dc2ace 100644 --- a/docs/plans/dynamic-discovering-piglet.md +++ b/docs/plans/dynamic-discovering-piglet.md @@ -246,12 +246,12 @@ as it completes. ### Task 3 — response compression -- [ ] **Step 6** `internal/ResponseCompressionTest` — `nullContentTypeIsNotCompressible`, +- [x] **Step 6** `internal/ResponseCompressionTest` — `nullContentTypeIsNotCompressible`, `jsonIsCompressible`, `problemJsonIsCompressible`, `yamlIsCompressible`, `textPlainWithCharsetIsCompressible`, `xmlSuffixIsCompressible`, `octetStreamIsNotCompressible`, `imagePngIsNotCompressible`, `eventStreamIsNotCompressible`, `gzipRoundTripsBytes`, `gzipStreamRoundTripsBytes`. Then implement. -- [ ] **Step 7** `internal/ResponseRendererTest` — the repo's **first direct renderer test**, so it +- [x] **Step 7** `internal/ResponseRendererTest` — the repo's **first direct renderer test**, so it starts with baseline coverage of behaviour it is about to change (`writesBytesWithContentLength`, `writesNullBodyWithMinusOne`, `writesSizedStreamWithLength`), then: `compressesJsonBodyOverThreshold`, `setsContentEncodingGzipWhenCompressed`, @@ -267,7 +267,7 @@ as it completes. `DispatchHandlerTest.stubExchange()` — it stubs only `getResponseHeaders()` today, so an unstubbed `getRequestHeaders()` returns null. Fixing the stub is honest and removes a fragile dependency on evaluation order in main code. -- [ ] **Step 8** Streaming and null bodies, same test class: +- [x] **Step 8** Streaming and null bodies, same test class: `compressesChunkedStreamWhenAcceptEncodingPresent`, `degradesSizedStreamToChunkedWhenCompressed`, `skipsCompressionForSizedStreamBelowThreshold`, `skipsCompressionForNullContentTypeStream`, diff --git a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java new file mode 100644 index 00000000..1970148e --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java @@ -0,0 +1,68 @@ +package com.retailsvc.http.internal; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.util.Set; +import java.util.zip.GZIPOutputStream; + +/** Response content-coding policy, and the gzip primitives the renderer writes through. */ +public final class ResponseCompression { + + private static final String TEXT_PREFIX = "text/"; + private static final String EVENT_STREAM = "text/event-stream"; + + private static final Set COMPRESSIBLE_TYPES = + Set.of( + "application/json", + "application/xml", + "application/yaml", + "application/x-yaml", + "application/javascript", + "application/x-ndjson"); + + private static final Set COMPRESSIBLE_SUFFIXES = Set.of("+json", "+xml", "+yaml"); + + private ResponseCompression() {} + + /** + * Whether a response of this content type is worth gzipping. Already-compressed payloads gain + * nothing, and {@code text/event-stream} must stay unbuffered so each event reaches the client as + * it is written. + * + *

An absent content type is never compressible. It cannot be resolved through {@link + * ContentTypeHeader#mediaType} here, because that reads {@code null} as {@code application/json}. + */ + public static boolean isCompressible(String contentType) { + if (contentType == null) { + return false; + } + String mediaType = ContentTypeHeader.mediaType(contentType); + if (mediaType.startsWith(TEXT_PREFIX)) { + return !EVENT_STREAM.equals(mediaType); + } + for (String suffix : COMPRESSIBLE_SUFFIXES) { + if (mediaType.endsWith(suffix)) { + return true; + } + } + return COMPRESSIBLE_TYPES.contains(mediaType); + } + + /** Deflates {@code body} into a complete gzip member. */ + public static byte[] gzip(byte[] body) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { + gzip.write(body); + } + return out.toByteArray(); + } + + /** + * Wraps {@code out} so a streamed body is deflated as it is written. Closing the returned stream + * writes the gzip trailer and releases the deflater's native memory, so the caller must close it. + */ + public static OutputStream gzipStream(OutputStream out) throws IOException { + return new GZIPOutputStream(out); + } +} diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 204935f8..580d39b2 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -1,5 +1,11 @@ package com.retailsvc.http.internal; +import static java.net.HttpURLConnection.HTTP_NOT_MODIFIED; +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; +import static java.net.HttpURLConnection.HTTP_OK; +import static java.net.HttpURLConnection.HTTP_PARTIAL; +import static java.net.HttpURLConnection.HTTP_RESET; + import com.retailsvc.http.Response; import com.retailsvc.http.TypeMapper; import com.sun.net.httpserver.Headers; @@ -12,14 +18,30 @@ /** Writes a {@link Response} to an {@link HttpExchange}. */ public final class ResponseRenderer { + /** Default smallest body worth gzipping: 1 KiB. */ + public static final long DEFAULT_MINIMUM_GZIP_BYTES = 1024; + private static final String CONTENT_TYPE = "Content-Type"; + private static final String CONTENT_ENCODING = "Content-Encoding"; + private static final String CONTENT_LENGTH = "Content-Length"; + private static final String VARY = "Vary"; + private static final String ACCEPT_ENCODING = "Accept-Encoding"; + private static final String GZIP = "gzip"; + private static final long UNKNOWN_LENGTH = -1; + private static final long CHUNKED = 0; private static final String DEFAULT_JSON = "application/json"; private static final String OCTET_STREAM = "application/octet-stream"; private final Map mappers; + private final long minimumGzipBytes; public ResponseRenderer(Map mappers) { + this(mappers, DEFAULT_MINIMUM_GZIP_BYTES); + } + + public ResponseRenderer(Map mappers, long minimumGzipBytes) { this.mappers = Map.copyOf(mappers); + this.minimumGzipBytes = minimumGzipBytes; } public void render(HttpExchange exchange, Response response) throws IOException { @@ -31,7 +53,7 @@ public void render(HttpExchange exchange, Response response) throws IOException int status = response.status(); if (body == null) { - exchange.sendResponseHeaders(status, -1); + renderEmpty(exchange, headers, status, response.contentType()); } else if (body instanceof BodyWriter writer) { renderStream(exchange, headers, status, response.contentType(), writer); } else { @@ -40,19 +62,78 @@ public void render(HttpExchange exchange, Response response) throws IOException } } - private static void renderStream( + /** + * Writes a bodiless response. Nothing can be coded here, but the response still has to say how a + * body would have been coded: a length declared for a body the client will fetch separately would + * describe the uncoded form, which is not what a coded {@code GET} would return. + */ + private void renderEmpty(HttpExchange exchange, Headers headers, int status, String contentType) + throws IOException { + if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { + headers.add(CONTENT_TYPE, contentType); + } + if (!headers.containsKey(CONTENT_ENCODING) + && ResponseCompression.isCompressible(contentType) + && bodyAllowed(status)) { + addVary(headers); + if (declaredLength(headers) >= minimumGzipBytes && acceptsGzip(exchange)) { + headers.remove(CONTENT_LENGTH); + } + } + exchange.sendResponseHeaders(status, -1); + } + + private void renderStream( HttpExchange exchange, Headers headers, int status, String contentType, BodyWriter writer) throws IOException { if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { headers.add(CONTENT_TYPE, contentType); } - long length = writer instanceof BodyWriter.Sized sized ? sized.length() : 0; - exchange.sendResponseHeaders(status, length); + long declared = writer instanceof BodyWriter.Sized sized ? sized.length() : UNKNOWN_LENGTH; + if (compressStream(exchange, headers, status, contentType, declared)) { + headers.set(CONTENT_ENCODING, GZIP); + exchange.sendResponseHeaders(status, CHUNKED); + try (OutputStream out = ResponseCompression.gzipStream(exchange.getResponseBody())) { + writer.writeTo(out); + } + return; + } + exchange.sendResponseHeaders(status, Math.max(declared, CHUNKED)); try (OutputStream out = exchange.getResponseBody()) { writer.writeTo(out); } } + /** + * A coded stream has to go out chunked, because the length a {@code Sized} body declares measures + * the uncoded form. A body of unknown length is compressed regardless of the threshold — + * buffering it to find out how big it is would defeat streaming it. + */ + private boolean compressStream( + HttpExchange exchange, Headers headers, int status, String contentType, long declaredLength) { + if (headers.containsKey(CONTENT_ENCODING) + || !ResponseCompression.isCompressible(contentType) + || !bodyAllowed(status)) { + return false; + } + addVary(headers); + boolean worthCoding = declaredLength < 0 || declaredLength >= minimumGzipBytes; + return worthCoding && acceptsGzip(exchange); + } + + /** The length a handler declared for a body it did not write, or -1 when absent or unreadable. */ + private static long declaredLength(Headers headers) { + String declared = headers.getFirst(CONTENT_LENGTH); + if (declared == null) { + return UNKNOWN_LENGTH; + } + try { + return Long.parseLong(declared.trim()); + } catch (NumberFormatException e) { + return UNKNOWN_LENGTH; + } + } + private void renderBytes( HttpExchange exchange, Headers headers, int status, String contentType, Object body) throws IOException { @@ -68,12 +149,71 @@ private void renderBytes( if (!headers.containsKey(CONTENT_TYPE)) { headers.add(CONTENT_TYPE, effectiveContentType); } - exchange.sendResponseHeaders(status, bytes.length == 0 ? -1 : bytes.length); - if (bytes.length > 0) { + byte[] payload = maybeCompress(exchange, headers, status, effectiveContentType, bytes); + exchange.sendResponseHeaders(status, payload.length == 0 ? -1 : payload.length); + if (payload.length > 0) { try (OutputStream out = exchange.getResponseBody()) { - out.write(bytes); + out.write(payload); + } + } + } + + /** + * Gzips the body when the client asked for it and the payload is big enough to be worth it. A + * handler that coded the body itself is left alone, and so is a payload that gzip fails to + * shrink. + */ + private byte[] maybeCompress( + HttpExchange exchange, Headers headers, int status, String contentType, byte[] bytes) + throws IOException { + if (headers.containsKey(CONTENT_ENCODING) + || !ResponseCompression.isCompressible(contentType) + || !bodyAllowed(status)) { + return bytes; + } + addVary(headers); + if (bytes.length < minimumGzipBytes || !acceptsGzip(exchange)) { + return bytes; + } + byte[] gzipped = ResponseCompression.gzip(bytes); + if (gzipped.length >= bytes.length) { + return bytes; + } + headers.set(CONTENT_ENCODING, GZIP); + return gzipped; + } + + /** Statuses that carry no content cannot carry a content coding either. */ + private static boolean bodyAllowed(int status) { + return status >= HTTP_OK + && status != HTTP_NO_CONTENT + && status != HTTP_RESET + && status != HTTP_PARTIAL + && status != HTTP_NOT_MODIFIED; + } + + private static boolean acceptsGzip(HttpExchange exchange) { + return AcceptEncodingHeader.acceptsGzip(exchange.getRequestHeaders().getFirst(ACCEPT_ENCODING)); + } + + /** + * Marks the response as varying by {@code Accept-Encoding} so shared caches keep the coded and + * uncoded forms apart. Announced whenever the body could have been coded, not only when it was, + * and merged into one field line so a client reading a single value sees the whole list. + */ + private static void addVary(Headers headers) { + String existing = headers.getFirst(VARY); + if (existing == null) { + headers.set(VARY, ACCEPT_ENCODING); + return; + } + for (String field : existing.split(",")) { + String trimmed = field.trim(); + if ("*".equals(trimmed) || ACCEPT_ENCODING.equalsIgnoreCase(trimmed)) { + return; } } + headers.set(VARY, existing + ", " + ACCEPT_ENCODING); } private byte[] serialize(Object body, String contentType) { diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index 2c038734..d526f98e 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -29,6 +29,7 @@ class DispatchHandlerTest { private static HttpExchange stubExchange() { HttpExchange exchange = mock(HttpExchange.class); + when(exchange.getRequestHeaders()).thenReturn(new Headers()); when(exchange.getResponseHeaders()).thenReturn(new Headers()); Map attrs = new HashMap<>(); doAnswer( diff --git a/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java new file mode 100644 index 00000000..b5bfdcf8 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java @@ -0,0 +1,94 @@ +package com.retailsvc.http.internal; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.util.zip.GZIPInputStream; +import org.junit.jupiter.api.Test; + +class ResponseCompressionTest { + + @Test + void nullContentTypeIsNotCompressible() { + assertThat(ResponseCompression.isCompressible(null)).isFalse(); + } + + @Test + void jsonIsCompressible() { + assertThat(ResponseCompression.isCompressible("application/json")).isTrue(); + } + + @Test + void problemJsonIsCompressible() { + assertThat(ResponseCompression.isCompressible("application/problem+json")).isTrue(); + } + + @Test + void yamlIsCompressible() { + assertThat(ResponseCompression.isCompressible("application/yaml")).isTrue(); + assertThat(ResponseCompression.isCompressible("application/x-yaml")).isTrue(); + } + + @Test + void textPlainWithCharsetIsCompressible() { + assertThat(ResponseCompression.isCompressible("text/plain; charset=utf-8")).isTrue(); + } + + @Test + void xmlSuffixIsCompressible() { + assertThat(ResponseCompression.isCompressible("image/svg+xml")).isTrue(); + assertThat(ResponseCompression.isCompressible("application/xml")).isTrue(); + } + + @Test + void formUrlEncodedIsNotCompressible() { + assertThat(ResponseCompression.isCompressible("application/octet-stream")).isFalse(); + } + + @Test + void imagePngIsNotCompressible() { + assertThat(ResponseCompression.isCompressible("image/png")).isFalse(); + } + + @Test + void eventStreamIsNotCompressible() { + assertThat(ResponseCompression.isCompressible("text/event-stream")).isFalse(); + } + + @Test + void matchIsCaseInsensitive() { + assertThat(ResponseCompression.isCompressible("Application/JSON")).isTrue(); + } + + @Test + void gzipRoundTripsBytes() throws IOException { + byte[] plain = "round trip me".repeat(20).getBytes(UTF_8); + + byte[] compressed = ResponseCompression.gzip(plain); + + assertThat(compressed).isNotEqualTo(plain); + assertThat(gunzip(compressed)).isEqualTo(plain); + } + + @Test + void gzipStreamRoundTripsBytes() throws IOException { + byte[] plain = "stream me".repeat(20).getBytes(UTF_8); + ByteArrayOutputStream sink = new ByteArrayOutputStream(); + + try (OutputStream out = ResponseCompression.gzipStream(sink)) { + out.write(plain); + } + + assertThat(gunzip(sink.toByteArray())).isEqualTo(plain); + } + + private static byte[] gunzip(byte[] data) throws IOException { + try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) { + return in.readAllBytes(); + } + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java new file mode 100644 index 00000000..2ead5e06 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java @@ -0,0 +1,362 @@ +package com.retailsvc.http.internal; + +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; +import static java.net.HttpURLConnection.HTTP_OK; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.retailsvc.http.GsonTypeMapper; +import com.retailsvc.http.Response; +import com.retailsvc.http.TypeMapper; +import com.sun.net.httpserver.Headers; +import com.sun.net.httpserver.HttpExchange; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Map; +import java.util.Random; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.zip.GZIPInputStream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class ResponseRendererTest { + + private static final Map MAPPERS = + Map.of("application/json", new GsonTypeMapper()); + private static final long THRESHOLD = 1024; + private static final String JSON = "application/json"; + private static final String TEXT = "text/plain"; + private static final String CONTENT_ENCODING = "Content-Encoding"; + private static final String VARY = "Vary"; + + private final ResponseRenderer renderer = new ResponseRenderer(MAPPERS, THRESHOLD); + private final Headers requestHeaders = new Headers(); + private final Headers responseHeaders = new Headers(); + private final ByteArrayOutputStream sink = new ByteArrayOutputStream(); + private final AtomicInteger status = new AtomicInteger(); + private final AtomicLong length = new AtomicLong(); + private HttpExchange exchange; + + @BeforeEach + void setUp() throws IOException { + exchange = mock(HttpExchange.class); + when(exchange.getRequestHeaders()).thenReturn(requestHeaders); + when(exchange.getResponseHeaders()).thenReturn(responseHeaders); + when(exchange.getResponseBody()).thenReturn(sink); + doAnswer( + invocation -> { + status.set(invocation.getArgument(0)); + length.set(invocation.getArgument(1)); + return null; + }) + .when(exchange) + .sendResponseHeaders(anyInt(), anyLong()); + } + + // -- baseline behaviour -- + + @Test + void writesBytesWithContentLength() throws IOException { + renderer.render(exchange, Response.bytes(HTTP_OK, "abc".getBytes(UTF_8), TEXT)); + + assertThat(status.get()).isEqualTo(HTTP_OK); + assertThat(length.get()).isEqualTo(3); + assertThat(sink.toByteArray()).isEqualTo("abc".getBytes(UTF_8)); + assertThat(responseHeaders.getFirst("Content-Type")).isEqualTo(TEXT); + } + + @Test + void writesNullBodyWithMinusOne() throws IOException { + renderer.render(exchange, Response.status(HTTP_NO_CONTENT)); + + assertThat(length.get()).isEqualTo(-1); + assertThat(sink.toByteArray()).isEmpty(); + } + + @Test + void writesEmptyByteBodyWithMinusOne() throws IOException { + renderer.render(exchange, Response.bytes(HTTP_OK, new byte[0], TEXT)); + + assertThat(length.get()).isEqualTo(-1); + } + + // -- compression -- + + @Test + void compressesJsonBodyOverThreshold() throws IOException { + acceptsGzip(); + byte[] body = largeText(); + + renderer.render(exchange, Response.bytes(HTTP_OK, body, JSON)); + + assertThat(gunzip(sink.toByteArray())).isEqualTo(body); + } + + @Test + void setsContentEncodingGzipWhenCompressed() throws IOException { + acceptsGzip(); + + renderer.render(exchange, Response.bytes(HTTP_OK, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + assertThat(responseHeaders.getFirst(VARY)).contains("Accept-Encoding"); + } + + @Test + void sentContentLengthMatchesCompressedPayload() throws IOException { + acceptsGzip(); + + renderer.render(exchange, Response.bytes(HTTP_OK, largeText(), JSON)); + + assertThat(length.get()).isEqualTo(sink.toByteArray().length); + assertThat(length.get()).isLessThan(largeText().length); + } + + @Test + void skipsCompressionBelowThreshold() throws IOException { + acceptsGzip(); + byte[] body = "{\"id\":\"small\"}".getBytes(UTF_8); + + renderer.render(exchange, Response.bytes(HTTP_OK, body, JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(body); + } + + @Test + void skipsCompressionForOctetStream() throws IOException { + acceptsGzip(); + byte[] body = largeText(); + + renderer.render(exchange, Response.bytes(HTTP_OK, body, "application/octet-stream")); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(body); + } + + @Test + void skipsCompressionWithoutAcceptEncoding() throws IOException { + byte[] body = largeText(); + + renderer.render(exchange, Response.bytes(HTTP_OK, body, JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(body); + } + + @Test + void skipsCompressionWhenGzipRefusedByQValue() throws IOException { + requestHeaders.add("Accept-Encoding", "gzip;q=0"); + + renderer.render(exchange, Response.bytes(HTTP_OK, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + } + + @Test + void skipsCompressionWhenHandlerAlreadySetContentEncoding() throws IOException { + acceptsGzip(); + byte[] body = largeText(); + + renderer.render( + exchange, Response.bytes(HTTP_OK, body, JSON).withHeader(CONTENT_ENCODING, "br")); + + assertThat(responseHeaders.get(CONTENT_ENCODING)).containsExactly("br"); + assertThat(sink.toByteArray()).isEqualTo(body); + } + + @Test + void addsVaryEvenWhenNotCompressed() throws IOException { + renderer.render(exchange, Response.bytes(HTTP_OK, "{}".getBytes(UTF_8), JSON)); + + assertThat(responseHeaders.getFirst(VARY)).isEqualTo("Accept-Encoding"); + } + + @Test + void appendsVaryToExistingValue() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, Response.bytes(HTTP_OK, largeText(), JSON).withHeader(VARY, "Origin")); + + assertThat(responseHeaders.get(VARY)).hasSize(1); + assertThat(responseHeaders.getFirst(VARY)).isEqualTo("Origin, Accept-Encoding"); + } + + @Test + void doesNotDuplicateVary() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, Response.bytes(HTTP_OK, largeText(), JSON).withHeader(VARY, "Accept-Encoding")); + + assertThat(responseHeaders.get(VARY)).containsExactly("Accept-Encoding"); + } + + @Test + void neverCompressesNoContentResponses() throws IOException { + acceptsGzip(); + + renderer.render(exchange, Response.bytes(HTTP_NO_CONTENT, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + } + + @Test + void fallsBackToPlainBytesWhenGzipIsLarger() throws IOException { + acceptsGzip(); + byte[] incompressible = new byte[2048]; + new Random(42).nextBytes(incompressible); + + renderer.render(exchange, Response.bytes(HTTP_OK, incompressible, TEXT)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(incompressible); + } + + @Test + void serializedBodyIsCompressed() throws IOException { + acceptsGzip(); + + renderer.render(exchange, Response.ok(Map.of("blob", "x".repeat(4096)))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + assertThat(new String(gunzip(sink.toByteArray()), UTF_8)).contains("xxxx"); + } + + // -- streaming and empty bodies -- + + @Test + void writesSizedStreamWithLength() throws IOException { + renderer.render( + exchange, Response.stream(HTTP_OK, 3, TEXT, out -> out.write("abc".getBytes(UTF_8)))); + + assertThat(length.get()).isEqualTo(3); + assertThat(sink.toByteArray()).isEqualTo("abc".getBytes(UTF_8)); + } + + @Test + void compressesChunkedStreamWhenAcceptEncodingPresent() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render(exchange, Response.stream(HTTP_OK, TEXT, out -> out.write(payload))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + assertThat(length.get()).isZero(); + assertThat(gunzip(sink.toByteArray())).isEqualTo(payload); + } + + @Test + void degradesSizedStreamToChunkedWhenCompressed() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render( + exchange, Response.stream(HTTP_OK, payload.length, TEXT, out -> out.write(payload))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + assertThat(length.get()).isZero(); + assertThat(gunzip(sink.toByteArray())).isEqualTo(payload); + } + + @Test + void skipsCompressionForSizedStreamBelowThreshold() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, Response.stream(HTTP_OK, 3, TEXT, out -> out.write("abc".getBytes(UTF_8)))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(length.get()).isEqualTo(3); + assertThat(sink.toByteArray()).isEqualTo("abc".getBytes(UTF_8)); + } + + @Test + void skipsCompressionForNullContentTypeStream() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render(exchange, Response.stream(HTTP_OK, null, out -> out.write(payload))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(payload); + } + + @Test + void emitsContentTypeOnNullBodyResponses() throws IOException { + renderer.render(exchange, Response.status(HTTP_OK).withContentType("application/yaml")); + + assertThat(responseHeaders.getFirst("Content-Type")).isEqualTo("application/yaml"); + assertThat(length.get()).isEqualTo(-1); + } + + @Test + void stripsContentLengthOnNullBodyWhenGetWouldCompress() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, + Response.status(HTTP_OK) + .withContentType("application/yaml") + .withHeader("Content-Length", "8506")); + + assertThat(responseHeaders.getFirst("Content-Length")).isNull(); + assertThat(responseHeaders.getFirst(VARY)).isEqualTo("Accept-Encoding"); + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + } + + @Test + void keepsContentLengthOnNullBodyWhenClientDoesNotAcceptGzip() throws IOException { + renderer.render( + exchange, + Response.status(HTTP_OK) + .withContentType("application/yaml") + .withHeader("Content-Length", "8506")); + + assertThat(responseHeaders.getFirst("Content-Length")).isEqualTo("8506"); + } + + @Test + void keepsContentLengthOnNullBodyForNonCompressibleType() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, + Response.status(HTTP_OK).withContentType("image/png").withHeader("Content-Length", "8506")); + + assertThat(responseHeaders.getFirst("Content-Length")).isEqualTo("8506"); + } + + @Test + void keepsContentLengthOnNullBodyBelowThreshold() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, + Response.status(HTTP_OK).withContentType(TEXT).withHeader("Content-Length", "12")); + + assertThat(responseHeaders.getFirst("Content-Length")).isEqualTo("12"); + } + + private void acceptsGzip() { + requestHeaders.add("Accept-Encoding", "gzip, deflate, br"); + } + + private static byte[] largeText() { + return ("{\"blob\":\"" + "abcdefgh".repeat(300) + "\"}").getBytes(UTF_8); + } + + private static byte[] gunzip(byte[] data) throws IOException { + try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) { + return in.readAllBytes(); + } + } +} From 33bab95a50c074695c6870d0bb91c80397e22ec8 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:44:12 +0200 Subject: [PATCH 03/16] feat: Make the gzip size limits configurable `maxDecompressedRequestBytes` moves the inflation ceiling off its 10 MiB default, for services whose legitimate payloads are larger. It is capped at Integer.MAX_VALUE because the inflated body is buffered into an array. `minimumGzipResponseBytes` moves the compression threshold off its 1 KiB default. Setting it to 0 compresses every compressible body; setting it above any response this server produces turns compression off, which is what a service behind a proxy that already terminates compression wants. --- docs/plans/dynamic-discovering-piglet.md | 2 +- .../com/retailsvc/http/OpenApiServer.java | 51 +++++++++++++++++-- .../http/OpenApiServerBuilderTest.java | 37 ++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/plans/dynamic-discovering-piglet.md b/docs/plans/dynamic-discovering-piglet.md index c1dc2ace..c086ca5d 100644 --- a/docs/plans/dynamic-discovering-piglet.md +++ b/docs/plans/dynamic-discovering-piglet.md @@ -278,7 +278,7 @@ as it completes. ### Task 4 — builder -- [ ] **Step 9** `OpenApiServerBuilderTest` — `maxDecompressedRequestBytesRejectsZero`, +- [x] **Step 9** `OpenApiServerBuilderTest` — `maxDecompressedRequestBytesRejectsZero`, `maxDecompressedRequestBytesRejectsNegative`, `minimumGzipResponseBytesRejectsNegative`. Then add the two setters, the `HandlerConfig` fields and the `build()` wiring. diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index ef2855af..23ee7187 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -65,7 +65,9 @@ record HandlerConfig( ExceptionHandler exceptionHandler, Map extras, boolean externalAuth, - List afterHooks) {} + List afterHooks, + long maxDecompressedRequestBytes, + long minimumGzipResponseBytes) {} OpenApiServer( List bindings, @@ -93,9 +95,10 @@ record HandlerConfig( this.httpServer = createHttpServer(socketAddress, sslContext); httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); - ResponseRenderer renderer = new ResponseRenderer(bodyMappers); + ResponseRenderer renderer = + new ResponseRenderer(bodyMappers, handlerConfig.minimumGzipResponseBytes()); RequestBodyReader bodyReader = - new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES); + new RequestBodyReader(handlerConfig.maxDecompressedRequestBytes()); boolean anyBindingAtRoot = wireBindings( httpServer, @@ -278,6 +281,8 @@ public static final class Builder { private final LinkedHashMap extras = new LinkedHashMap<>(); private final Map securityValidators = new LinkedHashMap<>(); private boolean externalAuth = false; + private long maxDecompressedRequestBytes = RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES; + private long minimumGzipResponseBytes = ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; private final List bindings = new ArrayList<>(); private Builder() {} @@ -427,6 +432,42 @@ public Builder https(Path certificateChainPem, Path privateKeyPem) { * stops immediately; positive values wait up to that many seconds for in-flight exchanges to * finish. */ + /** + * Ceiling on the inflated size of a gzip request body, 10 MiB by default. A compressed payload + * can expand by orders of magnitude, so this bounds what a single request may allocate; + * exceeding it fails the request with 413. Bodies that arrive uncompressed are not affected. + */ + public Builder maxDecompressedRequestBytes(long maxDecompressedRequestBytes) { + if (maxDecompressedRequestBytes <= 0) { + throw new IllegalArgumentException( + "maxDecompressedRequestBytes must be positive, got " + maxDecompressedRequestBytes); + } + if (maxDecompressedRequestBytes > Integer.MAX_VALUE) { + throw new IllegalArgumentException( + "maxDecompressedRequestBytes must not exceed " + + Integer.MAX_VALUE + + ", got " + + maxDecompressedRequestBytes); + } + this.maxDecompressedRequestBytes = maxDecompressedRequestBytes; + return this; + } + + /** + * Smallest response body worth gzipping, 1 KiB by default. Below this, the coding costs more + * than it saves. Set it to 0 to compress every compressible body, or high enough to exceed any + * response this server produces to stop compressing altogether — useful when a proxy in front + * already terminates compression. + */ + public Builder minimumGzipResponseBytes(long minimumGzipResponseBytes) { + if (minimumGzipResponseBytes < 0) { + throw new IllegalArgumentException( + "minimumGzipResponseBytes must be non-negative, got " + minimumGzipResponseBytes); + } + this.minimumGzipResponseBytes = minimumGzipResponseBytes; + return this; + } + public Builder shutdownTimeoutSeconds(int shutdownTimeoutSeconds) { if (shutdownTimeoutSeconds < 0) { throw new IllegalArgumentException( @@ -468,7 +509,9 @@ public OpenApiServer build() throws IOException { effectiveExceptionHandler, extras, externalAuth, - List.copyOf(afterHooks)); + List.copyOf(afterHooks), + maxDecompressedRequestBytes, + minimumGzipResponseBytes); int resolvedPort = resolvePort(); SSLContext sslContext = httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; diff --git a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java index f0f1bc2c..8a99c4ce 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java @@ -1,6 +1,7 @@ package com.retailsvc.http; import static java.util.Collections.emptyMap; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -49,6 +50,42 @@ void rejectsExtraPathEqualToSpecBasePathAtBuildTime() { .hasMessageContaining("/api"); } + @Test + void rejectsNonPositiveMaxDecompressedRequestBytes() { + OpenApiServer.Builder b = OpenApiServer.builder(); + + assertThatThrownBy(() -> b.maxDecompressedRequestBytes(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("0"); + assertThatThrownBy(() -> b.maxDecompressedRequestBytes(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("-1"); + } + + @Test + void rejectsOversizedMaxDecompressedRequestBytes() { + OpenApiServer.Builder b = OpenApiServer.builder(); + + assertThatThrownBy(() -> b.maxDecompressedRequestBytes(Integer.MAX_VALUE + 1L)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void rejectsNegativeMinimumGzipResponseBytes() { + OpenApiServer.Builder b = OpenApiServer.builder(); + + assertThatThrownBy(() -> b.minimumGzipResponseBytes(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("-1"); + } + + @Test + void acceptsContentCodingLimits() { + OpenApiServer.Builder b = OpenApiServer.builder(); + + assertThat(b.maxDecompressedRequestBytes(4096).minimumGzipResponseBytes(0)).isSameAs(b); + } + @Test void rejectsNegativeShutdownTimeout() { OpenApiServer.Builder b = OpenApiServer.builder(); From eabbe31f24b9e99e188de0f2e8b2a68b320af978 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:46:34 +0200 Subject: [PATCH 04/16] test: Cover gzip content encoding end to end Drives both directions over a real socket against the existing openapi.json fixture: `text-echo` echoes its body, so one call exercises request inflation and response coding together, and the spec resource route covers the streamed path. `java.net.http.HttpClient` neither sends `Accept-Encoding` nor decodes a coded response, so the tests set the header themselves and read bytes. That also makes `responseIsNotGzippedWithoutAcceptEncoding` the guard proving compression stays invisible to every other integration test. --- docs/plans/dynamic-discovering-piglet.md | 2 +- src/test/java/com/retailsvc/http/GzipIT.java | 307 +++++++++++++++++++ 2 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 src/test/java/com/retailsvc/http/GzipIT.java diff --git a/docs/plans/dynamic-discovering-piglet.md b/docs/plans/dynamic-discovering-piglet.md index c086ca5d..e8ce5bfa 100644 --- a/docs/plans/dynamic-discovering-piglet.md +++ b/docs/plans/dynamic-discovering-piglet.md @@ -284,7 +284,7 @@ as it completes. ### Task 5 — end to end -- [ ] **Step 10** `GzipIT` extending `ServerBaseTest`. Reuses the existing `/openapi.json` fixture +- [x] **Step 10** `GzipIT` extending `ServerBaseTest`. Reuses the existing `/openapi.json` fixture with runtime handler overrides — **no new spec files**. `text-echo` (`POST /text-echo`, `text/plain`, schema `{"type":"string"}`, no `maxLength`) echoes the body, so one call exercises both directions. Private `gzip(byte[])` / `gunzip(byte[])` helpers; requests built diff --git a/src/test/java/com/retailsvc/http/GzipIT.java b/src/test/java/com/retailsvc/http/GzipIT.java new file mode 100644 index 00000000..5301cea7 --- /dev/null +++ b/src/test/java/com/retailsvc/http/GzipIT.java @@ -0,0 +1,307 @@ +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; +import static java.net.HttpURLConnection.HTTP_OK; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import com.retailsvc.http.start.TextEchoHandler; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.util.Map; +import java.util.function.UnaryOperator; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; +import org.junit.jupiter.api.Test; + +/** End-to-end coverage of gzip request decoding and response coding over a real socket. */ +class GzipIT extends ServerBaseTest { + + private static final String TEXT_PLAIN = "text/plain"; + private static final String CONTENT_ENCODING = "Content-Encoding"; + private static final String ACCEPT_ENCODING = "Accept-Encoding"; + private static final String CONTENT_LENGTH = "Content-Length"; + + // -- request decoding -- + + @Test + void gzippedRequestBodyIsDecompressed() throws Exception { + try (var s = echoServer(); + var client = httpClient()) { + var request = + textEcho(s, gzip("hello gzip".getBytes(UTF_8))).header(CONTENT_ENCODING, "gzip").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.body()).isEqualTo("hello gzip"); + } + } + + @Test + void identityCodedRequestBodyIsUnaffected() throws Exception { + try (var s = echoServer(); + var client = httpClient()) { + var request = + textEcho(s, "plain".getBytes(UTF_8)).header(CONTENT_ENCODING, "identity").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.body()).isEqualTo("plain"); + } + } + + @Test + void handlerDoesNotSeeContentEncodingHeader() throws Exception { + RequestHandler reportsEncoding = + req -> Response.text(HTTP_OK, req.header(CONTENT_ENCODING).orElse("absent")); + try (var s = serverWith(Map.of("text-echo", reportsEncoding)); + var client = httpClient()) { + var request = + textEcho(s, gzip("body".getBytes(UTF_8))).header(CONTENT_ENCODING, "gzip").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.body()).isEqualTo("absent"); + } + } + + @Test + void unsupportedRequestEncodingReturns415() throws Exception { + try (var s = echoServer(); + var client = httpClient()) { + var request = textEcho(s, "body".getBytes(UTF_8)).header(CONTENT_ENCODING, "br").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_UNSUPPORTED_TYPE); + assertThat(response.headers().firstValue("Content-Type")) + .contains("application/problem+json"); + assertThat(response.body()).contains("Unsupported Media Type"); + } + } + + @Test + void malformedGzipRequestReturns400() throws Exception { + try (var s = echoServer(); + var client = httpClient()) { + var request = + textEcho(s, "not gzip at all".getBytes(UTF_8)).header(CONTENT_ENCODING, "gzip").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(response.body()).contains("malformed gzip request body"); + } + } + + @Test + void oversizedGzipRequestReturns413() throws Exception { + try (var s = echoServer(builder -> builder.maxDecompressedRequestBytes(1024)); + var client = httpClient()) { + var request = textEcho(s, gzip(new byte[8192])).header(CONTENT_ENCODING, "gzip").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_ENTITY_TOO_LARGE); + assertThat(response.body()).contains("Content Too Large"); + } + } + + // -- response coding -- + + @Test + void largeResponseIsGzippedWhenClientAcceptsGzip() throws Exception { + String payload = "compress me please ".repeat(200); + try (var s = echoServer(); + var client = httpClient()) { + var request = textEcho(s, payload.getBytes(UTF_8)).header(ACCEPT_ENCODING, "gzip").build(); + + HttpResponse response = client.send(request, BodyHandlers.ofByteArray()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).contains("gzip"); + assertThat(response.headers().firstValue("Vary")) + .hasValueSatisfying(vary -> assertThat(vary).contains(ACCEPT_ENCODING)); + assertThat(new String(gunzip(response.body()), UTF_8)).isEqualTo(payload); + assertThat(response.body().length).isLessThan(payload.length()); + } + } + + @Test + void responseIsNotGzippedWithoutAcceptEncoding() throws Exception { + String payload = "compress me please ".repeat(200); + try (var s = echoServer(); + var client = httpClient()) { + var request = textEcho(s, payload.getBytes(UTF_8)).build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).isEmpty(); + assertThat(response.body()).isEqualTo(payload); + } + } + + @Test + void smallResponseIsNotGzipped() throws Exception { + try (var s = echoServer(); + var client = httpClient()) { + var request = textEcho(s, "tiny".getBytes(UTF_8)).header(ACCEPT_ENCODING, "gzip").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).isEmpty(); + assertThat(response.body()).isEqualTo("tiny"); + } + } + + @Test + void gzipRefusedByQValueIsNotApplied() throws Exception { + String payload = "compress me please ".repeat(200); + try (var s = echoServer(); + var client = httpClient()) { + var request = + textEcho(s, payload.getBytes(UTF_8)).header(ACCEPT_ENCODING, "gzip;q=0").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).isEmpty(); + assertThat(response.body()).isEqualTo(payload); + } + } + + // -- streamed extra routes -- + + @Test + void streamedSpecResourceIsGzippedAndChunked() throws Exception { + try (var s = specServer(); + var client = httpClient()) { + var request = specRequest(s).header(ACCEPT_ENCODING, "gzip").GET().build(); + + HttpResponse response = client.send(request, BodyHandlers.ofByteArray()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.headers().firstValue(CONTENT_ENCODING)).contains("gzip"); + assertThat(response.headers().firstValue(CONTENT_LENGTH)).isEmpty(); + assertThat(gunzip(response.body())).isEqualTo(classpathBytes()); + } + } + + @Test + void streamedSpecResourceIsPlainWithoutAcceptEncoding() throws Exception { + try (var s = specServer(); + var client = httpClient()) { + var request = specRequest(s).GET().build(); + + HttpResponse response = client.send(request, BodyHandlers.ofByteArray()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).isEmpty(); + assertThat(response.body()).isEqualTo(classpathBytes()); + } + } + + @Test + void headOmitsContentLengthWhenGetWouldBeCompressed() throws Exception { + try (var s = specServer(); + var client = httpClient()) { + var request = + specRequest(s) + .header(ACCEPT_ENCODING, "gzip") + .method("HEAD", BodyPublishers.noBody()) + .build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.headers().firstValue(CONTENT_LENGTH)).isEmpty(); + assertThat(response.headers().firstValue("Content-Type")).contains("application/yaml"); + } + } + + @Test + void headKeepsContentLengthWithoutAcceptEncoding() throws Exception { + try (var s = specServer(); + var client = httpClient()) { + var request = specRequest(s).method("HEAD", BodyPublishers.noBody()).build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.headers().firstValue(CONTENT_LENGTH)) + .contains(String.valueOf(classpathBytes().length)); + } + } + + // -- fixtures -- + + private OpenApiServer echoServer() { + return echoServer(builder -> builder); + } + + private OpenApiServer echoServer(UnaryOperator customise) { + return serverWith(Map.of("text-echo", new TextEchoHandler()), customise); + } + + private OpenApiServer serverWith(Map handlers) { + return serverWith(handlers, builder -> builder); + } + + private OpenApiServer serverWith( + Map handlers, UnaryOperator customise) { + try { + server = + customise + .apply(newBuilder().spec(spec).handlers(stubAllHandlers(handlers)).port(0)) + .build(); + return server; + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private OpenApiServer specServer() { + return serverWith( + Map.of(), + builder -> builder.extraRoute("/openapi.yaml", Handlers.resourceHandler("/openapi.yaml"))); + } + + private HttpRequest.Builder textEcho(OpenApiServer server, byte[] body) { + return HttpRequest.newBuilder() + .uri(URI.create("http://localhost:%d/api/v1/text-echo".formatted(server.listenPort()))) + .header("Content-Type", TEXT_PLAIN) + .POST(BodyPublishers.ofByteArray(body)); + } + + private HttpRequest.Builder specRequest(OpenApiServer server) { + return HttpRequest.newBuilder() + .uri(URI.create("http://localhost:%d/openapi.yaml".formatted(server.listenPort()))); + } + + private static byte[] classpathBytes() throws IOException { + try (InputStream in = GzipIT.class.getResourceAsStream("/openapi.yaml")) { + return in.readAllBytes(); + } + } + + private static byte[] gzip(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { + gzip.write(data); + } + return out.toByteArray(); + } + + private static byte[] gunzip(byte[] data) throws IOException { + try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) { + return in.readAllBytes(); + } + } +} From 5cbb498420d683ebf9788347801951065ec3ea6d Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:48:16 +0200 Subject: [PATCH 05/16] docs: Document gzip content encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a "Content encoding" section under Server configuration covering both directions, the two limits and their defaults, the media types that qualify, and the deliberate non-goals. Adds Caveats entries for the two consequences a handler author can be surprised by: a strong ETag now spans two byte streams, and throwing mid-stream produces a valid gzip trailer over a short body rather than a framing error. Also corrects the architecture notes in CLAUDE.md, which still described a filter chain that no longer exists — ExceptionFilter on the spec context, the request body stashed as an exchange attribute, and a static `Request.bytes(exchange)` helper — and mentioned neither SecurityFilter nor ExtrasRouter. --- CLAUDE.md | 19 ++++--- README.md | 70 ++++++++++++++++++++++++ docs/plans/dynamic-discovering-piglet.md | 4 +- 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 70b0896e..5182ad6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,20 +26,23 @@ Java 25 is required (see `.java-version`). The server uses thread-per-request wi Request flow when `OpenApiServer` boots (`src/main/java/com/retailsvc/http/OpenApiServer.java`): 1. `HttpServer` is created on a port with a virtual-thread-per-task executor. -2. A single `HttpContext` is registered at `spec.basePath()` (the first `servers[].url` path from the OpenAPI doc). A catch-all `/` context returns 404. -3. Three filters run in order on every request: - - `ExceptionFilter` — wraps the chain; delegates uncaught exceptions to the user-supplied `ExceptionHandler` (default in `Handlers`). - - `RequestPreparationFilter` — reads the raw request body, stashes it as an exchange attribute, runs OpenAPI parameter + body validation via `DefaultValidator`, and stores the resolved `operationId` on the exchange. - - `DispatchHandler` — looks up the `HttpHandler` registered for that `operationId` in the user-supplied map and invokes it. Handler coverage is verified at boot, so the lookup never returns `null`. +2. One `HttpContext` is registered per spec binding at `spec.basePath()` (the first `servers[].url` path from the OpenAPI doc). Unless a binding owns `/`, a catch-all `/` context serves extra routes via `ExtrasRouter` and 404s everything else; `ExceptionFilter` wraps that context only. +3. On a binding context, two filters run in order, then the handler: + - `RequestPreparationFilter` — reads the request body through `RequestBodyReader` (which inflates a gzip `Content-Encoding` under a size cap), resolves the route, runs OpenAPI parameter + body validation via `DefaultValidator`, and binds the resulting `Request` into the `DispatchHandler.CURRENT` scoped value. It renders its own failures through the `ExceptionHandler` rather than relying on `ExceptionFilter`. + - `SecurityFilter` — enforces the spec's `securitySchemes` / `security`, re-binding the `Request` with resolved principals. It writes its 401/403 responses straight to the exchange. + - `DispatchHandler` — looks up the `RequestHandler` registered for the resolved `operationId` in the user-supplied map and invokes it, applying interceptors and response decorators. Handler coverage is verified at boot, so the lookup never returns `null`. + +Every response except `SecurityFilter`'s rejections is written by `ResponseRenderer`, which is also where response gzip coding is applied. Key abstractions: - `com.retailsvc.http.spec.Spec` — parsed from a consumer-supplied `Map` via `Spec.from(raw)`. No JSON library dependency in the library itself; callers use Gson, Jackson, SnakeYAML, etc. to produce the map. - Sealed `com.retailsvc.http.spec.schema.Schema` interface with per-kind records (`StringSchema`, `NumberSchema`, `IntegerSchema`, `ArraySchema`, `ObjectSchema`, `BooleanSchema`, `NullSchema`, `AnyOfSchema`, `AllOfSchema`, `OneOfSchema`). Pattern-match dispatch eliminates instanceof chains. -- `com.retailsvc.http.validate.DefaultValidator` — single class using `switch` pattern-match over `Schema` subtypes. Validation failures produce RFC 7807 `application/problem+json` 400 responses. +- `com.retailsvc.http.validate.DefaultValidator` — single class using `switch` pattern-match over `Schema` subtypes. Validation failures produce RFC 9457 `application/problem+json` 400 responses. - `com.retailsvc.http.internal.Router` — two indexes: exact path map and templated path list. Resolves `operationId` + extracted path variables for each request. -- `JsonMapper` — `@FunctionalInterface`; single method `Object mapFrom(byte[])`. Callers supply a lambda (see README). -- `com.retailsvc.http.Request` — static helper; `Request.bytes(exchange)` returns raw body bytes, `Request.parsed(exchange)` returns the `Object` produced by the `JsonMapper`. +- `TypeMapper` — per-media-type request parsing and response writing; registered via `Builder.bodyMapper(...)`, with `GsonTypeMapper` auto-registered when Gson is on the classpath. +- `com.retailsvc.http.Request` — an immutable record-like carrier built from primitives (body bytes, path parameters, raw query string, a header lookup function), never the `HttpExchange`. `bytes()` returns the decoded body, `parsed()` the object produced by the `TypeMapper`. +- `com.retailsvc.http.internal.RequestBodyReader` / `ResponseCompression` — inbound and outbound gzip. See the README's "Content encoding" section for the policy. ## Conventions diff --git a/README.md b/README.md index ff36077a..73174a17 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ endpoints declared in an OpenAPI 3.1.x specification. Handlers are pure function - [Body parsers and response writers](#body-parsers-and-response-writers) - [Server configuration](#server-configuration) - [HTTPS](#https) + - [Content encoding](#content-encoding) - [Interceptors and response decorators](#interceptors-and-response-decorators) - [After-response hooks](#after-response-hooks) - [Security](#security) @@ -48,6 +49,8 @@ endpoints declared in an OpenAPI 3.1.x specification. Handlers are pure function - OpenAPI `securitySchemes` and `security` enforcement (`apiKey`, `http bearer`, `http basic`), with an opt-out for sidecar / gateway authentication - RFC 9457 `application/problem+json` validation errors with an `errors[]` array of JSON-Pointers to the failing locations +- Transparent gzip: request bodies are inflated under a zip-bomb ceiling, responses are compressed + when the client accepts it and the payload is worth it - Built on the JDK's native `HttpServer` with thread-per-request behaviour using virtual threads ## Maven artifact @@ -459,6 +462,67 @@ explicitly — it isn't signed by a public CA. - TLS protocol / cipher overrides (JDK defaults apply: TLS 1.2 and 1.3) - Serving HTTP and HTTPS from one `OpenApiServer` instance +### Content encoding + +gzip is handled in both directions, with no configuration required. + +**Requests.** A body sent with `Content-Encoding: gzip` is inflated before OpenAPI validation runs, +so the validator, your `TypeMapper` and your handler all see plain bytes. `identity` is accepted as +the no-op it is. Any other coding — `br`, `deflate`, or two codings stacked — is rejected with +`415 Unsupported Media Type`, and a corrupt or truncated gzip stream with `400 Bad Request`. + +Once a body is inflated it no longer matches the headers that described it, so `Content-Encoding` is +hidden from `Request.header(...)` and `Content-Length` reports the inflated size. + +Inflation runs under a ceiling, because a few compressed kilobytes can expand into gigabytes: + +```java +OpenApiServer.builder() + .spec(spec) + .handlers(handlers) + .maxDecompressedRequestBytes(32 * 1024 * 1024) // default 10 MiB; over it, 413 + .build(); +``` + +Note this bounds the *inflated* size of a gzip body. It is not a request size limit — a body that +arrives uncompressed is read in full, as it always has been. + +**Responses.** A body is gzipped when the client sends `Accept-Encoding: gzip`, the media type is +text-shaped (`text/*`, `application/json`, `application/xml`, `application/yaml`, and the `+json` / +`+xml` / `+yaml` structured suffixes), and it is at least 1 KiB. Below that the coding costs more +than it saves; `application/octet-stream`, images and other already-compressed media are never +coded, and neither is `text/event-stream`, which has to stay unbuffered. + +```java +OpenApiServer.builder() + .spec(spec) + .handlers(handlers) + .minimumGzipResponseBytes(4096) // default 1024; 0 compresses everything compressible + .build(); +``` + +There is no on/off flag. If a proxy in front of you already terminates compression, set the +threshold above anything this server returns. + +`Vary: Accept-Encoding` is set whenever a body *could* have been coded, not only when it was, so +shared caches keep the two forms apart. It is merged into any `Vary` your handler already set. +A handler that sets its own `Content-Encoding` is left alone, and so is a payload gzip fails to +shrink. Statuses that carry no content never get a coding. + +Streamed responses (`Response.stream(...)`) are deflated as they are written. A length declared by +the sized overload describes the uncoded body, so a coded stream goes out chunked; a stream of +unknown length is coded regardless of the threshold, since measuring it would defeat streaming it. +For the same reason a `HEAD` whose `GET` would be compressed omits `Content-Length` rather than +advertising the uncoded length. + +**Not in this release** (each can land later without breaking the API): + +- brotli, zstd and `deflate`, in either direction +- the `Accept-Encoding` response header RFC 9110 recommends alongside a 415 +- compression of the `401`/`403` bodies produced by security scheme enforcement — those bypass the + renderer and are well under any sensible threshold +- per-route or per-operation opt-out + ### Graceful shutdown `OpenApiServer` exposes `stop(int delaySeconds)` for explicit shutdown that waits up to the given @@ -1218,6 +1282,12 @@ A few things worth keeping in mind when reading this: JDK `HttpExchange`. A future enhancement could plug in a higher-throughput backend (Jetty, Helidon Níma, Netty) by writing a new adapter behind `com.retailsvc.http.internal` while leaving handlers untouched. +- **gzip changes what an `ETag` identifies.** The library sets none, but a handler that sets a + strong `ETag` would use one entity tag for both the coded and uncoded forms of a body. Use a weak + tag (`W/"..."`), or set `Content-Encoding` yourself to opt that response out of compression. +- **A handler that throws mid-stream yields a valid gzip trailer.** Closing the coded stream + finishes the gzip member, so a client sees a complete-looking short body rather than the framing + error a truncated chunked response would have produced. - **Per-request state uses `ScopedValue`** (Java 25, JEP 506). This matters if a handler offloads work to an executor that's not a `StructuredTaskScope`-managed child thread: the `ScopedValue` is not visible there, so the handler must capture the values it needs (e.g. diff --git a/docs/plans/dynamic-discovering-piglet.md b/docs/plans/dynamic-discovering-piglet.md index e8ce5bfa..6179dc9f 100644 --- a/docs/plans/dynamic-discovering-piglet.md +++ b/docs/plans/dynamic-discovering-piglet.md @@ -304,7 +304,7 @@ as it completes. ### Task 6 — docs -- [ ] **Step 11** README: `### Request decompression` and `### Response compression` under +- [x] **Step 11** README: `### Request decompression` and `### Response compression` under `## Server configuration`, a TOC entry, a `## Highlights` bullet, and a **"Not in this release"** list matching the HTTPS section's convention — brotli/deflate/zstd, `Accept-Encoding` on 415 responses (RFC 9110 §15.5.16 SHOULD; `BadRequestException` carries no @@ -312,7 +312,7 @@ as it completes. bullets: the cap bounds *inflated* bytes only and is not a request size limit; a strong `ETag` set by a handler now spans two byte streams; a handler that throws mid-stream yields a valid gzip trailer over truncated content rather than a framing error. -- [ ] **Step 12** Correct the stale request-flow description in `CLAUDE.md` — it describes three +- [x] **Step 12** Correct the stale request-flow description in `CLAUDE.md` — it describes three filters including `ExceptionFilter` on the spec context, exchange-attribute body stashing and a `Request.bytes(exchange)` static helper, none of which match the current code, and it mentions neither `SecurityFilter` nor `ExtrasRouter`. From 2a454e0e254a1eae87c6675ac4d38d98066e53ed Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 14:53:41 +0200 Subject: [PATCH 06/16] test: Cover the remaining content-coding branches Fills the gaps JaCoCo flagged: repeated codings in one Accept-Encoding header, weight parameters mixed with others, a valueless parameter, 304 responses, an unparsable hand-set Content-Length, and streams whose handler supplied its own Content-Type or Content-Encoding. Branch coverage on AcceptEncodingHeader goes 70% to 93% and on ResponseRenderer 83% to 87%, keeping the new code clear of the Sonar new-code gate. --- .../internal/AcceptEncodingHeaderTest.java | 22 ++++++++ .../http/internal/ResponseRendererTest.java | 51 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java index 4574674c..f336d95d 100644 --- a/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java +++ b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java @@ -91,4 +91,26 @@ void malformedQValueIsTreatedAsAccepted() { void emptyTokensAreIgnored() { assertThat(AcceptEncodingHeader.acceptsGzip("deflate,,gzip")).isTrue(); } + + @Test + void repeatedGzipTokensTakeThePositiveWeight() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0, gzip")).isTrue(); + assertThat(AcceptEncodingHeader.acceptsGzip("gzip, x-gzip;q=0")).isTrue(); + } + + @Test + void repeatedWildcardsTakeThePositiveWeight() { + assertThat(AcceptEncodingHeader.acceptsGzip("*;q=0, *")).isTrue(); + } + + @Test + void parametersOtherThanWeightAreIgnored() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;level=9")).isTrue(); + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;level=9;q=0")).isFalse(); + } + + @Test + void valuelessParameterIsIgnored() { + assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q")).isTrue(); + } } diff --git a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java index 2ead5e06..1377b8f0 100644 --- a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java +++ b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static java.net.HttpURLConnection.HTTP_NOT_MODIFIED; import static java.net.HttpURLConnection.HTTP_NO_CONTENT; import static java.net.HttpURLConnection.HTTP_OK; import static java.nio.charset.StandardCharsets.UTF_8; @@ -346,6 +347,56 @@ void keepsContentLengthOnNullBodyBelowThreshold() throws IOException { assertThat(responseHeaders.getFirst("Content-Length")).isEqualTo("12"); } + @Test + void neverCompressesNotModifiedResponses() throws IOException { + acceptsGzip(); + + renderer.render(exchange, Response.bytes(HTTP_NOT_MODIFIED, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + } + + @Test + void keepsContentLengthOnNullBodyWhenUnparsable() throws IOException { + acceptsGzip(); + + renderer.render( + exchange, + Response.status(HTTP_OK) + .withContentType(TEXT) + .withHeader("Content-Length", "not-a-number")); + + assertThat(responseHeaders.getFirst("Content-Length")).isEqualTo("not-a-number"); + } + + @Test + void keepsHandlerSuppliedContentTypeOnStreams() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render( + exchange, + Response.stream(HTTP_OK, TEXT, out -> out.write(payload)) + .withHeader("Content-Type", "text/csv")); + + assertThat(responseHeaders.get("Content-Type")).containsExactly("text/csv"); + assertThat(gunzip(sink.toByteArray())).isEqualTo(payload); + } + + @Test + void skipsCompressionOnStreamWhenHandlerAlreadySetContentEncoding() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render( + exchange, + Response.stream(HTTP_OK, TEXT, out -> out.write(payload)) + .withHeader(CONTENT_ENCODING, "br")); + + assertThat(responseHeaders.get(CONTENT_ENCODING)).containsExactly("br"); + assertThat(sink.toByteArray()).isEqualTo(payload); + } + private void acceptsGzip() { requestHeaders.add("Accept-Encoding", "gzip, deflate, br"); } From bbcde958e8d6101188d6f71d66c750e8ac43b517 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:02:43 +0200 Subject: [PATCH 07/16] docs: Clarify that the gzip snippets show overrides, not defaults Both builder examples put the default in a comment beside a different literal, so the comment read as if it were annotating the value in the call. Say plainly that the call raises the default. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 73174a17..b6838a95 100644 --- a/README.md +++ b/README.md @@ -480,7 +480,7 @@ Inflation runs under a ceiling, because a few compressed kilobytes can expand in OpenApiServer.builder() .spec(spec) .handlers(handlers) - .maxDecompressedRequestBytes(32 * 1024 * 1024) // default 10 MiB; over it, 413 + .maxDecompressedRequestBytes(32 * 1024 * 1024) // raises the 10 MiB default; over it, 413 .build(); ``` @@ -497,7 +497,7 @@ coded, and neither is `text/event-stream`, which has to stay unbuffered. OpenApiServer.builder() .spec(spec) .handlers(handlers) - .minimumGzipResponseBytes(4096) // default 1024; 0 compresses everything compressible + .minimumGzipResponseBytes(4096) // raises the 1 KiB default; 0 compresses every eligible body .build(); ``` From 47450faf42d184d44817127ad06de66c1678f8ab Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:14:42 +0200 Subject: [PATCH 08/16] refactor: Collapse the duplicated gzip decision in the renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three render paths repeated the same guard — already coded, not compressible, or a status that carries no content — then announced Vary, then checked threshold and Accept-Encoding. That is now one shouldCompress method the three paths share, with a negative length meaning "unknown, treat as over the threshold" so streams keep their semantics. Also drops the single-argument ResponseRenderer constructor, which no longer had a caller in main, and trims incidental weight in the header parsers: the boxed tri-state in AcceptEncodingHeader becomes two plain booleans, the compressible-suffix Set becomes three endsWith calls, and single-use string constants are inlined to match ContentTypeHeader. 530 to 498 lines across the five files; behaviour unchanged. --- .../http/internal/AcceptEncodingHeader.java | 25 +++++------ .../http/internal/ContentEncodingHeader.java | 16 +++---- .../http/internal/ResponseCompression.java | 8 +--- .../http/internal/ResponseRenderer.java | 45 ++++++------------- .../http/internal/DispatchHandlerTest.java | 10 ++++- .../http/internal/ExtrasRouterTest.java | 3 +- .../RequestPreparationFilterTest.java | 3 +- 7 files changed, 43 insertions(+), 67 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java index faec2167..9ee2a7f1 100644 --- a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java +++ b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java @@ -5,10 +5,6 @@ /** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */ public final class AcceptEncodingHeader { - private static final String GZIP = "gzip"; - private static final String X_GZIP = "x-gzip"; - private static final String WILDCARD = "*"; - private static final String QUALITY = "q"; private static final double DEFAULT_QUALITY = 1.0; private AcceptEncodingHeader() {} @@ -22,8 +18,9 @@ public static boolean acceptsGzip(String header) { if (header == null) { return false; } - Boolean gzipAccepted = null; - Boolean wildcardAccepted = null; + boolean gzipSeen = false; + boolean gzipAccepted = false; + boolean wildcardAccepted = false; for (String token : header.split(",")) { String trimmed = token.trim(); if (trimmed.isEmpty()) { @@ -33,16 +30,14 @@ public static boolean acceptsGzip(String header) { String coding = (semi < 0 ? trimmed : trimmed.substring(0, semi)).trim().toLowerCase(Locale.ROOT); boolean accepted = quality(semi < 0 ? null : trimmed.substring(semi + 1)) > 0; - if (GZIP.equals(coding) || X_GZIP.equals(coding)) { - gzipAccepted = gzipAccepted == null ? accepted : gzipAccepted || accepted; - } else if (WILDCARD.equals(coding)) { - wildcardAccepted = wildcardAccepted == null ? accepted : wildcardAccepted || accepted; + if ("gzip".equals(coding) || "x-gzip".equals(coding)) { + gzipSeen = true; + gzipAccepted |= accepted; + } else if ("*".equals(coding)) { + wildcardAccepted |= accepted; } } - if (gzipAccepted != null) { - return gzipAccepted; - } - return wildcardAccepted != null && wildcardAccepted; + return gzipSeen ? gzipAccepted : wildcardAccepted; } /** @@ -60,7 +55,7 @@ private static double quality(String parameters) { continue; } String name = trimmed.substring(0, equals).trim().toLowerCase(Locale.ROOT); - if (QUALITY.equals(name)) { + if ("q".equals(name)) { try { return Double.parseDouble(trimmed.substring(equals + 1).trim()); } catch (NumberFormatException e) { diff --git a/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java index 4c9e2cd3..8627a85b 100644 --- a/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java +++ b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java @@ -5,19 +5,15 @@ /** Classifies a request {@code Content-Encoding} into the codings the server can decode. */ public final class ContentEncodingHeader { - private static final String IDENTITY_CODING = "identity"; - private static final String GZIP_CODING = "gzip"; - private static final String X_GZIP_CODING = "x-gzip"; - private ContentEncodingHeader() {} - /** The content coding applied to a request body. */ + /** + * The coding applied to a request body: none (absent or the {@code identity} no-op), a single + * gzip, or one this server cannot decode and the caller renders 415 for. + */ public enum Coding { - /** No coding, or the explicit {@code identity} no-op. */ NONE, - /** A single gzip coding. */ GZIP, - /** A coding this server cannot decode; the caller renders 415. */ UNSUPPORTED } @@ -33,13 +29,13 @@ public static Coding parse(String header) { Coding result = Coding.NONE; for (String token : header.split(",")) { String coding = token.trim().toLowerCase(Locale.ROOT); - if (coding.isEmpty() || IDENTITY_CODING.equals(coding)) { + if (coding.isEmpty() || "identity".equals(coding)) { continue; } if (result != Coding.NONE) { return Coding.UNSUPPORTED; } - if (GZIP_CODING.equals(coding) || X_GZIP_CODING.equals(coding)) { + if ("gzip".equals(coding) || "x-gzip".equals(coding)) { result = Coding.GZIP; } else { return Coding.UNSUPPORTED; diff --git a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java index 1970148e..df9c6717 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java @@ -21,8 +21,6 @@ public final class ResponseCompression { "application/javascript", "application/x-ndjson"); - private static final Set COMPRESSIBLE_SUFFIXES = Set.of("+json", "+xml", "+yaml"); - private ResponseCompression() {} /** @@ -41,10 +39,8 @@ public static boolean isCompressible(String contentType) { if (mediaType.startsWith(TEXT_PREFIX)) { return !EVENT_STREAM.equals(mediaType); } - for (String suffix : COMPRESSIBLE_SUFFIXES) { - if (mediaType.endsWith(suffix)) { - return true; - } + if (mediaType.endsWith("+json") || mediaType.endsWith("+xml") || mediaType.endsWith("+yaml")) { + return true; } return COMPRESSIBLE_TYPES.contains(mediaType); } diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 580d39b2..0625fbb7 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -35,10 +35,6 @@ public final class ResponseRenderer { private final Map mappers; private final long minimumGzipBytes; - public ResponseRenderer(Map mappers) { - this(mappers, DEFAULT_MINIMUM_GZIP_BYTES); - } - public ResponseRenderer(Map mappers, long minimumGzipBytes) { this.mappers = Map.copyOf(mappers); this.minimumGzipBytes = minimumGzipBytes; @@ -72,15 +68,11 @@ private void renderEmpty(HttpExchange exchange, Headers headers, int status, Str if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { headers.add(CONTENT_TYPE, contentType); } - if (!headers.containsKey(CONTENT_ENCODING) - && ResponseCompression.isCompressible(contentType) - && bodyAllowed(status)) { - addVary(headers); - if (declaredLength(headers) >= minimumGzipBytes && acceptsGzip(exchange)) { - headers.remove(CONTENT_LENGTH); - } + long declared = declaredLength(headers); + if (shouldCompress(exchange, headers, status, contentType, declared) && declared >= 0) { + headers.remove(CONTENT_LENGTH); } - exchange.sendResponseHeaders(status, -1); + exchange.sendResponseHeaders(status, UNKNOWN_LENGTH); } private void renderStream( @@ -90,7 +82,7 @@ private void renderStream( headers.add(CONTENT_TYPE, contentType); } long declared = writer instanceof BodyWriter.Sized sized ? sized.length() : UNKNOWN_LENGTH; - if (compressStream(exchange, headers, status, contentType, declared)) { + if (shouldCompress(exchange, headers, status, contentType, declared)) { headers.set(CONTENT_ENCODING, GZIP); exchange.sendResponseHeaders(status, CHUNKED); try (OutputStream out = ResponseCompression.gzipStream(exchange.getResponseBody())) { @@ -105,20 +97,19 @@ private void renderStream( } /** - * A coded stream has to go out chunked, because the length a {@code Sized} body declares measures - * the uncoded form. A body of unknown length is compressed regardless of the threshold — - * buffering it to find out how big it is would defeat streaming it. + * Whether a body of {@code length} bytes should be gzipped, marking the response as varying by + * {@code Accept-Encoding} whenever it could have been. A negative length means unknown, which + * counts as over the threshold: measuring a stream to find out would defeat streaming it. */ - private boolean compressStream( - HttpExchange exchange, Headers headers, int status, String contentType, long declaredLength) { + private boolean shouldCompress( + HttpExchange exchange, Headers headers, int status, String contentType, long length) { if (headers.containsKey(CONTENT_ENCODING) || !ResponseCompression.isCompressible(contentType) || !bodyAllowed(status)) { return false; } addVary(headers); - boolean worthCoding = declaredLength < 0 || declaredLength >= minimumGzipBytes; - return worthCoding && acceptsGzip(exchange); + return (length < 0 || length >= minimumGzipBytes) && acceptsGzip(exchange); } /** The length a handler declared for a body it did not write, or -1 when absent or unreadable. */ @@ -158,21 +149,11 @@ private void renderBytes( } } - /** - * Gzips the body when the client asked for it and the payload is big enough to be worth it. A - * handler that coded the body itself is left alone, and so is a payload that gzip fails to - * shrink. - */ + /** Gzips the body when it is worth it, leaving a payload gzip fails to shrink uncoded. */ private byte[] maybeCompress( HttpExchange exchange, Headers headers, int status, String contentType, byte[] bytes) throws IOException { - if (headers.containsKey(CONTENT_ENCODING) - || !ResponseCompression.isCompressible(contentType) - || !bodyAllowed(status)) { - return bytes; - } - addVary(headers); - if (bytes.length < minimumGzipBytes || !acceptsGzip(exchange)) { + if (!shouldCompress(exchange, headers, status, contentType, bytes.length)) { return bytes; } byte[] gzipped = ResponseCompression.gzip(bytes); diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index d526f98e..32846bc4 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static java.net.HttpURLConnection.HTTP_OK; import static org.assertj.core.api.Assertions.assertThat; @@ -44,14 +45,19 @@ private static HttpExchange stubExchange() { } private static DispatchHandler dispatcher(Map handlers) { - return new DispatchHandler(handlers, List.of(), List.of(), new ResponseRenderer(Map.of())); + return new DispatchHandler( + handlers, List.of(), List.of(), new ResponseRenderer(Map.of(), DEFAULT_MINIMUM_GZIP_BYTES)); } private static DispatchHandler dispatcher( Map handlers, List interceptors, List decorators) { - return new DispatchHandler(handlers, interceptors, decorators, new ResponseRenderer(Map.of())); + return new DispatchHandler( + handlers, + interceptors, + decorators, + new ResponseRenderer(Map.of(), DEFAULT_MINIMUM_GZIP_BYTES)); } private static void withRequest(String operationId, ScopedValue.CallableOp body) diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java index 0475d66a..314764b4 100644 --- a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; @@ -145,7 +146,7 @@ private static ExtrasRouter newRouter(Map extras) { Map mappers = Map.of("application/json", new GsonTypeMapper()); return new ExtrasRouter( extras, - new ResponseRenderer(mappers), + new ResponseRenderer(mappers, DEFAULT_MINIMUM_GZIP_BYTES), new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index 65b3fb54..7608d04f 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -114,7 +115,7 @@ public byte[] writeTo(Object value) { new DefaultValidator(spec::resolveSchema), mappers, rethrow, - new ResponseRenderer(mappers), + new ResponseRenderer(mappers, DEFAULT_MINIMUM_GZIP_BYTES), List.of(), new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } From c3ff28044461975388367f844924120d22b83d16 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:19:24 +0200 Subject: [PATCH 09/16] refactor: Carry the gzip collaborators instead of plumbing their sizes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HandlerConfig now holds the RequestBodyReader and the ResponseRenderer themselves rather than the two longs they are built from, and Builder constructs both. Since HandlerConfig already reached every wiring method, the extra parameters threaded through wireBindings, wireBinding and wireExtras all go away, and with them both java:S107 suppressions — the parameter counts were the signal that the numbers were at the wrong altitude. Also reuses ContentTypeHeader.parameter for the Accept-Encoding q weight instead of hand-rolling a second parameter parser, replaces the capped inflate loop with a single bounded readNBytes, and drops the gzipStream alias for the JDK constructor it wrapped. The five gzip files go 530 to 470 lines and OpenApiServer sheds 36. --- .../com/retailsvc/http/OpenApiServer.java | 76 +++++-------------- .../http/internal/AcceptEncodingHeader.java | 43 ++++------- .../http/internal/RequestBodyReader.java | 22 ++---- .../http/internal/ResponseCompression.java | 9 --- .../http/internal/ResponseRenderer.java | 35 +++++---- .../internal/ResponseCompressionTest.java | 14 ---- 6 files changed, 58 insertions(+), 141 deletions(-) diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index 23ee7187..6e1872dd 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -66,8 +66,8 @@ record HandlerConfig( Map extras, boolean externalAuth, List afterHooks, - long maxDecompressedRequestBytes, - long minimumGzipResponseBytes) {} + RequestBodyReader bodyReader, + ResponseRenderer renderer) {} OpenApiServer( List bindings, @@ -86,7 +86,6 @@ record HandlerConfig( requireNonNull(bodyMappers, "bodyMappers must not be null"); long t0 = System.currentTimeMillis(); - ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler(); InetSocketAddress socketAddress = (bindAddress == null) @@ -95,26 +94,8 @@ record HandlerConfig( this.httpServer = createHttpServer(socketAddress, sslContext); httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); - ResponseRenderer renderer = - new ResponseRenderer(bodyMappers, handlerConfig.minimumGzipResponseBytes()); - RequestBodyReader bodyReader = - new RequestBodyReader(handlerConfig.maxDecompressedRequestBytes()); - boolean anyBindingAtRoot = - wireBindings( - httpServer, - bindings, - bodyMappers, - handlerConfig, - exceptionHandler, - renderer, - bodyReader); - wireExtras( - httpServer, - anyBindingAtRoot, - handlerConfig.extras(), - exceptionHandler, - renderer, - bodyReader); + boolean anyBindingAtRoot = wireBindings(httpServer, bindings, bodyMappers, handlerConfig); + wireExtras(httpServer, anyBindingAtRoot, handlerConfig); httpServer.start(); this.shutdownTimeoutSeconds = shutdownTimeoutSeconds; @@ -131,42 +112,26 @@ private static HttpServer createHttpServer(InetSocketAddress addr, SSLContext ss return HttpServer.create(addr, 0); } - @SuppressWarnings("java:S107") private static boolean wireBindings( HttpServer httpServer, List bindings, Map bodyMappers, - HandlerConfig handlerConfig, - ExceptionHandler exceptionHandler, - ResponseRenderer renderer, - RequestBodyReader bodyReader) { + HandlerConfig handlerConfig) { boolean anyBindingAtRoot = false; for (SpecBinding binding : bindings) { String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/"); anyBindingAtRoot |= "/".equals(basePath); - wireBinding( - httpServer, - basePath, - binding, - bodyMappers, - handlerConfig, - exceptionHandler, - renderer, - bodyReader); + wireBinding(httpServer, basePath, binding, bodyMappers, handlerConfig); } return anyBindingAtRoot; } - @SuppressWarnings("java:S107") private static void wireBinding( HttpServer httpServer, String basePath, SpecBinding binding, Map bodyMappers, - HandlerConfig handlerConfig, - ExceptionHandler exceptionHandler, - ResponseRenderer renderer, - RequestBodyReader bodyReader) { + HandlerConfig handlerConfig) { Map operationsById = binding.spec().operations().stream() .collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op)); @@ -178,10 +143,10 @@ private static void wireBinding( binding.router(), binding.validator(), bodyMappers, - exceptionHandler, - renderer, + handlerConfig.exceptionHandler(), + handlerConfig.renderer(), handlerConfig.afterHooks(), - bodyReader)); + handlerConfig.bodyReader())); ctx.getFilters() .add( new SecurityFilter( @@ -195,16 +160,12 @@ private static void wireBinding( binding.handlers(), handlerConfig.interceptors(), handlerConfig.decorators(), - renderer)); + handlerConfig.renderer())); } private static void wireExtras( - HttpServer httpServer, - boolean anyBindingAtRoot, - Map extras, - ExceptionHandler exceptionHandler, - ResponseRenderer renderer, - RequestBodyReader bodyReader) { + HttpServer httpServer, boolean anyBindingAtRoot, HandlerConfig handlerConfig) { + Map extras = handlerConfig.extras(); if (anyBindingAtRoot) { if (!extras.isEmpty()) { throw new IllegalStateException( @@ -212,9 +173,12 @@ private static void wireExtras( } return; } - ExtrasRouter extrasRouter = new ExtrasRouter(extras, renderer, bodyReader); + ExtrasRouter extrasRouter = + new ExtrasRouter(extras, handlerConfig.renderer(), handlerConfig.bodyReader()); HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); - extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + extrasCtx + .getFilters() + .add(new ExceptionFilter(handlerConfig.exceptionHandler(), handlerConfig.renderer())); } private void logStartup(long t0) { @@ -510,8 +474,8 @@ public OpenApiServer build() throws IOException { extras, externalAuth, List.copyOf(afterHooks), - maxDecompressedRequestBytes, - minimumGzipResponseBytes); + new RequestBodyReader(maxDecompressedRequestBytes), + new ResponseRenderer(resolved, minimumGzipResponseBytes)); int resolvedPort = resolvePort(); SSLContext sslContext = httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; diff --git a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java index 9ee2a7f1..a83a017a 100644 --- a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java +++ b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java @@ -5,8 +5,6 @@ /** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */ public final class AcceptEncodingHeader { - private static final double DEFAULT_QUALITY = 1.0; - private AcceptEncodingHeader() {} /** @@ -22,14 +20,9 @@ public static boolean acceptsGzip(String header) { boolean gzipAccepted = false; boolean wildcardAccepted = false; for (String token : header.split(",")) { - String trimmed = token.trim(); - if (trimmed.isEmpty()) { - continue; - } - int semi = trimmed.indexOf(';'); - String coding = - (semi < 0 ? trimmed : trimmed.substring(0, semi)).trim().toLowerCase(Locale.ROOT); - boolean accepted = quality(semi < 0 ? null : trimmed.substring(semi + 1)) > 0; + int semi = token.indexOf(';'); + String coding = (semi < 0 ? token : token.substring(0, semi)).trim().toLowerCase(Locale.ROOT); + boolean accepted = positiveWeight(token); if ("gzip".equals(coding) || "x-gzip".equals(coding)) { gzipSeen = true; gzipAccepted |= accepted; @@ -41,28 +34,18 @@ public static boolean acceptsGzip(String header) { } /** - * Reads the {@code q} weight from a token's parameter list. An absent or unparsable weight is - * read as the default 1.0 — a malformed header should not silently disable compression. + * Whether the token's {@code q} weight admits the coding. An absent or unparsable weight reads as + * the default 1.0 — a malformed header should not silently disable compression. */ - private static double quality(String parameters) { - if (parameters == null) { - return DEFAULT_QUALITY; + private static boolean positiveWeight(String token) { + String weight = ContentTypeHeader.parameter(token, "q").orElse(null); + if (weight == null) { + return true; } - for (String parameter : parameters.split(";")) { - String trimmed = parameter.trim(); - int equals = trimmed.indexOf('='); - if (equals <= 0) { - continue; - } - String name = trimmed.substring(0, equals).trim().toLowerCase(Locale.ROOT); - if ("q".equals(name)) { - try { - return Double.parseDouble(trimmed.substring(equals + 1).trim()); - } catch (NumberFormatException e) { - return DEFAULT_QUALITY; - } - } + try { + return Double.parseDouble(weight) > 0; + } catch (NumberFormatException _) { + return true; } - return DEFAULT_QUALITY; } } diff --git a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java index 3526d1ba..170cff06 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java +++ b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java @@ -8,7 +8,6 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.EOFException; import java.io.IOException; import java.util.function.UnaryOperator; @@ -29,6 +28,7 @@ public final class RequestBodyReader { private static final int BUFFER_SIZE = 8192; private final long maxDecompressedBytes; + private final int readLimit; public RequestBodyReader(long maxDecompressedBytes) { if (maxDecompressedBytes <= 0) { @@ -36,6 +36,7 @@ public RequestBodyReader(long maxDecompressedBytes) { "maxDecompressedBytes must be positive, got " + maxDecompressedBytes); } this.maxDecompressedBytes = maxDecompressedBytes; + this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1) + 1; } /** @@ -67,20 +68,13 @@ private byte[] inflate(byte[] raw) throws IOException { return raw; } try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(raw), BUFFER_SIZE)) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[BUFFER_SIZE]; - long total = 0; - int read; - while ((read = in.read(buffer)) != -1) { - total += read; - if (total > maxDecompressedBytes) { - throw new BadRequestException( - HTTP_ENTITY_TOO_LARGE, - "decompressed request body exceeds " + maxDecompressedBytes + " bytes"); - } - out.write(buffer, 0, read); + byte[] inflated = in.readNBytes(readLimit); + if (inflated.length > maxDecompressedBytes) { + throw new BadRequestException( + HTTP_ENTITY_TOO_LARGE, + "decompressed request body exceeds " + maxDecompressedBytes + " bytes"); } - return out.toByteArray(); + return inflated; } catch (ZipException | EOFException e) { throw new BadRequestException(HTTP_BAD_REQUEST, "malformed gzip request body", e); } diff --git a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java index df9c6717..97d2e6cf 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java @@ -2,7 +2,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.util.Set; import java.util.zip.GZIPOutputStream; @@ -53,12 +52,4 @@ public static byte[] gzip(byte[] body) throws IOException { } return out.toByteArray(); } - - /** - * Wraps {@code out} so a streamed body is deflated as it is written. Closing the returned stream - * writes the gzip trailer and releases the deflater's native memory, so the caller must close it. - */ - public static OutputStream gzipStream(OutputStream out) throws IOException { - return new GZIPOutputStream(out); - } } diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 0625fbb7..e64d0e95 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -14,6 +14,7 @@ import java.io.OutputStream; import java.util.Locale; import java.util.Map; +import java.util.zip.GZIPOutputStream; /** Writes a {@link Response} to an {@link HttpExchange}. */ public final class ResponseRenderer { @@ -65,9 +66,7 @@ public void render(HttpExchange exchange, Response response) throws IOException */ private void renderEmpty(HttpExchange exchange, Headers headers, int status, String contentType) throws IOException { - if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { - headers.add(CONTENT_TYPE, contentType); - } + defaultContentType(headers, contentType); long declared = declaredLength(headers); if (shouldCompress(exchange, headers, status, contentType, declared) && declared >= 0) { headers.remove(CONTENT_LENGTH); @@ -78,24 +77,26 @@ private void renderEmpty(HttpExchange exchange, Headers headers, int status, Str private void renderStream( HttpExchange exchange, Headers headers, int status, String contentType, BodyWriter writer) throws IOException { - if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { - headers.add(CONTENT_TYPE, contentType); - } + defaultContentType(headers, contentType); long declared = writer instanceof BodyWriter.Sized sized ? sized.length() : UNKNOWN_LENGTH; - if (shouldCompress(exchange, headers, status, contentType, declared)) { + boolean gzip = shouldCompress(exchange, headers, status, contentType, declared); + if (gzip) { headers.set(CONTENT_ENCODING, GZIP); - exchange.sendResponseHeaders(status, CHUNKED); - try (OutputStream out = ResponseCompression.gzipStream(exchange.getResponseBody())) { - writer.writeTo(out); - } - return; } - exchange.sendResponseHeaders(status, Math.max(declared, CHUNKED)); - try (OutputStream out = exchange.getResponseBody()) { + exchange.sendResponseHeaders(status, gzip ? CHUNKED : Math.max(declared, CHUNKED)); + try (OutputStream out = + gzip ? new GZIPOutputStream(exchange.getResponseBody()) : exchange.getResponseBody()) { writer.writeTo(out); } } + /** Adds the response's own content type unless the handler already set one. */ + private static void defaultContentType(Headers headers, String contentType) { + if (contentType != null && !headers.containsKey(CONTENT_TYPE)) { + headers.add(CONTENT_TYPE, contentType); + } + } + /** * Whether a body of {@code length} bytes should be gzipped, marking the response as varying by * {@code Accept-Encoding} whenever it could have been. A negative length means unknown, which @@ -120,7 +121,7 @@ private static long declaredLength(Headers headers) { } try { return Long.parseLong(declared.trim()); - } catch (NumberFormatException e) { + } catch (NumberFormatException _) { return UNKNOWN_LENGTH; } } @@ -137,9 +138,7 @@ private void renderBytes( effectiveContentType = contentType != null ? contentType : DEFAULT_JSON; bytes = serialize(body, effectiveContentType); } - if (!headers.containsKey(CONTENT_TYPE)) { - headers.add(CONTENT_TYPE, effectiveContentType); - } + defaultContentType(headers, effectiveContentType); byte[] payload = maybeCompress(exchange, headers, status, effectiveContentType, bytes); exchange.sendResponseHeaders(status, payload.length == 0 ? -1 : payload.length); if (payload.length > 0) { diff --git a/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java index b5bfdcf8..8a27666c 100644 --- a/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java +++ b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java @@ -4,9 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.util.zip.GZIPInputStream; import org.junit.jupiter.api.Test; @@ -74,18 +72,6 @@ void gzipRoundTripsBytes() throws IOException { assertThat(gunzip(compressed)).isEqualTo(plain); } - @Test - void gzipStreamRoundTripsBytes() throws IOException { - byte[] plain = "stream me".repeat(20).getBytes(UTF_8); - ByteArrayOutputStream sink = new ByteArrayOutputStream(); - - try (OutputStream out = ResponseCompression.gzipStream(sink)) { - out.write(plain); - } - - assertThat(gunzip(sink.toByteArray())).isEqualTo(plain); - } - private static byte[] gunzip(byte[] data) throws IOException { try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) { return in.readAllBytes(); From 9c5253b7fbafc182d3baf570c73c512f97c86e1c Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:21:08 +0200 Subject: [PATCH 10/16] refactor: Trim the last of the gzip code Collapses the byte-body content-type resolution into two expressions and inlines the remaining single-use string constants. --- .../retailsvc/http/internal/ResponseCompression.java | 7 ++----- .../retailsvc/http/internal/ResponseRenderer.java | 12 +++--------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java index 97d2e6cf..52979afa 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java @@ -8,9 +8,6 @@ /** Response content-coding policy, and the gzip primitives the renderer writes through. */ public final class ResponseCompression { - private static final String TEXT_PREFIX = "text/"; - private static final String EVENT_STREAM = "text/event-stream"; - private static final Set COMPRESSIBLE_TYPES = Set.of( "application/json", @@ -35,8 +32,8 @@ public static boolean isCompressible(String contentType) { return false; } String mediaType = ContentTypeHeader.mediaType(contentType); - if (mediaType.startsWith(TEXT_PREFIX)) { - return !EVENT_STREAM.equals(mediaType); + if (mediaType.startsWith("text/")) { + return !"text/event-stream".equals(mediaType); } if (mediaType.endsWith("+json") || mediaType.endsWith("+xml") || mediaType.endsWith("+yaml")) { return true; diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index e64d0e95..5dad1e0e 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -129,15 +129,9 @@ private static long declaredLength(Headers headers) { private void renderBytes( HttpExchange exchange, Headers headers, int status, String contentType, Object body) throws IOException { - byte[] bytes; - String effectiveContentType; - if (body instanceof byte[] raw) { - bytes = raw; - effectiveContentType = contentType != null ? contentType : OCTET_STREAM; - } else { - effectiveContentType = contentType != null ? contentType : DEFAULT_JSON; - bytes = serialize(body, effectiveContentType); - } + String effectiveContentType = + contentType != null ? contentType : (body instanceof byte[] ? OCTET_STREAM : DEFAULT_JSON); + byte[] bytes = body instanceof byte[] raw ? raw : serialize(body, effectiveContentType); defaultContentType(headers, effectiveContentType); byte[] payload = maybeCompress(exchange, headers, status, effectiveContentType, bytes); exchange.sendResponseHeaders(status, payload.length == 0 ? -1 : payload.length); From cae91e69e31767fb425f2beab75382a1ec39d18e Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:24:03 +0200 Subject: [PATCH 11/16] fix: Restore the shutdownTimeoutSeconds javadoc The two content-coding setters were inserted between that javadoc and the method it documents, so it bound to nothing and shutdownTimeoutSeconds lost its documentation. Moves it back and folds the cap's two range checks into the one range they describe. Also un-nests the ternary that resolving a byte body's content type had grown, which SonarQube flags as S3358 on new code. --- .../java/com/retailsvc/http/OpenApiServer.java | 18 +++++++----------- .../http/internal/ResponseRenderer.java | 6 +++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index 6e1872dd..99ea25e0 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -391,24 +391,15 @@ public Builder https(Path certificateChainPem, Path privateKeyPem) { return this; } - /** - * Sets the default drain timeout used by {@link OpenApiServer#close()}. {@code 0} (the default) - * stops immediately; positive values wait up to that many seconds for in-flight exchanges to - * finish. - */ /** * Ceiling on the inflated size of a gzip request body, 10 MiB by default. A compressed payload * can expand by orders of magnitude, so this bounds what a single request may allocate; * exceeding it fails the request with 413. Bodies that arrive uncompressed are not affected. */ public Builder maxDecompressedRequestBytes(long maxDecompressedRequestBytes) { - if (maxDecompressedRequestBytes <= 0) { - throw new IllegalArgumentException( - "maxDecompressedRequestBytes must be positive, got " + maxDecompressedRequestBytes); - } - if (maxDecompressedRequestBytes > Integer.MAX_VALUE) { + if (maxDecompressedRequestBytes <= 0 || maxDecompressedRequestBytes > Integer.MAX_VALUE) { throw new IllegalArgumentException( - "maxDecompressedRequestBytes must not exceed " + "maxDecompressedRequestBytes must be between 1 and " + Integer.MAX_VALUE + ", got " + maxDecompressedRequestBytes); @@ -432,6 +423,11 @@ public Builder minimumGzipResponseBytes(long minimumGzipResponseBytes) { return this; } + /** + * Sets the default drain timeout used by {@link OpenApiServer#close()}. {@code 0} (the default) + * stops immediately; positive values wait up to that many seconds for in-flight exchanges to + * finish. + */ public Builder shutdownTimeoutSeconds(int shutdownTimeoutSeconds) { if (shutdownTimeoutSeconds < 0) { throw new IllegalArgumentException( diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 5dad1e0e..11e14d89 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -129,12 +129,12 @@ private static long declaredLength(Headers headers) { private void renderBytes( HttpExchange exchange, Headers headers, int status, String contentType, Object body) throws IOException { - String effectiveContentType = - contentType != null ? contentType : (body instanceof byte[] ? OCTET_STREAM : DEFAULT_JSON); + String fallback = body instanceof byte[] ? OCTET_STREAM : DEFAULT_JSON; + String effectiveContentType = contentType != null ? contentType : fallback; byte[] bytes = body instanceof byte[] raw ? raw : serialize(body, effectiveContentType); defaultContentType(headers, effectiveContentType); byte[] payload = maybeCompress(exchange, headers, status, effectiveContentType, bytes); - exchange.sendResponseHeaders(status, payload.length == 0 ? -1 : payload.length); + exchange.sendResponseHeaders(status, payload.length == 0 ? UNKNOWN_LENGTH : payload.length); if (payload.length > 0) { try (OutputStream out = exchange.getResponseBody()) { out.write(payload); From 0ac5e48736ee9d850549a569e36c6004c70ce392 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:40:39 +0200 Subject: [PATCH 12/16] fix: Drop a handler's Content-Length when a stream is compressed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A compressed stream goes out chunked, and the JDK's chunked branch sets Transfer-encoding without clearing a Content-Length the handler put on the response — so both framing headers reached the wire together. Before this branch a sized stream passed its real length, which made the JDK overwrite that header, so the conflict is new. The declared length also describes the uncompressed body, so it is wrong on the wire regardless of framing. renderEmpty already removed it for the same reason; renderStream now does too. --- .../retailsvc/http/internal/ResponseRenderer.java | 2 ++ .../http/internal/ResponseRendererTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 11e14d89..96bc5c8f 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -82,6 +82,8 @@ private void renderStream( boolean gzip = shouldCompress(exchange, headers, status, contentType, declared); if (gzip) { headers.set(CONTENT_ENCODING, GZIP); + // The coded body goes out chunked, and the JDK leaves a handler-set length in place there. + headers.remove(CONTENT_LENGTH); } exchange.sendResponseHeaders(status, gzip ? CHUNKED : Math.max(declared, CHUNKED)); try (OutputStream out = diff --git a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java index 1377b8f0..cff6f4a3 100644 --- a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java +++ b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java @@ -397,6 +397,21 @@ void skipsCompressionOnStreamWhenHandlerAlreadySetContentEncoding() throws IOExc assertThat(sink.toByteArray()).isEqualTo(payload); } + @Test + void stripsHandlerContentLengthWhenStreamIsCompressed() throws IOException { + acceptsGzip(); + byte[] payload = largeText(); + + renderer.render( + exchange, + Response.stream(HTTP_OK, payload.length, TEXT, out -> out.write(payload)) + .withHeader("Content-Length", String.valueOf(payload.length))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + assertThat(responseHeaders.getFirst("Content-Length")).isNull(); + assertThat(length.get()).isZero(); + } + private void acceptsGzip() { requestHeaders.add("Accept-Encoding", "gzip, deflate, br"); } From 5807c59e135d037b78b89cd7f6a284bd6d048e01 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:49:33 +0200 Subject: [PATCH 13/16] ci: Run the pre-commit formatter on the project's JDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-commit workflow set up Python but no Java, so google-java-format ran on the runner's default JDK 17 and could not parse this project's Java 25 sources — every file using an unnamed `_` binding or a record pattern failed to parse. Those constructs predate this branch; the job only fails when a pull request happens to touch such a file. Adds the same setup-java step pull_request.yaml already uses, keyed off .java-version, and bumps extenda/pre-commit-hooks to v0.16.1. Verified locally across the matrix: on JDK 17 the formatter fails with either jar version (1.28.0 cannot parse `_`, 1.36.1 throws LinkageError); on JDK 25 both pass. The JDK is the fix, the bump is housekeeping. --- .github/workflows/pre-commit.yml | 6 ++++++ .pre-commit-config.yaml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 343bd63e..1c354541 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -12,6 +12,12 @@ jobs: - name: Setup Python uses: actions/setup-python@v7 + - name: Setup Java + uses: actions/setup-java@v6 + with: + distribution: temurin + java-version-file: .java-version + - name: Run pre-commit uses: pre-commit/actions@v3.0.1 with: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bed54ace..e9661e2b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,7 +17,7 @@ repos: hooks: - id: editorconfig-checker - repo: https://github.com/extenda/pre-commit-hooks - rev: v0.15.0 + rev: v0.16.1 hooks: - id: google-java-formatter - id: commitlint From 15d49536d3c57fed55a3e741a41e8b127432e0f4 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Thu, 10 Sep 2026 15:51:56 +0200 Subject: [PATCH 14/16] fix: Resolve the SonarCloud findings on the gzip code - RequestBodyReader: do the clamp subtraction in long so the int arithmetic cannot be read as a narrowing hazard (S2184). The value is unchanged. - GzipIT: assert with hasSizeLessThan on the array rather than on its length field. - RequestBodyReaderTest: drop a throws IOException the body cannot throw, since the gzip call sits inside the assertion lambda. - RequestPreparationFilterTest: static-import the Mockito DSL, matching the convention the rest of the suite already follows. --- .../http/internal/RequestBodyReader.java | 2 +- src/test/java/com/retailsvc/http/GzipIT.java | 2 +- .../http/internal/RequestBodyReaderTest.java | 2 +- .../RequestPreparationFilterTest.java | 31 ++++++++++--------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java index 170cff06..4efc8861 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java +++ b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java @@ -36,7 +36,7 @@ public RequestBodyReader(long maxDecompressedBytes) { "maxDecompressedBytes must be positive, got " + maxDecompressedBytes); } this.maxDecompressedBytes = maxDecompressedBytes; - this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1) + 1; + this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1L) + 1; } /** diff --git a/src/test/java/com/retailsvc/http/GzipIT.java b/src/test/java/com/retailsvc/http/GzipIT.java index 5301cea7..fce83739 100644 --- a/src/test/java/com/retailsvc/http/GzipIT.java +++ b/src/test/java/com/retailsvc/http/GzipIT.java @@ -133,7 +133,7 @@ void largeResponseIsGzippedWhenClientAcceptsGzip() throws Exception { assertThat(response.headers().firstValue("Vary")) .hasValueSatisfying(vary -> assertThat(vary).contains(ACCEPT_ENCODING)); assertThat(new String(gunzip(response.body()), UTF_8)).isEqualTo(payload); - assertThat(response.body().length).isLessThan(payload.length()); + assertThat(response.body()).hasSizeLessThan(payload.length()); } } diff --git a/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java index 09d0d297..4fa093b2 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java @@ -77,7 +77,7 @@ void unsupportedCodingThrows415() { } @Test - void oversizedInflatedBodyThrows413() throws IOException { + void oversizedInflatedBodyThrows413() { byte[] bomb = new byte[(int) CAP * 4]; assertThatThrownBy(() -> reader.read(exchange(gzip(bomb), "gzip"))) diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index 7608d04f..66c8c7f7 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -4,7 +4,11 @@ import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.retailsvc.http.BadRequestException; import com.retailsvc.http.ExceptionHandler; @@ -41,7 +45,6 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.zip.GZIPOutputStream; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; class RequestPreparationFilterTest { @@ -51,10 +54,10 @@ private HttpExchange exchange(String method, String path, byte[] body) { private HttpExchange exchange(String method, String path, byte[] body, Headers headers) { HttpExchange ex = mock(HttpExchange.class); - Mockito.when(ex.getRequestMethod()).thenReturn(method); - Mockito.when(ex.getRequestURI()).thenReturn(URI.create(path)); - Mockito.when(ex.getRequestHeaders()).thenReturn(headers); - Mockito.when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body)); + when(ex.getRequestMethod()).thenReturn(method); + when(ex.getRequestURI()).thenReturn(URI.create(path)); + when(ex.getRequestHeaders()).thenReturn(headers); + when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body)); return ex; } @@ -140,7 +143,7 @@ void successPathBindsRequestContextDuringChain() throws Exception { AtomicReference> seenPathParams = new AtomicReference<>(); Filter.Chain chain = mock(Filter.Chain.class); - Mockito.doAnswer( + doAnswer( inv -> { Request req = DispatchHandler.CURRENT.get(); seenOpId.set(req.operationId()); @@ -148,13 +151,13 @@ void successPathBindsRequestContextDuringChain() throws Exception { return null; }) .when(chain) - .doFilter(Mockito.any()); + .doFilter(any()); f.doFilter(ex, chain); assertThat(seenOpId.get()).isEqualTo("get-user"); assertThat(seenPathParams.get()).containsEntry("id", "42"); - Mockito.verify(chain).doFilter(ex); + verify(chain).doFilter(ex); } @Test @@ -241,7 +244,7 @@ void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception { HttpExchange ex = exchange("GET", "/x?n=42", new byte[0]); Filter.Chain chain = mock(Filter.Chain.class); f.doFilter(ex, chain); - Mockito.verify(chain).doFilter(ex); + verify(chain).doFilter(ex); } @Test @@ -288,7 +291,7 @@ void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception { HttpExchange ex = exchange("GET", "/x?n=1.5", new byte[0]); Filter.Chain chain = mock(Filter.Chain.class); f.doFilter(ex, chain); - Mockito.verify(chain).doFilter(ex); + verify(chain).doFilter(ex); } @Test @@ -337,8 +340,8 @@ void booleanQueryParamCoercesTrueAndFalse() throws Exception { HttpExchange falseEx = exchange("GET", "/x?b=false", new byte[0]); f.doFilter(trueEx, trueChain); f.doFilter(falseEx, falseChain); - Mockito.verify(trueChain).doFilter(trueEx); - Mockito.verify(falseChain).doFilter(falseEx); + verify(trueChain).doFilter(trueEx); + verify(falseChain).doFilter(falseEx); } @Test @@ -383,7 +386,7 @@ void gzipRequestBodyIsInflatedBeforeValidation() throws Exception { AtomicReference seenBody = new AtomicReference<>(); AtomicReference seenEncoding = new AtomicReference<>("still here"); Filter.Chain chain = mock(Filter.Chain.class); - Mockito.doAnswer( + doAnswer( inv -> { Request req = DispatchHandler.CURRENT.get(); seenBody.set(req.bytes()); @@ -391,7 +394,7 @@ void gzipRequestBodyIsInflatedBeforeValidation() throws Exception { return null; }) .when(chain) - .doFilter(Mockito.any()); + .doFilter(any()); f.doFilter(ex, chain); From 327cc354277e7eff3b5511c70b4c543422d8b459 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 11 Sep 2026 16:26:04 +0200 Subject: [PATCH 15/16] refactor: Name the response threshold for what it measures minimumGzipResponseBytes becomes minCompressibleResponseBytes, and ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES becomes DEFAULT_MIN_COMPRESSIBLE_BYTES. The threshold is compared against the uncompressed body before any coding is chosen, so it neither depends on gzip nor measures a compressed size. "Compressible" says what it does: a body smaller than this is not worth coding. It now mirrors maxDecompressedRequestBytes - min and max, and both measure plain bytes. Neither name has been released, and every push to master publishes to Maven Central, so this is the last point at which the rename is free rather than a breaking change to a public builder method. --- README.md | 2 +- .../java/com/retailsvc/http/OpenApiServer.java | 15 ++++++++------- .../retailsvc/http/internal/ResponseRenderer.java | 12 ++++++------ .../retailsvc/http/OpenApiServerBuilderTest.java | 6 +++--- .../http/internal/DispatchHandlerTest.java | 9 ++++++--- .../retailsvc/http/internal/ExtrasRouterTest.java | 4 ++-- .../internal/RequestPreparationFilterTest.java | 4 ++-- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index b6838a95..1af38f04 100644 --- a/README.md +++ b/README.md @@ -497,7 +497,7 @@ coded, and neither is `text/event-stream`, which has to stay unbuffered. OpenApiServer.builder() .spec(spec) .handlers(handlers) - .minimumGzipResponseBytes(4096) // raises the 1 KiB default; 0 compresses every eligible body + .minCompressibleResponseBytes(4096) // raises the 1 KiB default; 0 compresses every eligible body .build(); ``` diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index 99ea25e0..e87cfde5 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -246,7 +246,7 @@ public static final class Builder { private final Map securityValidators = new LinkedHashMap<>(); private boolean externalAuth = false; private long maxDecompressedRequestBytes = RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES; - private long minimumGzipResponseBytes = ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; + private long minCompressibleResponseBytes = ResponseRenderer.DEFAULT_MIN_COMPRESSIBLE_BYTES; private final List bindings = new ArrayList<>(); private Builder() {} @@ -409,17 +409,18 @@ public Builder maxDecompressedRequestBytes(long maxDecompressedRequestBytes) { } /** - * Smallest response body worth gzipping, 1 KiB by default. Below this, the coding costs more + * Smallest response body worth compressing, 1 KiB by default. Below this, the coding costs more * than it saves. Set it to 0 to compress every compressible body, or high enough to exceed any * response this server produces to stop compressing altogether — useful when a proxy in front * already terminates compression. */ - public Builder minimumGzipResponseBytes(long minimumGzipResponseBytes) { - if (minimumGzipResponseBytes < 0) { + public Builder minCompressibleResponseBytes(long minCompressibleResponseBytes) { + if (minCompressibleResponseBytes < 0) { throw new IllegalArgumentException( - "minimumGzipResponseBytes must be non-negative, got " + minimumGzipResponseBytes); + "minCompressibleResponseBytes must be non-negative, got " + + minCompressibleResponseBytes); } - this.minimumGzipResponseBytes = minimumGzipResponseBytes; + this.minCompressibleResponseBytes = minCompressibleResponseBytes; return this; } @@ -471,7 +472,7 @@ public OpenApiServer build() throws IOException { externalAuth, List.copyOf(afterHooks), new RequestBodyReader(maxDecompressedRequestBytes), - new ResponseRenderer(resolved, minimumGzipResponseBytes)); + new ResponseRenderer(resolved, minCompressibleResponseBytes)); int resolvedPort = resolvePort(); SSLContext sslContext = httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index 96bc5c8f..a941dc35 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -19,8 +19,8 @@ /** Writes a {@link Response} to an {@link HttpExchange}. */ public final class ResponseRenderer { - /** Default smallest body worth gzipping: 1 KiB. */ - public static final long DEFAULT_MINIMUM_GZIP_BYTES = 1024; + /** Default smallest body worth compressing: 1 KiB. */ + public static final long DEFAULT_MIN_COMPRESSIBLE_BYTES = 1024; private static final String CONTENT_TYPE = "Content-Type"; private static final String CONTENT_ENCODING = "Content-Encoding"; @@ -34,11 +34,11 @@ public final class ResponseRenderer { private static final String OCTET_STREAM = "application/octet-stream"; private final Map mappers; - private final long minimumGzipBytes; + private final long minCompressibleBytes; - public ResponseRenderer(Map mappers, long minimumGzipBytes) { + public ResponseRenderer(Map mappers, long minCompressibleBytes) { this.mappers = Map.copyOf(mappers); - this.minimumGzipBytes = minimumGzipBytes; + this.minCompressibleBytes = minCompressibleBytes; } public void render(HttpExchange exchange, Response response) throws IOException { @@ -112,7 +112,7 @@ private boolean shouldCompress( return false; } addVary(headers); - return (length < 0 || length >= minimumGzipBytes) && acceptsGzip(exchange); + return (length < 0 || length >= minCompressibleBytes) && acceptsGzip(exchange); } /** The length a handler declared for a body it did not write, or -1 when absent or unreadable. */ diff --git a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java index 8a99c4ce..59723819 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java @@ -71,10 +71,10 @@ void rejectsOversizedMaxDecompressedRequestBytes() { } @Test - void rejectsNegativeMinimumGzipResponseBytes() { + void rejectsNegativeMinCompressibleResponseBytes() { OpenApiServer.Builder b = OpenApiServer.builder(); - assertThatThrownBy(() -> b.minimumGzipResponseBytes(-1)) + assertThatThrownBy(() -> b.minCompressibleResponseBytes(-1)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("-1"); } @@ -83,7 +83,7 @@ void rejectsNegativeMinimumGzipResponseBytes() { void acceptsContentCodingLimits() { OpenApiServer.Builder b = OpenApiServer.builder(); - assertThat(b.maxDecompressedRequestBytes(4096).minimumGzipResponseBytes(0)).isSameAs(b); + assertThat(b.maxDecompressedRequestBytes(4096).minCompressibleResponseBytes(0)).isSameAs(b); } @Test diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index 32846bc4..3ffdea6d 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -1,6 +1,6 @@ package com.retailsvc.http.internal; -import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MIN_COMPRESSIBLE_BYTES; import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static java.net.HttpURLConnection.HTTP_OK; import static org.assertj.core.api.Assertions.assertThat; @@ -46,7 +46,10 @@ private static HttpExchange stubExchange() { private static DispatchHandler dispatcher(Map handlers) { return new DispatchHandler( - handlers, List.of(), List.of(), new ResponseRenderer(Map.of(), DEFAULT_MINIMUM_GZIP_BYTES)); + handlers, + List.of(), + List.of(), + new ResponseRenderer(Map.of(), DEFAULT_MIN_COMPRESSIBLE_BYTES)); } private static DispatchHandler dispatcher( @@ -57,7 +60,7 @@ private static DispatchHandler dispatcher( handlers, interceptors, decorators, - new ResponseRenderer(Map.of(), DEFAULT_MINIMUM_GZIP_BYTES)); + new ResponseRenderer(Map.of(), DEFAULT_MIN_COMPRESSIBLE_BYTES)); } private static void withRequest(String operationId, ScopedValue.CallableOp body) diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java index 314764b4..ca7ed15e 100644 --- a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java @@ -1,6 +1,6 @@ package com.retailsvc.http.internal; -import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MIN_COMPRESSIBLE_BYTES; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; @@ -146,7 +146,7 @@ private static ExtrasRouter newRouter(Map extras) { Map mappers = Map.of("application/json", new GsonTypeMapper()); return new ExtrasRouter( extras, - new ResponseRenderer(mappers, DEFAULT_MINIMUM_GZIP_BYTES), + new ResponseRenderer(mappers, DEFAULT_MIN_COMPRESSIBLE_BYTES), new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index 66c8c7f7..14025ad6 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -1,6 +1,6 @@ package com.retailsvc.http.internal; -import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MINIMUM_GZIP_BYTES; +import static com.retailsvc.http.internal.ResponseRenderer.DEFAULT_MIN_COMPRESSIBLE_BYTES; import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -118,7 +118,7 @@ public byte[] writeTo(Object value) { new DefaultValidator(spec::resolveSchema), mappers, rethrow, - new ResponseRenderer(mappers, DEFAULT_MINIMUM_GZIP_BYTES), + new ResponseRenderer(mappers, DEFAULT_MIN_COMPRESSIBLE_BYTES), List.of(), new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); } From 319078ae0ecefc1b4ed927adc3976e9c3ed2d738 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 11 Sep 2026 16:53:26 +0200 Subject: [PATCH 16/16] feat: Let callers register their own content codings Adds ContentCoding, a public extension point for HTTP content codings, so a service can offer zstd, brotli or deflate without this library taking a compression dependency. gzip becomes the built-in instance of the same interface. Codings are registered on the builder with contentCoding, or with requestContentCoding / responseContentCoding for one direction only; a request coded with a response-only coding is answered 415. The client's q-weights pick the response coding, and on a tie registered codings win over gzip in registration order. The interface wraps streams rather than whole bodies. That keeps the decompression cap in the library: the server reads at most maxDecompressedRequestBytes from whatever stream a coding returns, so a lazy decoder is bounded without doing anything itself. Registration fails fast on reserved tokens (gzip, x-gzip, identity, *), on duplicates within a direction, and on anything that is not a lower-case RFC 9110 token. Tokens are written verbatim into response headers, so that check is what keeps them free of injected CR/LF. --- CLAUDE.md | 6 +- README.md | 32 ++- .../com/retailsvc/http/ContentCoding.java | 53 ++++ .../com/retailsvc/http/OpenApiServer.java | 47 +++- .../http/internal/AcceptEncodingHeader.java | 80 ++++-- .../http/internal/ContentCodings.java | 86 +++++++ .../http/internal/ContentEncodingHeader.java | 53 ++-- .../retailsvc/http/internal/GzipCoding.java | 36 +++ .../http/internal/RequestBodyReader.java | 61 ++--- .../http/internal/ResponseCompression.java | 15 +- .../http/internal/ResponseRenderer.java | 55 +++-- .../com/retailsvc/http/ContentCodingIT.java | 229 ++++++++++++++++++ .../http/OpenApiServerBuilderTest.java | 38 +++ .../internal/AcceptEncodingHeaderTest.java | 97 ++++++-- .../http/internal/ContentCodingsTest.java | 130 ++++++++++ .../internal/ContentEncodingHeaderTest.java | 70 ++++-- .../http/internal/DispatchHandlerTest.java | 10 +- .../http/internal/ExtrasRouterTest.java | 10 +- .../http/internal/RequestBodyReaderTest.java | 105 +++++++- .../RequestPreparationFilterTest.java | 9 +- .../internal/ResponseCompressionTest.java | 2 +- .../http/internal/ResponseRendererTest.java | 105 +++++++- .../retailsvc/http/support/TestCodings.java | 60 +++++ 23 files changed, 1209 insertions(+), 180 deletions(-) create mode 100644 src/main/java/com/retailsvc/http/ContentCoding.java create mode 100644 src/main/java/com/retailsvc/http/internal/ContentCodings.java create mode 100644 src/main/java/com/retailsvc/http/internal/GzipCoding.java create mode 100644 src/test/java/com/retailsvc/http/ContentCodingIT.java create mode 100644 src/test/java/com/retailsvc/http/internal/ContentCodingsTest.java create mode 100644 src/test/java/com/retailsvc/http/support/TestCodings.java diff --git a/CLAUDE.md b/CLAUDE.md index 5182ad6a..15ec717a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,11 +28,11 @@ Request flow when `OpenApiServer` boots (`src/main/java/com/retailsvc/http/OpenA 1. `HttpServer` is created on a port with a virtual-thread-per-task executor. 2. One `HttpContext` is registered per spec binding at `spec.basePath()` (the first `servers[].url` path from the OpenAPI doc). Unless a binding owns `/`, a catch-all `/` context serves extra routes via `ExtrasRouter` and 404s everything else; `ExceptionFilter` wraps that context only. 3. On a binding context, two filters run in order, then the handler: - - `RequestPreparationFilter` — reads the request body through `RequestBodyReader` (which inflates a gzip `Content-Encoding` under a size cap), resolves the route, runs OpenAPI parameter + body validation via `DefaultValidator`, and binds the resulting `Request` into the `DispatchHandler.CURRENT` scoped value. It renders its own failures through the `ExceptionHandler` rather than relying on `ExceptionFilter`. + - `RequestPreparationFilter` — reads the request body through `RequestBodyReader` (which decodes a registered `Content-Encoding` — gzip is built in — under a size cap), resolves the route, runs OpenAPI parameter + body validation via `DefaultValidator`, and binds the resulting `Request` into the `DispatchHandler.CURRENT` scoped value. It renders its own failures through the `ExceptionHandler` rather than relying on `ExceptionFilter`. - `SecurityFilter` — enforces the spec's `securitySchemes` / `security`, re-binding the `Request` with resolved principals. It writes its 401/403 responses straight to the exchange. - `DispatchHandler` — looks up the `RequestHandler` registered for the resolved `operationId` in the user-supplied map and invokes it, applying interceptors and response decorators. Handler coverage is verified at boot, so the lookup never returns `null`. -Every response except `SecurityFilter`'s rejections is written by `ResponseRenderer`, which is also where response gzip coding is applied. +Every response except `SecurityFilter`'s rejections is written by `ResponseRenderer`, which is also where response content coding is applied. Key abstractions: @@ -42,7 +42,7 @@ Key abstractions: - `com.retailsvc.http.internal.Router` — two indexes: exact path map and templated path list. Resolves `operationId` + extracted path variables for each request. - `TypeMapper` — per-media-type request parsing and response writing; registered via `Builder.bodyMapper(...)`, with `GsonTypeMapper` auto-registered when Gson is on the classpath. - `com.retailsvc.http.Request` — an immutable record-like carrier built from primitives (body bytes, path parameters, raw query string, a header lookup function), never the `HttpExchange`. `bytes()` returns the decoded body, `parsed()` the object produced by the `TypeMapper`. -- `com.retailsvc.http.internal.RequestBodyReader` / `ResponseCompression` — inbound and outbound gzip. See the README's "Content encoding" section for the policy. +- `com.retailsvc.http.ContentCoding` — a pluggable HTTP content coding. gzip is built in (`internal/GzipCoding`); callers register others on the builder, held per direction in `internal/ContentCodings`. `RequestBodyReader` decodes requests under the size cap and `ResponseRenderer` codes responses. See the README's "Content encoding" section for the policy. ## Conventions diff --git a/README.md b/README.md index 1af38f04..b03cde2f 100644 --- a/README.md +++ b/README.md @@ -468,8 +468,8 @@ gzip is handled in both directions, with no configuration required. **Requests.** A body sent with `Content-Encoding: gzip` is inflated before OpenAPI validation runs, so the validator, your `TypeMapper` and your handler all see plain bytes. `identity` is accepted as -the no-op it is. Any other coding — `br`, `deflate`, or two codings stacked — is rejected with -`415 Unsupported Media Type`, and a corrupt or truncated gzip stream with `400 Bad Request`. +the no-op it is. A coding the server has not registered — `br`, say — or two codings stacked is +rejected with `415 Unsupported Media Type`, and a corrupt or truncated body with `400 Bad Request`. Once a body is inflated it no longer matches the headers that described it, so `Content-Encoding` is hidden from `Request.header(...)` and `Content-Length` reports the inflated size. @@ -484,8 +484,8 @@ OpenApiServer.builder() .build(); ``` -Note this bounds the *inflated* size of a gzip body. It is not a request size limit — a body that -arrives uncompressed is read in full, as it always has been. +Note this bounds the *inflated* size of a coded body, whatever the coding. It is not a request size +limit — a body that arrives uncompressed is read in full, as it always has been. **Responses.** A body is gzipped when the client sends `Accept-Encoding: gzip`, the media type is text-shaped (`text/*`, `application/json`, `application/xml`, `application/yaml`, and the `+json` / @@ -506,18 +506,34 @@ threshold above anything this server returns. `Vary: Accept-Encoding` is set whenever a body *could* have been coded, not only when it was, so shared caches keep the two forms apart. It is merged into any `Vary` your handler already set. -A handler that sets its own `Content-Encoding` is left alone, and so is a payload gzip fails to -shrink. Statuses that carry no content never get a coding. +A handler that sets its own `Content-Encoding` is left alone, and so is a payload the coding +fails to shrink. Statuses that carry no content never get a coding. -Streamed responses (`Response.stream(...)`) are deflated as they are written. A length declared by +Streamed responses (`Response.stream(...)`) are coded as they are written. A length declared by the sized overload describes the uncoded body, so a coded stream goes out chunked; a stream of unknown length is coded regardless of the threshold, since measuring it would defeat streaming it. For the same reason a `HEAD` whose `GET` would be compressed omits `Content-Length` rather than advertising the uncoded length. +**Other codings.** The library ships gzip only, and so carries no compression dependency. To offer +another, implement `ContentCoding` and register it: + +```java +OpenApiServer.builder() + .spec(spec) + .handlers(handlers) + .contentCoding(new ZstdCoding()) // your ContentCoding implementation + .build(); +``` + +The client's weights pick the coding; on a tie, registered codings win over gzip, in registration +order. `decode` and `encode` wrap streams rather than whole bodies, so a decoder that reads lazily +is held to `maxDecompressedRequestBytes` without doing anything itself. `requestContentCoding` and +`responseContentCoding` register one direction only; a request coded with a response-only coding +gets 415. Tokens must be lower-case, and `gzip`, `x-gzip`, `identity` and `*` are reserved. + **Not in this release** (each can land later without breaking the API): -- brotli, zstd and `deflate`, in either direction - the `Accept-Encoding` response header RFC 9110 recommends alongside a 415 - compression of the `401`/`403` bodies produced by security scheme enforcement — those bypass the renderer and are well under any sensible threshold diff --git a/src/main/java/com/retailsvc/http/ContentCoding.java b/src/main/java/com/retailsvc/http/ContentCoding.java new file mode 100644 index 00000000..c65a1b4b --- /dev/null +++ b/src/main/java/com/retailsvc/http/ContentCoding.java @@ -0,0 +1,53 @@ +package com.retailsvc.http; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Set; + +/** + * An HTTP content coding (RFC 9110 §8.4.1) that the server decodes on requests and applies to + * responses. The library ships {@code gzip}; anything else — {@code zstd}, {@code br}, {@code + * deflate} — is supplied by the caller and registered on {@link + * OpenApiServer.Builder#contentCoding(ContentCoding)}, which keeps this library free of any + * compression dependency. + * + *

One instance serves every request, so implementations must be immutable and safe for + * concurrent use. + * + *

Both methods wrap a stream rather than convert a whole body: return a stream that codes as it + * is read or written. For {@link #decode} this is what bounds the work — the server reads at most + * {@code maxDecompressedRequestBytes} from the stream you return, so a lazy decoder is protected + * from decompression bombs without doing anything, while one that expands the whole body up front + * has already spent what that limit exists to protect. + */ +public interface ContentCoding { + + /** + * The {@code Content-Encoding} and {@code Accept-Encoding} token, such as {@code zstd}. Must be a + * lower-case RFC 9110 token; {@code gzip}, {@code x-gzip}, {@code identity} and {@code *} are + * reserved. + */ + String token(); + + /** + * Further tokens that mean this same coding on the wire, such as a legacy {@code x-} name. + * Recognised on input only; a coded response always announces {@link #token()}. + */ + default Set aliases() { + return Set.of(); + } + + /** + * Wraps a coded request body in a stream of the decoded bytes. The argument is always held in + * memory, so a read failure means the body is malformed: throw {@link IOException} and the server + * answers 400. Any other exception is treated as a fault in the coding. + */ + InputStream decode(InputStream coded) throws IOException; + + /** + * Wraps a response stream so that everything written to the returned stream reaches {@code sink} + * coded. Closing the returned stream must finish the coded payload and close {@code sink}. + */ + OutputStream encode(OutputStream sink) throws IOException; +} diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index e87cfde5..89f09691 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -4,6 +4,7 @@ import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newThreadPerTaskExecutor; +import com.retailsvc.http.internal.ContentCodings; import com.retailsvc.http.internal.DispatchHandler; import com.retailsvc.http.internal.ExceptionFilter; import com.retailsvc.http.internal.ExtrasRouter; @@ -247,6 +248,8 @@ public static final class Builder { private boolean externalAuth = false; private long maxDecompressedRequestBytes = RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES; private long minCompressibleResponseBytes = ResponseRenderer.DEFAULT_MIN_COMPRESSIBLE_BYTES; + private final List requestCodings = new ArrayList<>(); + private final List responseCodings = new ArrayList<>(); private final List bindings = new ArrayList<>(); private Builder() {} @@ -424,6 +427,45 @@ public Builder minCompressibleResponseBytes(long minCompressibleResponseBytes) { return this; } + /** + * Registers a content coding the server decodes on requests and applies to responses, alongside + * the built-in gzip. When a client weights several codings equally, the ones registered here + * win over gzip, in registration order; otherwise the client's weights decide. A decoded body + * is still held to {@link #maxDecompressedRequestBytes(long)}. + * + * @throws IllegalArgumentException if a token or alias is not a lower-case RFC 9110 token, or + * is one of the reserved {@code gzip}, {@code x-gzip}, {@code identity} or {@code *} + * @throws IllegalStateException if a token or alias is already registered in either direction + */ + public Builder contentCoding(ContentCoding coding) { + ContentCodings.requireRegistrable(coding, requestCodings); + ContentCodings.requireRegistrable(coding, responseCodings); + requestCodings.add(coding); + responseCodings.add(coding); + return this; + } + + /** + * Registers a coding the server only decodes on requests; responses never use it. Validated as + * for {@link #contentCoding(ContentCoding)}. + */ + public Builder requestContentCoding(ContentCoding coding) { + ContentCodings.requireRegistrable(coding, requestCodings); + requestCodings.add(coding); + return this; + } + + /** + * Registers a coding the server only applies to responses. A request coded with it is answered + * 415, which keeps a decoder you have no use for off the request path. Validated as for {@link + * #contentCoding(ContentCoding)}. + */ + public Builder responseContentCoding(ContentCoding coding) { + ContentCodings.requireRegistrable(coding, responseCodings); + responseCodings.add(coding); + return this; + } + /** * Sets the default drain timeout used by {@link OpenApiServer#close()}. {@code 0} (the default) * stops immediately; positive values wait up to that many seconds for in-flight exchanges to @@ -463,6 +505,7 @@ public OpenApiServer build() throws IOException { Map resolved = resolveBodyMappers(bodyMappers); ExceptionHandler effectiveExceptionHandler = exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler(); + ContentCodings codings = ContentCodings.of(requestCodings, responseCodings); HandlerConfig handlerConfig = new HandlerConfig( interceptors, @@ -471,8 +514,8 @@ public OpenApiServer build() throws IOException { extras, externalAuth, List.copyOf(afterHooks), - new RequestBodyReader(maxDecompressedRequestBytes), - new ResponseRenderer(resolved, minCompressibleResponseBytes)); + new RequestBodyReader(maxDecompressedRequestBytes, codings.decoders()), + new ResponseRenderer(resolved, minCompressibleResponseBytes, codings.encoders())); int resolvedPort = resolvePort(); SSLContext sslContext = httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; diff --git a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java index a83a017a..1f27c14a 100644 --- a/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java +++ b/src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java @@ -1,51 +1,87 @@ package com.retailsvc.http.internal; +import com.retailsvc.http.ContentCoding; +import java.util.HashMap; +import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Optional; /** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */ public final class AcceptEncodingHeader { + private static final double DEFAULT_WEIGHT = 1.0; + private AcceptEncodingHeader() {} /** - * Whether the client accepts a gzip-coded response. A {@code null}, blank, or unrelated header - * yields {@code false}. An explicit {@code gzip;q=0} is a refusal and outranks a positive - * wildcard; a wildcard applies only when gzip is not listed in its own right. + * Chooses the coding the client weights highest among {@code candidates}, which arrive in server + * preference order, so the earlier candidate wins a tie. Empty means send the body uncoded — also + * the answer for an absent header, since not asking for a coding is not the same as accepting + * any. + * + *

A coding the client names outranks its {@code *} entry, so {@code gzip;q=0} refuses gzip + * even beside a positive wildcard. A weight of zero refuses, and a name listed twice takes its + * higher weight. */ - public static boolean acceptsGzip(String header) { - if (header == null) { - return false; + public static Optional select(String header, List candidates) { + if (header == null || candidates.isEmpty()) { + return Optional.empty(); + } + Map weights = weights(header); + ContentCoding best = null; + double bestWeight = 0; + for (ContentCoding candidate : candidates) { + double weight = weightOf(candidate, weights); + if (weight > bestWeight) { + best = candidate; + bestWeight = weight; + } } - boolean gzipSeen = false; - boolean gzipAccepted = false; - boolean wildcardAccepted = false; + return Optional.ofNullable(best); + } + + /** The weight the client gave each coding it listed, keeping the higher for a repeated one. */ + private static Map weights(String header) { + Map weights = new HashMap<>(); for (String token : header.split(",")) { int semi = token.indexOf(';'); String coding = (semi < 0 ? token : token.substring(0, semi)).trim().toLowerCase(Locale.ROOT); - boolean accepted = positiveWeight(token); - if ("gzip".equals(coding) || "x-gzip".equals(coding)) { - gzipSeen = true; - gzipAccepted |= accepted; - } else if ("*".equals(coding)) { - wildcardAccepted |= accepted; + if (!coding.isEmpty()) { + weights.merge(coding, weight(token), Math::max); } } - return gzipSeen ? gzipAccepted : wildcardAccepted; + return weights; + } + + /** A candidate's weight from its own name or an alias, and only failing both, the wildcard. */ + private static double weightOf(ContentCoding coding, Map weights) { + Double listed = weights.get(coding.token()); + for (String alias : coding.aliases()) { + Double aliased = weights.get(alias); + if (aliased != null && (listed == null || aliased > listed)) { + listed = aliased; + } + } + if (listed != null) { + return listed; + } + return weights.getOrDefault("*", 0.0); } /** - * Whether the token's {@code q} weight admits the coding. An absent or unparsable weight reads as - * the default 1.0 — a malformed header should not silently disable compression. + * Reads the {@code q} weight from a token. An absent or unparsable weight reads as the default + * 1.0 — a malformed header should not silently disable compression. */ - private static boolean positiveWeight(String token) { + private static double weight(String token) { String weight = ContentTypeHeader.parameter(token, "q").orElse(null); if (weight == null) { - return true; + return DEFAULT_WEIGHT; } try { - return Double.parseDouble(weight) > 0; + return Double.parseDouble(weight); } catch (NumberFormatException _) { - return true; + return DEFAULT_WEIGHT; } } } diff --git a/src/main/java/com/retailsvc/http/internal/ContentCodings.java b/src/main/java/com/retailsvc/http/internal/ContentCodings.java new file mode 100644 index 00000000..20055bf4 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/ContentCodings.java @@ -0,0 +1,86 @@ +package com.retailsvc.http.internal; + +import static java.util.Objects.requireNonNull; + +import com.retailsvc.http.ContentCoding; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * The content codings a server decodes and applies: the built-in gzip plus whatever the caller + * registered, split by direction. + * + * @param decoders request codings keyed by every token that names them, aliases included + * @param encoders response codings in server preference order, the built-in gzip last + */ +public record ContentCodings(Map decoders, List encoders) { + + private static final ContentCoding GZIP = new GzipCoding(); + private static final Set RESERVED = Set.of("gzip", "x-gzip", "identity", "*"); + private static final String TOKEN_SYMBOLS = "!#$%&'*+-.^_`|~"; + + public ContentCodings { + decoders = Map.copyOf(decoders); + encoders = List.copyOf(encoders); + } + + /** + * Builds the registry from codings already accepted by {@link #requireRegistrable}. Registered + * codings come before gzip, so a client that weights them equally gets the one the caller added. + */ + public static ContentCodings of( + List requestCodings, List responseCodings) { + Map decoders = new HashMap<>(); + for (ContentCoding coding : requestCodings) { + names(coding).forEach(name -> decoders.put(name, coding)); + } + names(GZIP).forEach(name -> decoders.put(name, GZIP)); + List encoders = new ArrayList<>(responseCodings); + encoders.add(GZIP); + return new ContentCodings(decoders, encoders); + } + + /** + * Rejects a coding that cannot be registered alongside {@code registered}: a token or alias that + * is not a lower-case RFC 9110 token, one that is reserved, or one already claimed. Tokens are + * written verbatim into response headers, so the token check also keeps those headers clean. + */ + public static void requireRegistrable(ContentCoding coding, List registered) { + requireNonNull(coding, "coding must not be null"); + Set names = names(coding); + names.forEach(ContentCodings::requireToken); + for (ContentCoding other : registered) { + for (String name : names(other)) { + if (names.contains(name)) { + throw new IllegalStateException("duplicate content coding '" + name + "'"); + } + } + } + } + + private static Set names(ContentCoding coding) { + Set names = new LinkedHashSet<>(); + names.add(coding.token()); + names.addAll(requireNonNull(coding.aliases(), "content coding aliases must not be null")); + return names; + } + + private static void requireToken(String token) { + requireNonNull(token, "content coding token must not be null"); + if (token.isEmpty() || !token.chars().allMatch(ContentCodings::isTokenChar)) { + throw new IllegalArgumentException( + "content coding '" + token + "' is not a lower-case RFC 9110 token"); + } + if (RESERVED.contains(token)) { + throw new IllegalArgumentException("content coding '" + token + "' is reserved"); + } + } + + private static boolean isTokenChar(int c) { + return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || TOKEN_SYMBOLS.indexOf(c) >= 0; + } +} diff --git a/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java index 8627a85b..c2ccfdb8 100644 --- a/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java +++ b/src/main/java/com/retailsvc/http/internal/ContentEncodingHeader.java @@ -1,46 +1,51 @@ package com.retailsvc.http.internal; +import com.retailsvc.http.ContentCoding; import java.util.Locale; +import java.util.Map; -/** Classifies a request {@code Content-Encoding} into the codings the server can decode. */ +/** Classifies a request {@code Content-Encoding} against the codings the server can decode. */ public final class ContentEncodingHeader { private ContentEncodingHeader() {} - /** - * The coding applied to a request body: none (absent or the {@code identity} no-op), a single - * gzip, or one this server cannot decode and the caller renders 415 for. - */ - public enum Coding { - NONE, - GZIP, - UNSUPPORTED + /** How a request body is coded, as far as this server is concerned. */ + public sealed interface RequestCoding { + + /** Absent, blank, or the {@code identity} no-op: the body is already plain. */ + record Identity() implements RequestCoding {} + + /** A single coding this server can decode. */ + record Coded(ContentCoding coding) implements RequestCoding {} + + /** A coding this server cannot decode, or two stacked; the caller renders 415. */ + record Unsupported() implements RequestCoding {} } /** - * Classifies the header value. {@code null}, blank and {@code identity} are all {@link - * Coding#NONE}; a single gzip coding — optionally alongside {@code identity} — is {@link - * Coding#GZIP}. Anything else, including two stacked codings, is {@link Coding#UNSUPPORTED}. + * Classifies the header value against {@code decoders}, keyed by lower-case token. {@code null}, + * blank and {@code identity} are all {@link RequestCoding.Identity}; a single known coding — + * optionally alongside {@code identity} — is {@link RequestCoding.Coded}. An unknown coding, or + * two stacked, is {@link RequestCoding.Unsupported}. */ - public static Coding parse(String header) { + public static RequestCoding parse(String header, Map decoders) { if (header == null) { - return Coding.NONE; + return new RequestCoding.Identity(); } - Coding result = Coding.NONE; + ContentCoding found = null; for (String token : header.split(",")) { - String coding = token.trim().toLowerCase(Locale.ROOT); - if (coding.isEmpty() || "identity".equals(coding)) { + String name = token.trim().toLowerCase(Locale.ROOT); + if (name.isEmpty() || "identity".equals(name)) { continue; } - if (result != Coding.NONE) { - return Coding.UNSUPPORTED; + if (found != null) { + return new RequestCoding.Unsupported(); } - if ("gzip".equals(coding) || "x-gzip".equals(coding)) { - result = Coding.GZIP; - } else { - return Coding.UNSUPPORTED; + found = decoders.get(name); + if (found == null) { + return new RequestCoding.Unsupported(); } } - return result; + return found == null ? new RequestCoding.Identity() : new RequestCoding.Coded(found); } } diff --git a/src/main/java/com/retailsvc/http/internal/GzipCoding.java b/src/main/java/com/retailsvc/http/internal/GzipCoding.java new file mode 100644 index 00000000..1eec3690 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/GzipCoding.java @@ -0,0 +1,36 @@ +package com.retailsvc.http.internal; + +import com.retailsvc.http.ContentCoding; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Set; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; + +/** The built-in {@code gzip} coding, which every server offers. */ +final class GzipCoding implements ContentCoding { + + private static final Set ALIASES = Set.of("x-gzip"); + private static final int BUFFER_SIZE = 8192; + + @Override + public String token() { + return "gzip"; + } + + @Override + public Set aliases() { + return ALIASES; + } + + @Override + public InputStream decode(InputStream coded) throws IOException { + return new GZIPInputStream(coded, BUFFER_SIZE); + } + + @Override + public OutputStream encode(OutputStream sink) throws IOException { + return new GZIPOutputStream(sink); + } +} diff --git a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java index 4efc8861..d3deed97 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java +++ b/src/main/java/com/retailsvc/http/internal/RequestBodyReader.java @@ -5,88 +5,93 @@ import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; import com.retailsvc.http.BadRequestException; +import com.retailsvc.http.ContentCoding; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Coded; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Identity; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Unsupported; import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; -import java.io.EOFException; import java.io.IOException; +import java.io.InputStream; +import java.util.Map; import java.util.function.UnaryOperator; -import java.util.zip.GZIPInputStream; -import java.util.zip.ZipException; /** - * Reads the raw request body, transparently inflating a gzip {@code Content-Encoding} under a hard - * cap on the decompressed size. Immutable and shared across requests. + * Reads the raw request body, transparently decoding a registered {@code Content-Encoding} under a + * hard cap on the decoded size. Immutable and shared across requests. */ public final class RequestBodyReader { - /** Default ceiling on the inflated size of a gzip request body: 10 MiB. */ + /** Default ceiling on the decoded size of a coded request body: 10 MiB. */ public static final long DEFAULT_MAX_DECOMPRESSED_BYTES = 10L * 1024 * 1024; private static final String CONTENT_ENCODING = "Content-Encoding"; private static final String CONTENT_LENGTH = "Content-Length"; - private static final int BUFFER_SIZE = 8192; private final long maxDecompressedBytes; private final int readLimit; + private final Map decoders; - public RequestBodyReader(long maxDecompressedBytes) { + public RequestBodyReader(long maxDecompressedBytes, Map decoders) { if (maxDecompressedBytes <= 0) { throw new IllegalArgumentException( "maxDecompressedBytes must be positive, got " + maxDecompressedBytes); } this.maxDecompressedBytes = maxDecompressedBytes; this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1L) + 1; + this.decoders = Map.copyOf(decoders); } /** * Reads and decodes the request body. * * @throws BadRequestException 415 when the coding is not one this server decodes, 413 when the - * inflated body exceeds the cap, 400 when the gzip stream is malformed or truncated + * decoded body exceeds the cap, 400 when the coded body is malformed or truncated */ public Body read(HttpExchange exchange) throws IOException { Headers headers = exchange.getRequestHeaders(); String header = headers.getFirst(CONTENT_ENCODING); byte[] raw = exchange.getRequestBody().readAllBytes(); - return switch (ContentEncodingHeader.parse(header)) { - case NONE -> new Body(raw, headers::getFirst); - case GZIP -> decoded(inflate(raw), headers); - case UNSUPPORTED -> + return switch (ContentEncodingHeader.parse(header, decoders)) { + case Identity _ -> new Body(raw, headers::getFirst); + case Coded(ContentCoding coding) -> decoded(decode(coding, raw), headers); + case Unsupported _ -> throw new BadRequestException( HTTP_UNSUPPORTED_TYPE, "unsupported Content-Encoding: " + header); }; } /** - * Inflates a complete gzip member. The body is buffered before inflating so that an empty body - * stays an empty body — inflating the exchange stream directly would fail at construction and be - * indistinguishable from a truncated stream. + * Decodes a complete coded body. The body is buffered before this runs, so the coding only ever + * reads memory: any read failure is the body's fault, and an empty body stays empty rather than + * failing the way a stream with no header would. */ - private byte[] inflate(byte[] raw) throws IOException { + private byte[] decode(ContentCoding coding, byte[] raw) { if (raw.length == 0) { return raw; } - try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(raw), BUFFER_SIZE)) { - byte[] inflated = in.readNBytes(readLimit); - if (inflated.length > maxDecompressedBytes) { + try (InputStream in = coding.decode(new ByteArrayInputStream(raw))) { + byte[] decoded = in.readNBytes(readLimit); + if (decoded.length > maxDecompressedBytes) { throw new BadRequestException( HTTP_ENTITY_TOO_LARGE, "decompressed request body exceeds " + maxDecompressedBytes + " bytes"); } - return inflated; - } catch (ZipException | EOFException e) { - throw new BadRequestException(HTTP_BAD_REQUEST, "malformed gzip request body", e); + return decoded; + } catch (IOException e) { + throw new BadRequestException( + HTTP_BAD_REQUEST, "malformed " + coding.token() + " request body", e); } } /** - * Presents the inflated body as if it had arrived uncoded: the coding is gone, so reporting it - * alongside the decoded bytes would misdescribe them, and the stored length measures the - * compressed payload rather than what the handler can read. + * Presents the decoded body as if it had arrived uncoded: the coding is gone, so reporting it + * alongside the decoded bytes would misdescribe them, and the stored length measures the coded + * payload rather than what the handler can read. */ private static Body decoded(byte[] bytes, Headers headers) { - String inflatedLength = Integer.toString(bytes.length); + String decodedLength = Integer.toString(bytes.length); return new Body( bytes, name -> { @@ -94,7 +99,7 @@ private static Body decoded(byte[] bytes, Headers headers) { return null; } if (CONTENT_LENGTH.equalsIgnoreCase(name)) { - return inflatedLength; + return decodedLength; } return headers.getFirst(name); }); diff --git a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java index 52979afa..2b2d018c 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseCompression.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseCompression.java @@ -1,11 +1,12 @@ package com.retailsvc.http.internal; +import com.retailsvc.http.ContentCoding; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.util.Set; -import java.util.zip.GZIPOutputStream; -/** Response content-coding policy, and the gzip primitives the renderer writes through. */ +/** Which responses are worth coding, and coding a whole body at once. */ public final class ResponseCompression { private static final Set COMPRESSIBLE_TYPES = @@ -20,7 +21,7 @@ public final class ResponseCompression { private ResponseCompression() {} /** - * Whether a response of this content type is worth gzipping. Already-compressed payloads gain + * Whether a response of this content type is worth compressing. Already-compressed payloads gain * nothing, and {@code text/event-stream} must stay unbuffered so each event reaches the client as * it is written. * @@ -41,11 +42,11 @@ public static boolean isCompressible(String contentType) { return COMPRESSIBLE_TYPES.contains(mediaType); } - /** Deflates {@code body} into a complete gzip member. */ - public static byte[] gzip(byte[] body) throws IOException { + /** Codes {@code body} completely with {@code coding}. */ + public static byte[] encode(ContentCoding coding, byte[] body) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); - try (GZIPOutputStream gzip = new GZIPOutputStream(out)) { - gzip.write(body); + try (OutputStream coded = coding.encode(out)) { + coded.write(body); } return out.toByteArray(); } diff --git a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java index a941dc35..d75fdc11 100644 --- a/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java +++ b/src/main/java/com/retailsvc/http/internal/ResponseRenderer.java @@ -6,15 +6,16 @@ import static java.net.HttpURLConnection.HTTP_PARTIAL; import static java.net.HttpURLConnection.HTTP_RESET; +import com.retailsvc.http.ContentCoding; import com.retailsvc.http.Response; import com.retailsvc.http.TypeMapper; import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.IOException; import java.io.OutputStream; +import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.zip.GZIPOutputStream; /** Writes a {@link Response} to an {@link HttpExchange}. */ public final class ResponseRenderer { @@ -27,7 +28,6 @@ public final class ResponseRenderer { private static final String CONTENT_LENGTH = "Content-Length"; private static final String VARY = "Vary"; private static final String ACCEPT_ENCODING = "Accept-Encoding"; - private static final String GZIP = "gzip"; private static final long UNKNOWN_LENGTH = -1; private static final long CHUNKED = 0; private static final String DEFAULT_JSON = "application/json"; @@ -35,10 +35,13 @@ public final class ResponseRenderer { private final Map mappers; private final long minCompressibleBytes; + private final List encoders; - public ResponseRenderer(Map mappers, long minCompressibleBytes) { + public ResponseRenderer( + Map mappers, long minCompressibleBytes, List encoders) { this.mappers = Map.copyOf(mappers); this.minCompressibleBytes = minCompressibleBytes; + this.encoders = List.copyOf(encoders); } public void render(HttpExchange exchange, Response response) throws IOException { @@ -68,7 +71,7 @@ private void renderEmpty(HttpExchange exchange, Headers headers, int status, Str throws IOException { defaultContentType(headers, contentType); long declared = declaredLength(headers); - if (shouldCompress(exchange, headers, status, contentType, declared) && declared >= 0) { + if (selectCoding(exchange, headers, status, contentType, declared) != null && declared >= 0) { headers.remove(CONTENT_LENGTH); } exchange.sendResponseHeaders(status, UNKNOWN_LENGTH); @@ -79,15 +82,15 @@ private void renderStream( throws IOException { defaultContentType(headers, contentType); long declared = writer instanceof BodyWriter.Sized sized ? sized.length() : UNKNOWN_LENGTH; - boolean gzip = shouldCompress(exchange, headers, status, contentType, declared); - if (gzip) { - headers.set(CONTENT_ENCODING, GZIP); + ContentCoding coding = selectCoding(exchange, headers, status, contentType, declared); + if (coding != null) { + headers.set(CONTENT_ENCODING, coding.token()); // The coded body goes out chunked, and the JDK leaves a handler-set length in place there. headers.remove(CONTENT_LENGTH); } - exchange.sendResponseHeaders(status, gzip ? CHUNKED : Math.max(declared, CHUNKED)); + exchange.sendResponseHeaders(status, coding != null ? CHUNKED : Math.max(declared, CHUNKED)); try (OutputStream out = - gzip ? new GZIPOutputStream(exchange.getResponseBody()) : exchange.getResponseBody()) { + coding != null ? coding.encode(exchange.getResponseBody()) : exchange.getResponseBody()) { writer.writeTo(out); } } @@ -100,19 +103,24 @@ private static void defaultContentType(Headers headers, String contentType) { } /** - * Whether a body of {@code length} bytes should be gzipped, marking the response as varying by - * {@code Accept-Encoding} whenever it could have been. A negative length means unknown, which - * counts as over the threshold: measuring a stream to find out would defeat streaming it. + * The coding to apply to a body of {@code length} bytes, or {@code null} to send it uncoded, + * marking the response as varying by {@code Accept-Encoding} whenever it could have been coded. A + * negative length means unknown, which counts as over the threshold: measuring a stream to find + * out would defeat streaming it. */ - private boolean shouldCompress( + private ContentCoding selectCoding( HttpExchange exchange, Headers headers, int status, String contentType, long length) { if (headers.containsKey(CONTENT_ENCODING) || !ResponseCompression.isCompressible(contentType) || !bodyAllowed(status)) { - return false; + return null; } addVary(headers); - return (length < 0 || length >= minCompressibleBytes) && acceptsGzip(exchange); + if (length >= 0 && length < minCompressibleBytes) { + return null; + } + String accepted = exchange.getRequestHeaders().getFirst(ACCEPT_ENCODING); + return AcceptEncodingHeader.select(accepted, encoders).orElse(null); } /** The length a handler declared for a body it did not write, or -1 when absent or unreadable. */ @@ -144,19 +152,20 @@ private void renderBytes( } } - /** Gzips the body when it is worth it, leaving a payload gzip fails to shrink uncoded. */ + /** Codes the body when it is worth it, leaving a payload the coding fails to shrink uncoded. */ private byte[] maybeCompress( HttpExchange exchange, Headers headers, int status, String contentType, byte[] bytes) throws IOException { - if (!shouldCompress(exchange, headers, status, contentType, bytes.length)) { + ContentCoding coding = selectCoding(exchange, headers, status, contentType, bytes.length); + if (coding == null) { return bytes; } - byte[] gzipped = ResponseCompression.gzip(bytes); - if (gzipped.length >= bytes.length) { + byte[] coded = ResponseCompression.encode(coding, bytes); + if (coded.length >= bytes.length) { return bytes; } - headers.set(CONTENT_ENCODING, GZIP); - return gzipped; + headers.set(CONTENT_ENCODING, coding.token()); + return coded; } /** Statuses that carry no content cannot carry a content coding either. */ @@ -168,10 +177,6 @@ private static boolean bodyAllowed(int status) { && status != HTTP_NOT_MODIFIED; } - private static boolean acceptsGzip(HttpExchange exchange) { - return AcceptEncodingHeader.acceptsGzip(exchange.getRequestHeaders().getFirst(ACCEPT_ENCODING)); - } - /** * Marks the response as varying by {@code Accept-Encoding} so shared caches keep the coded and * uncoded forms apart. Announced whenever the body could have been coded, not only when it was, diff --git a/src/test/java/com/retailsvc/http/ContentCodingIT.java b/src/test/java/com/retailsvc/http/ContentCodingIT.java new file mode 100644 index 00000000..ecf5d48a --- /dev/null +++ b/src/test/java/com/retailsvc/http/ContentCodingIT.java @@ -0,0 +1,229 @@ +package com.retailsvc.http; + +import static com.retailsvc.http.support.TestCodings.deflate; +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; +import static java.net.HttpURLConnection.HTTP_OK; +import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import com.retailsvc.http.start.TextEchoHandler; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.util.Map; +import java.util.Optional; +import java.util.function.UnaryOperator; +import java.util.zip.DeflaterOutputStream; +import java.util.zip.InflaterInputStream; +import org.junit.jupiter.api.Test; + +/** + * End-to-end coverage of a caller-registered content coding. The coding is {@code deflate} from + * java.util.zip, so the extension point is proved without any compression dependency. + */ +class ContentCodingIT extends ServerBaseTest { + + private static final String CONTENT_ENCODING = "Content-Encoding"; + private static final String ACCEPT_ENCODING = "Accept-Encoding"; + private static final String PAYLOAD = "compress me please ".repeat(200); + + // -- requests -- + + @Test + void deflatedRequestBodyIsDecoded() throws Exception { + try (var s = echoServer(b -> b.contentCoding(deflate())); + var client = httpClient()) { + var request = + textEcho(s, deflated("hello deflate".getBytes(UTF_8))) + .header(CONTENT_ENCODING, "deflate") + .build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.body()).isEqualTo("hello deflate"); + } + } + + @Test + void deflateBombIsStoppedByTheServersCap() throws Exception { + try (var s = echoServer(b -> b.contentCoding(deflate()).maxDecompressedRequestBytes(1024)); + var client = httpClient()) { + var request = + textEcho(s, deflated(new byte[8192])).header(CONTENT_ENCODING, "deflate").build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_ENTITY_TOO_LARGE); + } + } + + @Test + void malformedDeflateBodyReturns400() throws Exception { + try (var s = echoServer(b -> b.contentCoding(deflate())); + var client = httpClient()) { + var request = + textEcho(s, "not deflate at all".getBytes(UTF_8)) + .header(CONTENT_ENCODING, "deflate") + .build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(response.body()).contains("malformed deflate request body"); + } + } + + // -- responses -- + + @Test + void responseIsCodedWithTheRegisteredCoding() throws Exception { + try (var s = echoServer(b -> b.contentCoding(deflate())); + var client = httpClient()) { + var request = textEcho(s, PAYLOAD.getBytes(UTF_8)).header(ACCEPT_ENCODING, "deflate").build(); + + HttpResponse response = client.send(request, BodyHandlers.ofByteArray()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).contains("deflate"); + assertThat(response.headers().firstValue("Vary")) + .hasValueSatisfying(vary -> assertThat(vary).contains(ACCEPT_ENCODING)); + assertThat(new String(inflate(response.body()), UTF_8)).isEqualTo(PAYLOAD); + } + } + + @Test + void registeredCodingWinsATieWithGzip() throws Exception { + assertThat(codingChosenFor("gzip, deflate")).contains("deflate"); + } + + @Test + void clientWeightStillDecides() throws Exception { + assertThat(codingChosenFor("deflate;q=0.1, gzip;q=0.9")).contains("gzip"); + } + + @Test + void streamedResponseUsesTheRegisteredCoding() throws Exception { + try (var s = + echoServer( + b -> + b.contentCoding(deflate()) + .extraRoute("/openapi.yaml", Handlers.resourceHandler("/openapi.yaml"))); + var client = httpClient()) { + var request = + HttpRequest.newBuilder() + .uri(URI.create("http://localhost:%d/openapi.yaml".formatted(s.listenPort()))) + .header(ACCEPT_ENCODING, "deflate") + .GET() + .build(); + + HttpResponse response = client.send(request, BodyHandlers.ofByteArray()); + + assertThat(response.headers().firstValue(CONTENT_ENCODING)).contains("deflate"); + assertThat(response.headers().firstValue("Content-Length")).isEmpty(); + assertThat(inflate(response.body())).isEqualTo(classpathBytes()); + } + } + + // -- one direction only -- + + @Test + void requestOnlyCodingDecodesButNeverCodesAResponse() throws Exception { + try (var s = echoServer(b -> b.requestContentCoding(deflate())); + var client = httpClient()) { + var request = + textEcho(s, deflated(PAYLOAD.getBytes(UTF_8))) + .header(CONTENT_ENCODING, "deflate") + .header(ACCEPT_ENCODING, "deflate") + .build(); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(HTTP_OK); + assertThat(response.headers().firstValue(CONTENT_ENCODING)).isEmpty(); + assertThat(response.body()).isEqualTo(PAYLOAD); + } + } + + @Test + void responseOnlyCodingCodesButIsRefusedOnRequests() throws Exception { + try (var s = echoServer(b -> b.responseContentCoding(deflate())); + var client = httpClient()) { + var coded = textEcho(s, PAYLOAD.getBytes(UTF_8)).header(ACCEPT_ENCODING, "deflate").build(); + var refused = + textEcho(s, deflated(PAYLOAD.getBytes(UTF_8))) + .header(CONTENT_ENCODING, "deflate") + .build(); + + var codedResponse = client.send(coded, BodyHandlers.ofByteArray()); + var refusedResponse = client.send(refused, BodyHandlers.ofString()); + + assertThat(codedResponse.headers().firstValue(CONTENT_ENCODING)).contains("deflate"); + assertThat(refusedResponse.statusCode()).isEqualTo(HTTP_UNSUPPORTED_TYPE); + } + } + + // -- fixtures -- + + private Optional codingChosenFor(String acceptEncoding) throws Exception { + try (var s = echoServer(b -> b.contentCoding(deflate())); + var client = httpClient()) { + var request = + textEcho(s, PAYLOAD.getBytes(UTF_8)).header(ACCEPT_ENCODING, acceptEncoding).build(); + return client + .send(request, BodyHandlers.ofByteArray()) + .headers() + .firstValue(CONTENT_ENCODING); + } + } + + private OpenApiServer echoServer(UnaryOperator customise) { + try { + server = + customise + .apply( + newBuilder() + .spec(spec) + .handlers(stubAllHandlers(Map.of("text-echo", new TextEchoHandler()))) + .port(0)) + .build(); + return server; + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private HttpRequest.Builder textEcho(OpenApiServer server, byte[] body) { + return HttpRequest.newBuilder() + .uri(URI.create("http://localhost:%d/api/v1/text-echo".formatted(server.listenPort()))) + .header("Content-Type", "text/plain") + .POST(BodyPublishers.ofByteArray(body)); + } + + private static byte[] classpathBytes() throws IOException { + try (InputStream in = ContentCodingIT.class.getResourceAsStream("/openapi.yaml")) { + return in.readAllBytes(); + } + } + + private static byte[] deflated(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (DeflaterOutputStream deflater = new DeflaterOutputStream(out)) { + deflater.write(data); + } + return out.toByteArray(); + } + + private static byte[] inflate(byte[] data) throws IOException { + try (InflaterInputStream in = new InflaterInputStream(new ByteArrayInputStream(data))) { + return in.readAllBytes(); + } + } +} diff --git a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java index 59723819..ce755e60 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerBuilderTest.java @@ -1,5 +1,7 @@ package com.retailsvc.http; +import static com.retailsvc.http.support.TestCodings.deflate; +import static com.retailsvc.http.support.TestCodings.named; import static java.util.Collections.emptyMap; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -86,6 +88,42 @@ void acceptsContentCodingLimits() { assertThat(b.maxDecompressedRequestBytes(4096).minCompressibleResponseBytes(0)).isSameAs(b); } + @Test + void contentCodingRejectsTheBuiltInGzip() { + OpenApiServer.Builder b = OpenApiServer.builder(); + ContentCoding gzip = named("gzip"); + + assertThatThrownBy(() -> b.contentCoding(gzip)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("gzip"); + } + + @Test + void contentCodingRejectsATokenRegisteredTwice() { + OpenApiServer.Builder b = OpenApiServer.builder().contentCoding(deflate()); + ContentCoding again = deflate(); + + assertThatThrownBy(() -> b.contentCoding(again)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("deflate"); + } + + @Test + void oneCodingMayBeRegisteredOncePerDirection() { + ContentCoding deflate = deflate(); + OpenApiServer.Builder b = OpenApiServer.builder(); + + assertThat(b.requestContentCoding(deflate).responseContentCoding(deflate)).isSameAs(b); + } + + @Test + void contentCodingConflictsWithAnEarlierOneWayRegistration() { + OpenApiServer.Builder b = OpenApiServer.builder().responseContentCoding(deflate()); + ContentCoding bothWays = deflate(); + + assertThatThrownBy(() -> b.contentCoding(bothWays)).isInstanceOf(IllegalStateException.class); + } + @Test void rejectsNegativeShutdownTimeout() { OpenApiServer.Builder b = OpenApiServer.builder(); diff --git a/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java index f336d95d..131652eb 100644 --- a/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java +++ b/src/test/java/com/retailsvc/http/internal/AcceptEncodingHeaderTest.java @@ -1,116 +1,165 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.support.TestCodings.named; import static org.assertj.core.api.Assertions.assertThat; +import com.retailsvc.http.ContentCoding; +import java.util.List; import org.junit.jupiter.api.Test; class AcceptEncodingHeaderTest { + private static final ContentCoding GZIP = new GzipCoding(); + private static final ContentCoding ZSTD = named("zstd"); + private static final List ZSTD_THEN_GZIP = List.of(ZSTD, GZIP); + @Test void nullHeaderIsNotAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip(null)).isFalse(); + assertThat(accepts(null)).isFalse(); } @Test void blankHeaderIsNotAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip(" ")).isFalse(); + assertThat(accepts(" ")).isFalse(); } @Test void plainGzipIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip")).isTrue(); + assertThat(accepts("gzip")).isTrue(); } @Test void gzipAmongOtherCodingsIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("br, deflate, gzip")).isTrue(); + assertThat(accepts("br, deflate, gzip")).isTrue(); } @Test void caseInsensitiveGzipIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("GZip")).isTrue(); + assertThat(accepts("GZip")).isTrue(); } @Test void xGzipIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("x-gzip")).isTrue(); + assertThat(accepts("x-gzip")).isTrue(); } @Test void explicitZeroQValueIsRefused() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0")).isFalse(); - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0.0")).isFalse(); + assertThat(accepts("gzip;q=0")).isFalse(); + assertThat(accepts("gzip;q=0.0")).isFalse(); } @Test void positiveQValueIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0.5")).isTrue(); + assertThat(accepts("gzip;q=0.5")).isTrue(); } @Test void wildcardIsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("*")).isTrue(); + assertThat(accepts("*")).isTrue(); } @Test void wildcardWithZeroQValueIsRefused() { - assertThat(AcceptEncodingHeader.acceptsGzip("*;q=0")).isFalse(); + assertThat(accepts("*;q=0")).isFalse(); } @Test void explicitGzipBeatsWildcardRefusal() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip, *;q=0")).isTrue(); + assertThat(accepts("gzip, *;q=0")).isTrue(); } @Test void explicitGzipRefusalBeatsWildcard() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0, *")).isFalse(); + assertThat(accepts("gzip;q=0, *")).isFalse(); } @Test void identityOnlyIsNotAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("identity")).isFalse(); + assertThat(accepts("identity")).isFalse(); } @Test void deflateOnlyIsNotAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("deflate, br")).isFalse(); + assertThat(accepts("deflate, br")).isFalse(); } @Test void surroundingWhitespaceIsTolerated() { - assertThat(AcceptEncodingHeader.acceptsGzip(" deflate , gzip ; q=0.8 ")).isTrue(); + assertThat(accepts(" deflate , gzip ; q=0.8 ")).isTrue(); } @Test void malformedQValueIsTreatedAsAccepted() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=bogus")).isTrue(); + assertThat(accepts("gzip;q=bogus")).isTrue(); } @Test void emptyTokensAreIgnored() { - assertThat(AcceptEncodingHeader.acceptsGzip("deflate,,gzip")).isTrue(); + assertThat(accepts("deflate,,gzip")).isTrue(); } @Test void repeatedGzipTokensTakeThePositiveWeight() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q=0, gzip")).isTrue(); - assertThat(AcceptEncodingHeader.acceptsGzip("gzip, x-gzip;q=0")).isTrue(); + assertThat(accepts("gzip;q=0, gzip")).isTrue(); + assertThat(accepts("gzip, x-gzip;q=0")).isTrue(); } @Test void repeatedWildcardsTakeThePositiveWeight() { - assertThat(AcceptEncodingHeader.acceptsGzip("*;q=0, *")).isTrue(); + assertThat(accepts("*;q=0, *")).isTrue(); } @Test void parametersOtherThanWeightAreIgnored() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;level=9")).isTrue(); - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;level=9;q=0")).isFalse(); + assertThat(accepts("gzip;level=9")).isTrue(); + assertThat(accepts("gzip;level=9;q=0")).isFalse(); } @Test void valuelessParameterIsIgnored() { - assertThat(AcceptEncodingHeader.acceptsGzip("gzip;q")).isTrue(); + assertThat(accepts("gzip;q")).isTrue(); + } + + // -- choosing among several codings -- + + @Test + void clientWeightOutranksServerPreference() { + assertThat(AcceptEncodingHeader.select("gzip;q=1.0, zstd;q=0.5", ZSTD_THEN_GZIP)) + .contains(GZIP); + } + + @Test + void equalWeightsGoToTheServersFirstChoice() { + assertThat(AcceptEncodingHeader.select("gzip, zstd", ZSTD_THEN_GZIP)).contains(ZSTD); + } + + @Test + void wildcardGoesToTheServersFirstChoice() { + assertThat(AcceptEncodingHeader.select("*", ZSTD_THEN_GZIP)).contains(ZSTD); + } + + @Test + void refusingTheFirstChoiceFallsBackToTheNext() { + assertThat(AcceptEncodingHeader.select("zstd;q=0, gzip", ZSTD_THEN_GZIP)).contains(GZIP); + } + + @Test + void aliasOfALaterChoiceIsHonoured() { + assertThat(AcceptEncodingHeader.select("x-gzip", ZSTD_THEN_GZIP)).contains(GZIP); + } + + @Test + void noSupportedCodingSelectsNothing() { + assertThat(AcceptEncodingHeader.select("br, deflate", ZSTD_THEN_GZIP)).isEmpty(); + } + + @Test + void noCandidatesSelectsNothing() { + assertThat(AcceptEncodingHeader.select("gzip", List.of())).isEmpty(); + } + + private static boolean accepts(String header) { + return AcceptEncodingHeader.select(header, List.of(GZIP)).isPresent(); } } diff --git a/src/test/java/com/retailsvc/http/internal/ContentCodingsTest.java b/src/test/java/com/retailsvc/http/internal/ContentCodingsTest.java new file mode 100644 index 00000000..7f7eceb2 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/ContentCodingsTest.java @@ -0,0 +1,130 @@ +package com.retailsvc.http.internal; + +import static com.retailsvc.http.support.TestCodings.named; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.ContentCoding; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +class ContentCodingsTest { + + @Test + void gzipIsTheOnlyCodingWhenNoneAreRegistered() { + ContentCodings codings = ContentCodings.of(List.of(), List.of()); + + assertThat(codings.encoders()).extracting(ContentCoding::token).containsExactly("gzip"); + assertThat(codings.decoders()).containsOnlyKeys("gzip", "x-gzip"); + } + + @Test + void registeredEncodersArePreferredOverGzipInRegistrationOrder() { + ContentCodings codings = ContentCodings.of(List.of(), List.of(named("zstd"), named("br"))); + + assertThat(codings.encoders()) + .extracting(ContentCoding::token) + .containsExactly("zstd", "br", "gzip"); + } + + @Test + void decodersAreKeyedByTokenAndAlias() { + ContentCoding deflate = named("deflate", "x-deflate"); + + ContentCodings codings = ContentCodings.of(List.of(deflate), List.of()); + + assertThat(codings.decoders()) + .containsEntry("deflate", deflate) + .containsEntry("x-deflate", deflate); + } + + @Test + void requestAndResponseRegistrationsAreIndependent() { + ContentCodings codings = ContentCodings.of(List.of(named("br")), List.of(named("zstd"))); + + assertThat(codings.decoders()).containsKey("br").doesNotContainKey("zstd"); + assertThat(codings.encoders()).extracting(ContentCoding::token).containsExactly("zstd", "gzip"); + } + + @ParameterizedTest + @ValueSource(strings = {"zstd", "br", "x-custom.v2", "a1"}) + void acceptsLowerCaseRfc9110Tokens(String token) { + ContentCoding coding = named(token); + + assertThatCode(() -> ContentCodings.requireRegistrable(coding, List.of())) + .doesNotThrowAnyException(); + } + + @ParameterizedTest + @ValueSource( + strings = { + "", + "zs td", + "zs,td", + "zs;td", + "zstd\r\nX-Injected: yes", + "ZSTD", + "zs\"td", + "zs/td" + }) + void rejectsTokensThatAreNotLowerCaseRfc9110Tokens(String token) { + ContentCoding coding = named(token); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(coding, List.of())) + .isInstanceOf(IllegalArgumentException.class); + } + + @ParameterizedTest + @ValueSource(strings = {"gzip", "x-gzip", "identity", "*"}) + void rejectsReservedTokens(String token) { + ContentCoding coding = named(token); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(coding, List.of())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(token); + } + + @Test + void rejectsReservedAlias() { + ContentCoding aliasedToGzip = named("fastgzip", "gzip"); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(aliasedToGzip, List.of())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void rejectsNullCoding() { + assertThatThrownBy(() -> ContentCodings.requireRegistrable(null, List.of())) + .isInstanceOf(NullPointerException.class); + } + + @Test + void rejectsNullToken() { + ContentCoding nameless = named(null); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(nameless, List.of())) + .isInstanceOf(NullPointerException.class); + } + + @Test + void rejectsATokenAlreadyRegistered() { + List registered = List.of(named("zstd")); + ContentCoding duplicate = named("zstd"); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(duplicate, registered)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("zstd"); + } + + @Test + void rejectsAnAliasThatCollidesWithARegisteredToken() { + List registered = List.of(named("br")); + ContentCoding brotli = named("brotli", "br"); + + assertThatThrownBy(() -> ContentCodings.requireRegistrable(brotli, registered)) + .isInstanceOf(IllegalStateException.class); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java b/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java index 67019ac8..caac40dd 100644 --- a/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java +++ b/src/test/java/com/retailsvc/http/internal/ContentEncodingHeaderTest.java @@ -1,67 +1,101 @@ package com.retailsvc.http.internal; -import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.GZIP; -import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.NONE; -import static com.retailsvc.http.internal.ContentEncodingHeader.Coding.UNSUPPORTED; +import static com.retailsvc.http.support.TestCodings.named; import static org.assertj.core.api.Assertions.assertThat; +import com.retailsvc.http.ContentCoding; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Coded; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Identity; +import com.retailsvc.http.internal.ContentEncodingHeader.RequestCoding.Unsupported; +import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; class ContentEncodingHeaderTest { + private static final Map GZIP_ONLY = + ContentCodings.of(List.of(), List.of()).decoders(); + private static final RequestCoding GZIPPED = new Coded(GZIP_ONLY.get("gzip")); + private static final RequestCoding IDENTITY = new Identity(); + private static final RequestCoding UNSUPPORTED = new Unsupported(); + @Test - void nullHeaderIsNone() { - assertThat(ContentEncodingHeader.parse(null)).isEqualTo(NONE); + void nullHeaderIsIdentity() { + assertThat(parse(null)).isEqualTo(IDENTITY); } @Test - void emptyHeaderIsNone() { - assertThat(ContentEncodingHeader.parse(" ")).isEqualTo(NONE); + void emptyHeaderIsIdentity() { + assertThat(parse(" ")).isEqualTo(IDENTITY); } @Test - void identityIsNone() { - assertThat(ContentEncodingHeader.parse("identity")).isEqualTo(NONE); + void identityIsNotACoding() { + assertThat(parse("identity")).isEqualTo(IDENTITY); } @Test void gzipIsGzip() { - assertThat(ContentEncodingHeader.parse("gzip")).isEqualTo(GZIP); + assertThat(parse("gzip")).isEqualTo(GZIPPED); } @Test void xGzipIsGzip() { - assertThat(ContentEncodingHeader.parse("x-gzip")).isEqualTo(GZIP); + assertThat(parse("x-gzip")).isEqualTo(GZIPPED); } @Test void mixedCaseGzipIsGzip() { - assertThat(ContentEncodingHeader.parse("GZip")).isEqualTo(GZIP); + assertThat(parse("GZip")).isEqualTo(GZIPPED); } @Test void gzipWithIdentityIsGzip() { - assertThat(ContentEncodingHeader.parse("identity, gzip")).isEqualTo(GZIP); + assertThat(parse("identity, gzip")).isEqualTo(GZIPPED); } @Test void surroundingWhitespaceIsTolerated() { - assertThat(ContentEncodingHeader.parse(" gzip ")).isEqualTo(GZIP); + assertThat(parse(" gzip ")).isEqualTo(GZIPPED); } @Test void brotliIsUnsupported() { - assertThat(ContentEncodingHeader.parse("br")).isEqualTo(UNSUPPORTED); + assertThat(parse("br")).isEqualTo(UNSUPPORTED); } @Test void deflateIsUnsupported() { - assertThat(ContentEncodingHeader.parse("deflate")).isEqualTo(UNSUPPORTED); + assertThat(parse("deflate")).isEqualTo(UNSUPPORTED); } @Test void stackedCodingsAreUnsupported() { - assertThat(ContentEncodingHeader.parse("gzip, gzip")).isEqualTo(UNSUPPORTED); - assertThat(ContentEncodingHeader.parse("gzip, br")).isEqualTo(UNSUPPORTED); + assertThat(parse("gzip, gzip")).isEqualTo(UNSUPPORTED); + assertThat(parse("gzip, br")).isEqualTo(UNSUPPORTED); + } + + @Test + void registeredCodingIsRecognised() { + ContentCoding zstd = named("zstd"); + + RequestCoding coding = ContentEncodingHeader.parse("zstd", decoders(zstd)); + + assertThat(coding).isEqualTo(new Coded(zstd)); + } + + @Test + void stackingARegisteredCodingIsStillUnsupported() { + assertThat(ContentEncodingHeader.parse("zstd, gzip", decoders(named("zstd")))) + .isEqualTo(UNSUPPORTED); + } + + private static RequestCoding parse(String header) { + return ContentEncodingHeader.parse(header, GZIP_ONLY); + } + + private static Map decoders(ContentCoding coding) { + return ContentCodings.of(List.of(coding), List.of()).decoders(); } } diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java index 3ffdea6d..9624c5f0 100644 --- a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java +++ b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java @@ -49,7 +49,10 @@ private static DispatchHandler dispatcher(Map handlers) handlers, List.of(), List.of(), - new ResponseRenderer(Map.of(), DEFAULT_MIN_COMPRESSIBLE_BYTES)); + new ResponseRenderer( + Map.of(), + DEFAULT_MIN_COMPRESSIBLE_BYTES, + ContentCodings.of(List.of(), List.of()).encoders())); } private static DispatchHandler dispatcher( @@ -60,7 +63,10 @@ private static DispatchHandler dispatcher( handlers, interceptors, decorators, - new ResponseRenderer(Map.of(), DEFAULT_MIN_COMPRESSIBLE_BYTES)); + new ResponseRenderer( + Map.of(), + DEFAULT_MIN_COMPRESSIBLE_BYTES, + ContentCodings.of(List.of(), List.of()).encoders())); } private static void withRequest(String operationId, ScopedValue.CallableOp body) diff --git a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java index ca7ed15e..29c89618 100644 --- a/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java +++ b/src/test/java/com/retailsvc/http/internal/ExtrasRouterTest.java @@ -20,6 +20,7 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import java.util.zip.GZIPOutputStream; @@ -146,8 +147,13 @@ private static ExtrasRouter newRouter(Map extras) { Map mappers = Map.of("application/json", new GsonTypeMapper()); return new ExtrasRouter( extras, - new ResponseRenderer(mappers, DEFAULT_MIN_COMPRESSIBLE_BYTES), - new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); + new ResponseRenderer( + mappers, + DEFAULT_MIN_COMPRESSIBLE_BYTES, + ContentCodings.of(List.of(), List.of()).encoders()), + new RequestBodyReader( + RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES, + ContentCodings.of(List.of(), List.of()).decoders())); } private static void invoke(ExtrasRouter router, String path) throws Exception { diff --git a/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java index 4fa093b2..2cb1c4a1 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.support.TestCodings.deflate; import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_ENTITY_TOO_LARGE; import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE; @@ -10,12 +11,18 @@ import static org.mockito.Mockito.when; import com.retailsvc.http.BadRequestException; +import com.retailsvc.http.ContentCoding; import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.zip.DeflaterOutputStream; import java.util.zip.GZIPOutputStream; import java.util.zip.ZipException; import org.junit.jupiter.api.Test; @@ -23,14 +30,18 @@ class RequestBodyReaderTest { private static final long CAP = 1024; - private final RequestBodyReader reader = new RequestBodyReader(CAP); + private static final Map GZIP_ONLY = + ContentCodings.of(List.of(), List.of()).decoders(); + private final RequestBodyReader reader = new RequestBodyReader(CAP, GZIP_ONLY); + private final RequestBodyReader withDeflate = + new RequestBodyReader(CAP, ContentCodings.of(List.of(deflate()), List.of()).decoders()); @Test void constructorRejectsNonPositiveCap() { - assertThatThrownBy(() -> new RequestBodyReader(0)) + assertThatThrownBy(() -> new RequestBodyReader(0, GZIP_ONLY)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("maxDecompressedBytes"); - assertThatThrownBy(() -> new RequestBodyReader(-1)) + assertThatThrownBy(() -> new RequestBodyReader(-1, GZIP_ONLY)) .isInstanceOf(IllegalArgumentException.class); } @@ -70,17 +81,19 @@ void emptyGzipBodyIsReturnedEmpty() throws IOException { @Test void unsupportedCodingThrows415() { - assertThatThrownBy(() -> reader.read(exchange("x".getBytes(UTF_8), "br"))) + HttpExchange brotli = exchange("x".getBytes(UTF_8), "br"); + + assertThatThrownBy(() -> reader.read(brotli)) .isInstanceOfSatisfying( BadRequestException.class, e -> assertThat(e.status()).isEqualTo(HTTP_UNSUPPORTED_TYPE)); } @Test - void oversizedInflatedBodyThrows413() { - byte[] bomb = new byte[(int) CAP * 4]; + void oversizedInflatedBodyThrows413() throws IOException { + HttpExchange bomb = exchange(gzip(new byte[(int) CAP * 4]), "gzip"); - assertThatThrownBy(() -> reader.read(exchange(gzip(bomb), "gzip"))) + assertThatThrownBy(() -> reader.read(bomb)) .isInstanceOfSatisfying( BadRequestException.class, e -> assertThat(e.status()).isEqualTo(HTTP_ENTITY_TOO_LARGE)); @@ -97,9 +110,9 @@ void bodyExactlyAtCapIsAccepted() throws IOException { @Test void malformedGzipThrows400WithCause() { - byte[] garbage = "not gzip at all".getBytes(UTF_8); + HttpExchange garbage = exchange("not gzip at all".getBytes(UTF_8), "gzip"); - assertThatThrownBy(() -> reader.read(exchange(garbage, "gzip"))) + assertThatThrownBy(() -> reader.read(garbage)) .isInstanceOfSatisfying( BadRequestException.class, e -> { @@ -111,9 +124,9 @@ void malformedGzipThrows400WithCause() { @Test void truncatedGzipThrows400() throws IOException { byte[] complete = gzip("some reasonably long payload to truncate".getBytes(UTF_8)); - byte[] truncated = Arrays.copyOf(complete, complete.length - 6); + HttpExchange truncated = exchange(Arrays.copyOf(complete, complete.length - 6), "gzip"); - assertThatThrownBy(() -> reader.read(exchange(truncated, "gzip"))) + assertThatThrownBy(() -> reader.read(truncated)) .isInstanceOfSatisfying( BadRequestException.class, e -> assertThat(e.status()).isEqualTo(HTTP_BAD_REQUEST)); } @@ -150,6 +163,68 @@ void plainBodyKeepsOriginalHeaderLookup() throws IOException { assertThat(body.headerLookup().apply("X-Custom")).isEqualTo("value"); } + // -- registered codings -- + + @Test + void registeredCodingIsDecoded() throws IOException { + byte[] plain = "hello deflate".getBytes(UTF_8); + + RequestBodyReader.Body body = withDeflate.read(exchange(deflated(plain), "deflate")); + + assertThat(body.bytes()).isEqualTo(plain); + } + + @Test + void capAppliesToARegisteredCoding() throws IOException { + HttpExchange bomb = exchange(deflated(new byte[(int) CAP * 4]), "deflate"); + + assertThatThrownBy(() -> withDeflate.read(bomb)) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> assertThat(e.status()).isEqualTo(HTTP_ENTITY_TOO_LARGE)); + } + + @Test + void malformedBodyNamesTheRegisteredCoding() { + HttpExchange garbage = exchange("not deflate at all".getBytes(UTF_8), "deflate"); + + assertThatThrownBy(() -> withDeflate.read(garbage)) + .isInstanceOfSatisfying( + BadRequestException.class, + e -> { + assertThat(e.status()).isEqualTo(HTTP_BAD_REQUEST); + assertThat(e.getMessage()).contains("deflate"); + }); + } + + @Test + void faultInACodingIsNotReportedAsAClientError() { + ContentCoding broken = + new ContentCoding() { + @Override + public String token() { + return "broken"; + } + + @Override + public InputStream decode(InputStream coded) { + throw new IllegalStateException("bug in the coding"); + } + + @Override + public OutputStream encode(OutputStream sink) { + return sink; + } + }; + RequestBodyReader faulty = + new RequestBodyReader(CAP, ContentCodings.of(List.of(broken), List.of()).decoders()); + HttpExchange request = exchange("x".getBytes(UTF_8), "broken"); + + assertThatThrownBy(() -> faulty.read(request)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("bug in the coding"); + } + private static HttpExchange exchange(byte[] body, String contentEncoding) { Headers headers = new Headers(); if (contentEncoding != null) { @@ -170,4 +245,12 @@ private static byte[] gzip(byte[] data) throws IOException { } return out.toByteArray(); } + + private static byte[] deflated(byte[] data) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (DeflaterOutputStream deflater = new DeflaterOutputStream(out)) { + deflater.write(data); + } + return out.toByteArray(); + } } diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java index 14025ad6..cd64f789 100644 --- a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java +++ b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java @@ -118,9 +118,14 @@ public byte[] writeTo(Object value) { new DefaultValidator(spec::resolveSchema), mappers, rethrow, - new ResponseRenderer(mappers, DEFAULT_MIN_COMPRESSIBLE_BYTES), + new ResponseRenderer( + mappers, + DEFAULT_MIN_COMPRESSIBLE_BYTES, + ContentCodings.of(List.of(), List.of()).encoders()), List.of(), - new RequestBodyReader(RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES)); + new RequestBodyReader( + RequestBodyReader.DEFAULT_MAX_DECOMPRESSED_BYTES, + ContentCodings.of(List.of(), List.of()).decoders())); } @Test diff --git a/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java index 8a27666c..93d476cf 100644 --- a/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java +++ b/src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java @@ -66,7 +66,7 @@ void matchIsCaseInsensitive() { void gzipRoundTripsBytes() throws IOException { byte[] plain = "round trip me".repeat(20).getBytes(UTF_8); - byte[] compressed = ResponseCompression.gzip(plain); + byte[] compressed = ResponseCompression.encode(new GzipCoding(), plain); assertThat(compressed).isNotEqualTo(plain); assertThat(gunzip(compressed)).isEqualTo(plain); diff --git a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java index cff6f4a3..ad8d24e8 100644 --- a/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java +++ b/src/test/java/com/retailsvc/http/internal/ResponseRendererTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.internal; +import static com.retailsvc.http.support.TestCodings.deflate; import static java.net.HttpURLConnection.HTTP_NOT_MODIFIED; import static java.net.HttpURLConnection.HTTP_NO_CONTENT; import static java.net.HttpURLConnection.HTTP_OK; @@ -11,6 +12,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.retailsvc.http.ContentCoding; import com.retailsvc.http.GsonTypeMapper; import com.retailsvc.http.Response; import com.retailsvc.http.TypeMapper; @@ -18,12 +20,17 @@ import com.sun.net.httpserver.HttpExchange; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.FilterOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.List; import java.util.Map; import java.util.Random; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.zip.GZIPInputStream; +import java.util.zip.InflaterInputStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -32,12 +39,14 @@ class ResponseRendererTest { private static final Map MAPPERS = Map.of("application/json", new GsonTypeMapper()); private static final long THRESHOLD = 1024; + private static final List GZIP_ONLY = + ContentCodings.of(List.of(), List.of()).encoders(); private static final String JSON = "application/json"; private static final String TEXT = "text/plain"; private static final String CONTENT_ENCODING = "Content-Encoding"; private static final String VARY = "Vary"; - private final ResponseRenderer renderer = new ResponseRenderer(MAPPERS, THRESHOLD); + private final ResponseRenderer renderer = new ResponseRenderer(MAPPERS, THRESHOLD, GZIP_ONLY); private final Headers requestHeaders = new Headers(); private final Headers responseHeaders = new Headers(); private final ByteArrayOutputStream sink = new ByteArrayOutputStream(); @@ -412,6 +421,100 @@ void stripsHandlerContentLengthWhenStreamIsCompressed() throws IOException { assertThat(length.get()).isZero(); } + // -- registered codings -- + + @Test + void registeredCodingCodesTheBodyAndNamesItself() throws IOException { + requestHeaders.add("Accept-Encoding", "deflate"); + byte[] body = largeText(); + + withDeflate().render(exchange, Response.bytes(HTTP_OK, body, JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("deflate"); + assertThat(inflate(sink.toByteArray())).isEqualTo(body); + } + + @Test + void registeredCodingCodesAStream() throws IOException { + requestHeaders.add("Accept-Encoding", "deflate"); + byte[] payload = largeText(); + + withDeflate().render(exchange, Response.stream(HTTP_OK, TEXT, out -> out.write(payload))); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("deflate"); + assertThat(length.get()).isZero(); + assertThat(inflate(sink.toByteArray())).isEqualTo(payload); + } + + @Test + void registeredCodingWinsOverGzipWhenWeightedEqually() throws IOException { + requestHeaders.add("Accept-Encoding", "gzip, deflate"); + + withDeflate().render(exchange, Response.bytes(HTTP_OK, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("deflate"); + } + + @Test + void gzipStillServesClientsThatOnlyTakeGzip() throws IOException { + requestHeaders.add("Accept-Encoding", "gzip"); + + withDeflate().render(exchange, Response.bytes(HTTP_OK, largeText(), JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isEqualTo("gzip"); + } + + @Test + void codingThatDoesNotShrinkTheBodyFallsBackToIdentity() throws IOException { + requestHeaders.add("Accept-Encoding", "bloat"); + byte[] body = largeText(); + ResponseRenderer bloating = + new ResponseRenderer( + MAPPERS, THRESHOLD, ContentCodings.of(List.of(), List.of(bloat())).encoders()); + + bloating.render(exchange, Response.bytes(HTTP_OK, body, JSON)); + + assertThat(responseHeaders.getFirst(CONTENT_ENCODING)).isNull(); + assertThat(sink.toByteArray()).isEqualTo(body); + } + + private static ResponseRenderer withDeflate() { + return new ResponseRenderer( + MAPPERS, THRESHOLD, ContentCodings.of(List.of(), List.of(deflate())).encoders()); + } + + /** A coding that doubles every byte, so it can never shrink a body. */ + private static ContentCoding bloat() { + return new ContentCoding() { + @Override + public String token() { + return "bloat"; + } + + @Override + public InputStream decode(InputStream coded) { + return coded; + } + + @Override + public OutputStream encode(OutputStream sink) { + return new FilterOutputStream(sink) { + @Override + public void write(int b) throws IOException { + out.write(b); + out.write(b); + } + }; + } + }; + } + + private static byte[] inflate(byte[] data) throws IOException { + try (InflaterInputStream in = new InflaterInputStream(new ByteArrayInputStream(data))) { + return in.readAllBytes(); + } + } + private void acceptsGzip() { requestHeaders.add("Accept-Encoding", "gzip, deflate, br"); } diff --git a/src/test/java/com/retailsvc/http/support/TestCodings.java b/src/test/java/com/retailsvc/http/support/TestCodings.java new file mode 100644 index 00000000..ca1a2453 --- /dev/null +++ b/src/test/java/com/retailsvc/http/support/TestCodings.java @@ -0,0 +1,60 @@ +package com.retailsvc.http.support; + +import com.retailsvc.http.ContentCoding; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Set; +import java.util.zip.DeflaterOutputStream; +import java.util.zip.InflaterInputStream; + +/** Content codings for tests, built on java.util.zip so no test needs a compression library. */ +public final class TestCodings { + + private TestCodings() {} + + /** The zlib {@code deflate} coding: a real, dependency-free coding to register from tests. */ + public static ContentCoding deflate() { + return new ContentCoding() { + @Override + public String token() { + return "deflate"; + } + + @Override + public InputStream decode(InputStream coded) { + return new InflaterInputStream(coded); + } + + @Override + public OutputStream encode(OutputStream sink) { + return new DeflaterOutputStream(sink); + } + }; + } + + /** A coding that only carries its names; bodies pass through untouched. */ + public static ContentCoding named(String token, String... aliases) { + Set aliasSet = Set.of(aliases); + return new ContentCoding() { + @Override + public String token() { + return token; + } + + @Override + public Set aliases() { + return aliasSet; + } + + @Override + public InputStream decode(InputStream coded) { + return coded; + } + + @Override + public OutputStream encode(OutputStream sink) { + return sink; + } + }; + } +}