Skip to content

chore(clients): suppress unavoidable X509Certificate DN deprecation warnings - #3582

Open
brittytino wants to merge 38 commits into
AutoMQ:mainfrom
brittytino:chore/suppress-x509-dn-deprecation-warnings
Open

chore(clients): suppress unavoidable X509Certificate DN deprecation warnings#3582
brittytino wants to merge 38 commits into
AutoMQ:mainfrom
brittytino:chore/suppress-x509-dn-deprecation-warnings

Conversation

@brittytino

Copy link
Copy Markdown
Contributor

Why

Fixes #3282.

CommonNameLoggingTrustManagerFactoryWrapper.NeverExpiringX509Certificate extends X509Certificate directly. getSubjectDN()/getIssuerDN() are abstract on 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 returned Principal: the original certificate's getSubjectDN() typically returns an implementation-specific Principal (e.g. sun.security.x509.X500Name), while getSubjectX500Principal() returns a javax.security.auth.x500.X500Principal. I verified with a real self-signed cert that these two are not .equals() to each other even though their toString() output is identical:

subjectDN class: class sun.security.x509.X500Name
subjectX500 class: class javax.security.auth.x500.X500Principal
subjectDN.equals(subjectX500): false

Making that swap would silently change the wrapper's observable behavior and would break the existing test (CommonNameLoggingTrustManagerFactoryWrapperTest#testNeverExpiringX509Certificate), which asserts cert.getIssuerDN() / cert.getSubjectDN() equal wrappedCert.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.

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>
brittytino and others added 8 commits February 18, 2026 19:36
…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
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.

[CLEANUP] Replace deprecated X509Certificate.getSubjectDN() and getIssuerDN() with X500Principal equivalents

1 participant