From fa662f2f07a47d3ee3018a6c11b5d5466d3e9a1e Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 26 Aug 2026 15:41:13 -0400 Subject: [PATCH] fix(internal): narrow TLS classification to certificate failures only (SDK-2789) hasTlsOrCertificateCause matched any SSLException or GeneralSecurityException anywhere in the cause chain. Because SDK traffic is HTTPS, every transport error arrives through the TLS layer, so this swept in transient faults that have nothing to do with certificate validity. Confirmed against a real SSLServerSocket: a peer closing the connection mid-handshake with a FIN produces SSLHandshakeException: Remote host terminated the handshake caused by EOFException: SSL peer shut down incorrectly which the old predicate classified UNEXPECTED, putting a data source into extended-regime backoff (5 min - 1 hr) for what is typically a load balancer drain, a connection-limit close, or an idle timeout during a slow handshake. The same fault delivered as a RST instead produces SocketException and was classified NORMAL -- so the regime depended on whether the peer sent FIN or RST, an arbitrary detail of the intermediary. The compounding case is worse than a single stall: a connection that flaps faster than the 60s healthy-operation reset window never accumulates enough continuous connectivity to reset, so it ratchets 5m -> 10m -> 20m -> 40m -> 1h and stays there. Now matches only genuinely long-lived certificate problems: - CertificateException (expired, not yet valid, hostname mismatch from a verifier; also covers sun.security.validator.ValidatorException) - CertPathValidatorException / CertPathBuilderException (untrusted chain) - SSLPeerUnverifiedException (hostname mismatch) This aligns Java with the Go server SDK's classifier, which enumerates only certificate errors (tls.CertificateVerificationError, x509.UnknownAuthorityError, x509.HostnameError, x509.CertificateInvalidError) and treats everything else as normal. The previous Java behavior was a divergence from that reference, not a different reading of the spec. Verified empirically that a genuinely untrusted chain still classifies UNEXPECTED: JSSE produces SSLHandshakeException -> ValidatorException -> SunCertPathBuilderException, and the first two match the new predicate. Tests: replaces sslHandshakeIsUnexpected (which asserted the over-broad behavior) with bareSslHandshakeFailureIsNormal and peerClosedMidHandshakeIsNormal; adds coverage for CertPathValidatorException, CertPathBuilderException, CertificateNotYetValidException, and the real untrusted-chain wrapper shape. --- .../sdk/internal/http/FailureClass.java | 12 ++++-- .../http/HttpErrorsClassificationTest.java | 42 ++++++++++++++++--- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/lib/shared/internal/src/main/java/com/launchdarkly/sdk/internal/http/FailureClass.java b/lib/shared/internal/src/main/java/com/launchdarkly/sdk/internal/http/FailureClass.java index c678b718..280ead16 100644 --- a/lib/shared/internal/src/main/java/com/launchdarkly/sdk/internal/http/FailureClass.java +++ b/lib/shared/internal/src/main/java/com/launchdarkly/sdk/internal/http/FailureClass.java @@ -1,8 +1,10 @@ package com.launchdarkly.sdk.internal.http; -import javax.net.ssl.SSLException; +import javax.net.ssl.SSLPeerUnverifiedException; -import java.security.GeneralSecurityException; +import java.security.cert.CertPathBuilderException; +import java.security.cert.CertPathValidatorException; +import java.security.cert.CertificateException; /** * Classifies a failure into one of two regimes: {@link #NORMAL} or @@ -34,8 +36,10 @@ public enum FailureClass { */ static boolean hasTlsOrCertificateCause(Throwable t) { for (Throwable c = t; c != null; c = c.getCause()) { - if (c instanceof SSLException - || c instanceof GeneralSecurityException) { + if (c instanceof CertificateException + || c instanceof CertPathValidatorException + || c instanceof CertPathBuilderException + || c instanceof SSLPeerUnverifiedException) { return true; } } diff --git a/lib/shared/internal/src/test/java/com/launchdarkly/sdk/internal/http/HttpErrorsClassificationTest.java b/lib/shared/internal/src/test/java/com/launchdarkly/sdk/internal/http/HttpErrorsClassificationTest.java index 9b284b75..cf9d78bd 100644 --- a/lib/shared/internal/src/test/java/com/launchdarkly/sdk/internal/http/HttpErrorsClassificationTest.java +++ b/lib/shared/internal/src/test/java/com/launchdarkly/sdk/internal/http/HttpErrorsClassificationTest.java @@ -2,14 +2,19 @@ import org.junit.Test; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLPeerUnverifiedException; +import java.io.EOFException; import java.io.IOException; import java.net.ConnectException; import java.net.SocketTimeoutException; +import java.security.cert.CertPathBuilderException; +import java.security.cert.CertPathValidatorException; import java.security.cert.CertificateException; import java.security.cert.CertificateExpiredException; +import java.security.cert.CertificateNotYetValidException; import static com.launchdarkly.sdk.internal.http.FailureClass.NORMAL; import static com.launchdarkly.sdk.internal.http.FailureClass.UNEXPECTED; @@ -57,9 +62,6 @@ public class HttpErrorsClassificationTest { } // TLS / certificate validation failures are UNEXPECTED. - @Test public void sslHandshakeIsUnexpected() { - assertEquals(UNEXPECTED, HttpErrors.classifyTransportFailure(new SSLHandshakeException("handshake failed"))); - } @Test public void sslPeerUnverifiedIsUnexpected() { assertEquals(UNEXPECTED, HttpErrors.classifyTransportFailure(new SSLPeerUnverifiedException("peer not verified"))); } @@ -69,10 +71,40 @@ public class HttpErrorsClassificationTest { @Test public void certificateExpiredIsUnexpected() { assertEquals(UNEXPECTED, HttpErrors.classifyTransportFailure(new CertificateExpiredException("expired"))); } + @Test public void certificateNotYetValidIsUnexpected() { + assertEquals(UNEXPECTED, + HttpErrors.classifyTransportFailure(new CertificateNotYetValidException("not yet valid"))); + } + @Test public void certPathValidatorFailureIsUnexpected() { + assertEquals(UNEXPECTED, + HttpErrors.classifyTransportFailure(new CertPathValidatorException("path invalid"))); + } + @Test public void certPathBuilderFailureIsUnexpected() { + assertEquals(UNEXPECTED, + HttpErrors.classifyTransportFailure(new CertPathBuilderException("cannot build path"))); + } + + @Test public void untrustedChainWrappedInHandshakeExceptionIsUnexpected() { + SSLHandshakeException e = new SSLHandshakeException("PKIX path building failed"); + e.initCause(new CertPathBuilderException("unable to find valid certification path")); + assertEquals(UNEXPECTED, HttpErrors.classifyTransportFailure(e)); + } + + @Test public void bareSslHandshakeFailureIsNormal() { + assertEquals(NORMAL, HttpErrors.classifyTransportFailure(new SSLHandshakeException("handshake failed"))); + } + @Test public void peerClosedMidHandshakeIsNormal() { + SSLHandshakeException e = new SSLHandshakeException("Remote host terminated the handshake"); + e.initCause(new EOFException("SSL peer shut down incorrectly")); + assertEquals(NORMAL, HttpErrors.classifyTransportFailure(e)); + } + @Test public void sslExceptionFromConnectionResetIsNormal() { + assertEquals(NORMAL, HttpErrors.classifyTransportFailure(new SSLException("Connection reset"))); + } // Cause-chain walk finds TLS deep in wrapper exceptions. - @Test public void sslCauseWrappedIsUnexpected() { - IOException wrapper = new IOException("wrapped", new SSLHandshakeException("real cause")); + @Test public void certificateCauseWrappedIsUnexpected() { + IOException wrapper = new IOException("wrapped", new CertificateException("real cause")); assertEquals(UNEXPECTED, HttpErrors.classifyTransportFailure(wrapper)); } }