Commits
Tarun Sukhani authored 5bce13a8286
refactor: unify SSL pipeline under Netty SslContext for both PEM + JKS, h2 + h1 (release 1.12.20) PF-66: pre-PF-66, the SslHttpServerPipelineFactory had two parallel SSL paths in initChannel — one through Netty's SslContext (used only when play.http2.enabled=true) and one through the JDK SSLEngine via SslHttpServerContextFactory (used when h2 was off). The dual paths duplicated cert-loading code; the JDK path used a custom BouncyCastle- based PEMKeyManager; and the JDK SSLEngine's ALPN selection on JDK 25 was implementation-defined and empirically client-preferred, silently downgrading h2 to h1.1 even when both sides supported h2. This commit unifies both paths through a single Netty SslContext builder, with ALPN configured iff play.http2.enabled=true: - buildHttp2SslContext renamed to buildSslContext(boolean withAlpn). Same PEM/JKS detection (cert+key file presence first, falls back to keystore.file). ALPN config now conditional on withAlpn. Per- connection knobs (play.netty.clientAuth, play.ssl.enabledCiphers, play.ssl.enabledProtocols) lifted from the deleted SSLEngine-setter block to SslContextBuilder methods (clientAuth, ciphers, protocols) so the cached SslContext serves every connection. - The static cache is renamed cachedSslContext (no longer "http2"- specific). play.http2.enabled is read once at startup, so a single cache covers either path. - initChannel reduces from 50 lines to ~10. The JDK SSLEngine branch and the SSLEngine import are gone. - The stale "JKS users wanting h2 must convert to PEM" comment block is replaced — JKS+h2 has been working since PF-58 already, but the comment was never updated to reflect that. SslHttpServerContextFactory (164 lines) is now orphan and deleted — no test, module, or external caller references it. PEMKeyManager and the BouncyCastle-based PEM-parsing dance go with it; Netty's SslContextBuilder.forServer(certFile, keyFile) reads PEM directly via Netty's own PemReader, no BC required. Http3SslContextFactory's Javadoc cross-references to SslHttpServerContextFactory updated to point at the unified buildSslContext. Tests: - New buildSslContextFromJksWithAlpnAdvertisesH2ThenHttp1 verifies the post-PF-66 outcome: a JKS keystore + http2-enabled produces an SslContext whose ALPN negotiator advertises [h2, http/1.1] in priority order. Pre-fix the JKS path went through SSLEngine and got no ALPN advertisement at all. - New buildSslContextFromJksWithoutAlpnSucceeds verifies the JKS+h1.1- only path still builds via the unified Netty path (replacing the deleted JDK-engine flow). - The JKS keystore is generated via keytool subprocess (the same way PF-67's `play enable-https` does it) rather than via BouncyCastle — avoids pulling bcutil-jdk18on as a new framework test dep. - Existing flag-parsing and ALPN_PROTOCOLS-order tests retained. End-to-end ALPN handshake-level testing remains in integration.Http2FunctionalTest, which stands up a real Netty server with a self-signed test cert (PEM today; could be extended to JKS as follow-up, but the unit tests in this commit cover the SslContext- build path that was the actual bug surface). ant compile clean, full ant unittest BUILD SUCCESSFUL. BouncyCastle (bcprov-jdk18on, bcpkix-jdk18on) is now potentially orphan in framework code — no remaining direct callers. Will be flagged by ant resolve (PF-62) on its next audit; cleanup deferred to a follow-up to keep this commit surgical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>