Commits
Tarun Sukhani authored 6fb9ed6de6e
[PF-37..PF-53] Audit follow-ups: clear all 17 post-migration findings
Closes the remaining audit tickets from the multi-agent post-migration
review (PF-37..PF-43) and the external Codex review (PF-44..PF-53).
299/299 unit tests pass.
Protocol correctness:
- PF-37 StreamChunkAggregator: strip only the `chunked` token from
Transfer-Encoding so coexisting codings (e.g. gzip) survive aggregation
- PF-38 PlayHandler: install Netty's WebSocketFrameAggregator after the
handshake so fragmented WS messages are reassembled before reaching
the controller
- PF-39 PlayHandler: server-initiated close now writes a
CloseWebSocketFrame with status 1000 via the handshaker; peers no
longer observe 1006 ("Abnormal Closure")
- PF-40 LazyChunkedInput: stop pre-encoding chunked frames; under
Netty 4, HttpResponseEncoder auto-frames ByteBufs in chunked state and
the manual `len\r\n...\r\n` wrappers double-encoded the wire stream.
The terminator is now emitted by LastHttpContent.EMPTY_LAST_CONTENT
- PF-47 FileService: malformed Range headers (missing `bytes=` prefix,
non-numeric values) no longer NPE inside ChunkedWriteHandler;
byteRanges is initialised to an empty array on parse failure and
isEndOfInput guards against null
- PF-48 FileService: suffix-range parsing (`Range: bytes=-N`) now
returns exactly the last N bytes (was N+1, off by one)
- PF-52 FileService: multipart/byteranges responses are now well-formed
-- Content-Range header (was the typo "ContentRange"), inter-part
CRLF separators, and the closing --boundary-- delimiter
Resource lifetime:
- PF-41 FileService: write the body without flush, then writeAndFlush
the LastHttpContent; the keep-alive close listener attaches to the
trailer's future so the channel is not closed before the chunked body
has drained
- PF-42 FileService: close the ChunkedInput (which owns the
RandomAccessFile) when the channel was already closed before the
write started, eliminating a per-request RAF leak
- PF-44 StreamChunkAggregator: 413 oversize rejection now deletes the
partial spool file before clearing the reference; cleanupSpool() does
not delete (the success path transfers ownership), so the rejection
path needed an explicit unlink
- PF-45 PlayHandler: plugin rawInvocation paths now run cleanupSpooledBody
in a finally; previously this only ran inside NettyInvocation, leaking
one temp file per upload that hit the spool threshold whenever a plugin
short-circuited the controller
- PF-46 PlayHandler: NettyInvocation now defers spool cleanup across
suspend/resume via a `suspended` flag set in the suspend() override
and reset at the start of each run(); the resumed invocation can read
the body again
- PF-51 SslHttpServerContextFactory: PEM SSL trust manager now loads an
empty in-memory keystore via `load(null, null)` before
TrustManagerFactory.init(); the previous uninitialised keystore threw
KeyStoreException on JDK 17+
Concurrency:
- PF-43 PlayHandler: staticPathsCache migrated to ConcurrentHashMap; the
previous unsynchronized containsKey raced with the worker-thread put
on the RenderStatic catch path. Synchronized blocks dropped
- PF-49 HttpServerPipelineFactory: classes cache migrated to
ConcurrentHashMap; the previous shared HashMap.computeIfAbsent raced
under concurrent initChannel calls
- PF-50 HttpServerPipelineFactory + SslHttpServerPipelineFactory: trim
pipeline FQCNs before Class.forName; previously a leading space in
play.netty.pipeline / play.ssl.netty.pipeline killed pipeline construction
Documentation:
- PF-53 configuration.textile: documents play.netty.maxContentLength
semantic change (per-chunk cap -> hard 413 cap), the new
play.netty.spoolThresholdBytes property, and the play.threads.virtual.*
family (global toggle + per-subsystem overrides for invoker/jobs/mail)
- releasenotes-1.12.0.textile: promotes the "known limitations" and
"out of scope" entries into "what's fixed" with one bullet per fixed
ticket; updates verified-tests count to 299/299
Verified with `ant compile` and `ant unittest`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>