Skip to content

feat: Gzip content encoding - #126

Open
thced wants to merge 14 commits into
masterfrom
feat/gzip-content-encoding
Open

feat: Gzip content encoding#126
thced wants to merge 14 commits into
masterfrom
feat/gzip-content-encoding

Conversation

@thced

@thced thced commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

No description provided.

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`.
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.
`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.
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.
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.
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.
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.
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.
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.
Collapses the byte-body content-type resolution into two expressions and
inlines the remaining single-use string constants.
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.
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.
@thced
thced force-pushed the feat/gzip-content-encoding branch from 8d1a3d6 to 0ac5e48 Compare September 10, 2026 13:45
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.
- 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.
@thced
thced force-pushed the feat/gzip-content-encoding branch from cc99c62 to 15d4953 Compare September 10, 2026 14:54
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants