chore(clients): suppress unavoidable X509Certificate DN deprecation warnings - #3582
Open
brittytino wants to merge 38 commits into
Open
chore(clients): suppress unavoidable X509Certificate DN deprecation warnings#3582brittytino wants to merge 38 commits into
brittytino wants to merge 38 commits into
Conversation
Signed-off-by: Tino Britty <153193545+brittytino@users.noreply.github.com>
Signed-off-by: Tino Britty <153193545+brittytino@users.noreply.github.com>
…ro functional changes Refactor client configuration methods to improve clarity and reduce redundancy. Signed-off-by: Tino Britty <153193545+brittytino@users.noreply.github.com>
Signed-off-by: Tino Britty <153193545+brittytino@users.noreply.github.com>
…arnings getSubjectDN()/getIssuerDN() are declared abstract on X509Certificate, so NeverExpiringX509Certificate must still override them even though both methods are deprecated since JDK 16. Swapping the internal delegation to getSubjectX500Principal()/getIssuerX500Principal() would change the returned Principal's runtime type (X500Principal vs the original certificate's implementation-specific type), which breaks equals()-based comparisons against the original certificate elsewhere (e.g. the existing unit test). Suppress the warnings on the required overrides instead, keeping behavior unchanged, and do the same at the test call sites that intentionally exercise the deprecated methods for backward-compat verification. Fixes AutoMQ#3282
brittytino
requested review from
1sonofqiu,
Gezi-lzq and
superhx
as code owners
September 6, 2026 17:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Fixes #3282.
CommonNameLoggingTrustManagerFactoryWrapper.NeverExpiringX509CertificateextendsX509Certificatedirectly.getSubjectDN()/getIssuerDN()areabstracton that class (deprecated since JDK 16, but never made non-abstract or removed), so any direct subclass is still required to implement them. The build currently emits 4 deprecation warnings from this file (one at each override declaration, one at each internal delegating call).What / How
I looked at swapping the internal delegation to
getSubjectX500Principal()/getIssuerX500Principal()as suggested in the issue, but that changes the runtime type of the returnedPrincipal: the original certificate'sgetSubjectDN()typically returns an implementation-specificPrincipal(e.g.sun.security.x509.X500Name), whilegetSubjectX500Principal()returns ajavax.security.auth.x500.X500Principal. I verified with a real self-signed cert that these two are not.equals()to each other even though theirtoString()output is identical:Making that swap would silently change the wrapper's observable behavior and would break the existing test (
CommonNameLoggingTrustManagerFactoryWrapperTest#testNeverExpiringX509Certificate), which assertscert.getIssuerDN()/cert.getSubjectDN()equalwrappedCert.getIssuerDN()/wrappedCert.getSubjectDN().Since the overrides can't be removed and changing their delegation isn't behavior-preserving, this PR keeps the delegation exactly as-is and adds
@SuppressWarnings("deprecation")on the two required overrides (with a short comment explaining why they can't be removed), plus on the test method that intentionally exercises the deprecated methods to verify backward-compatible behavior. No functional change.Testing
./gradlew :clients:compileJava :clients:compileTestJava— the 4 warnings from this file are gone; only pre-existing, unrelated warnings remain (AccessController/Subject.getSubject,JsonNode#fields(),ObjectMapper#setSerializationInclusion)../gradlew :clients:test --tests "org.apache.kafka.common.security.ssl.CommonNameLoggingTrustManagerFactoryWrapperTest"— all 11 tests pass../gradlew --build-cache :clients:checkstyleMain :clients:checkstyleTest :clients:spotbugsMain :clients:spotbugsTest :clients:spotlessJavaCheck— all pass, no formatting changes.