use AbstractClientTlsStrategy instead

apache · Sep 21, 2024 · a2e43fa · a2e43fa
1 parent bd7e619
commit a2e43fa
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 45 deletions.
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
@@ -51,6 +51,7 @@
 
 import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.concurrent.FutureCallback;
 import org.apache.hc.core5.http.HttpHost;
@@ -271,7 +272,7 @@ void verifySession(
                     final X509Certificate x509 = (X509Certificate) cert;
                     final X500Principal peer = x509.getSubjectX500Principal();
 
-                    LOG.debug(" peer principal: {}", peer);
+                    LOG.debug("Escaped peer principal: {}", toEscapedString(peer));
                     final Collection<List<?>> altNames1 = x509.getSubjectAlternativeNames();
                     if (altNames1 != null) {
                         final List<String> altNames = new ArrayList<>();
@@ -284,7 +285,7 @@ void verifySession(
                     }
 
                     final X500Principal issuer = x509.getIssuerX500Principal();
-                    LOG.debug(" issuer principal: {}", issuer);
+                    LOG.debug("Escaped issuer principal: {}", toEscapedString(issuer));
                     final Collection<List<?>> altNames2 = x509.getIssuerAlternativeNames();
                     if (altNames2 != null) {
                         final List<String> altNames = new ArrayList<>();
@@ -322,4 +323,33 @@ void verifySession(
         }
     }
 
+    /**
+     * Converts an X500Principal to a cleaned string by escaping control characters.
+     * <p>
+     * This method processes the RFC2253 format of the X500Principal and escapes
+     * any ISO control characters to avoid issues in logging or other outputs.
+     * Control characters are replaced with their escaped hexadecimal representation.
+     * </p>
+     *
+     * <p><strong>Note:</strong> For testing purposes, this method is package-private
+     * to allow access within the same package. This allows tests to verify the correct
+     * behavior of the escaping process.</p>
+     *
+     * @param principal the X500Principal to escape
+     * @return the escaped string representation of the X500Principal
+     */
+    @Internal
+    String toEscapedString(final X500Principal principal) {
+        final String principalValue = principal.getName(X500Principal.RFC2253);
+        final StringBuilder sanitizedPrincipal = new StringBuilder(principalValue.length());
+        for (final char c : principalValue.toCharArray()) {
+            if (Character.isISOControl(c)) {
+                sanitizedPrincipal.append("\\x").append(String.format("%02x", (int) c));
+            } else {
+                sanitizedPrincipal.append(c);
+            }
+        }
+        return sanitizedPrincipal.toString();
+    }
+
 }
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java
@@ -57,7 +57,6 @@
 
 import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.core5.annotation.Contract;
-import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.protocol.HttpContext;
@@ -404,15 +403,9 @@ void verifySession(
                 final Certificate cert = certs[0];
                 if (cert instanceof X509Certificate) {
                     final X509Certificate x509 = (X509Certificate) cert;
-                    // Sanitize and log peer principal
                     final X500Principal peer = x509.getSubjectX500Principal();
-                    LOG.debug("Escaped peer principal: {}", toEscapedString(peer));
-
-                    // Sanitize and log issuer principal
-                    final X500Principal issuer = x509.getIssuerX500Principal();
-                    LOG.debug("Escaped issuer principal: {}", toEscapedString(issuer));
-
 
+                    LOG.debug(" peer principal: {}", peer);
                     final Collection<List<?>> altNames1 = x509.getSubjectAlternativeNames();
                     if (altNames1 != null) {
                         final List<String> altNames = new ArrayList<>();
@@ -424,6 +417,8 @@ void verifySession(
                         LOG.debug(" peer alternative names: {}", altNames);
                     }
 
+                    final X500Principal issuer = x509.getIssuerX500Principal();
+                    LOG.debug(" issuer principal: {}", issuer);
                     final Collection<List<?>> altNames2 = x509.getIssuerAlternativeNames();
                     if (altNames2 != null) {
                         final List<String> altNames = new ArrayList<>();
@@ -461,33 +456,4 @@ void verifySession(
         }
     }
 
-    /**
-     * Converts an X500Principal to a cleaned string by escaping control characters.
-     * <p>
-     * This method processes the RFC2253 format of the X500Principal and escapes
-     * any ISO control characters to avoid issues in logging or other outputs.
-     * Control characters are replaced with their escaped hexadecimal representation.
-     * </p>
-     *
-     * <p><strong>Note:</strong> For testing purposes, this method is package-private
-     * to allow access within the same package. This allows tests to verify the correct
-     * behavior of the escaping process.</p>
-     *
-     * @param principal the X500Principal to escape
-     * @return the escaped string representation of the X500Principal
-     */
-    @Internal
-    String toEscapedString(final X500Principal principal) {
-        final String principalValue = principal.getName(X500Principal.RFC2253);
-        final StringBuilder sanitizedPrincipal = new StringBuilder(principalValue.length());
-        for (final char c : principalValue.toCharArray()) {
-            if (Character.isISOControl(c)) {
-                sanitizedPrincipal.append("\\x").append(String.format("%02x", (int) c));
-            } else {
-                sanitizedPrincipal.append(c);
-            }
-        }
-        return sanitizedPrincipal.toString();
-    }
-
 }
diff --git a/...p/ssl/SSLConnectionSocketFactoryTest.java → ...tp/ssl/AbstractClientTlsStrategyTest.java b/...p/ssl/SSLConnectionSocketFactoryTest.java → ...tp/ssl/AbstractClientTlsStrategyTest.java
@@ -33,22 +33,42 @@
 
 import java.security.cert.X509Certificate;
 
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSession;
 import javax.security.auth.x500.X500Principal;
 
+import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
+import org.apache.hc.core5.reactor.ssl.TlsDetails;
 import org.apache.hc.core5.ssl.SSLContexts;
 import org.junit.jupiter.api.Test;
 
-public class SSLConnectionSocketFactoryTest {
+public class AbstractClientTlsStrategyTest {
 
     @Test
     public void testToEscapedString_withControlCharacters() {
         // Create a X500Principal with control characters
         final X500Principal principal = new X500Principal("CN=Test\b\bName\n,O=TestOrg");
 
+        // Create a mock subclass of AbstractClientTlsStrategy
+        final AbstractClientTlsStrategy tlsStrategy = new AbstractClientTlsStrategy(
+                SSLContexts.createDefault(),
+                null, null, SSLBufferMode.STATIC,
+                HostnameVerificationPolicy.BUILTIN,
+                HttpsSupport.getDefaultHostnameVerifier()) {
+            @Override
+            void applyParameters(final SSLEngine sslEngine, final SSLParameters sslParameters, final String[] appProtocols) {
+                // No-op for test
+            }
+
+            @Override
+            TlsDetails createTlsDetails(final SSLEngine sslEngine) {
+                return null;  // No-op for test
+            }
+        };
+
         // Call the toEscapedString method
-        final SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(SSLContexts.createDefault());
-        final String escaped = factory.toEscapedString(principal);
+        final String escaped = tlsStrategy.toEscapedString(principal);
 
         // Assert that control characters are properly escaped
         assertEquals("CN=Test\\x08\\x08Name\\x0a,O=TestOrg", escaped);
@@ -68,9 +88,27 @@ public void testVerifySession_escapedPeerAndIssuer() throws Exception {
         when(mockCert.getSubjectX500Principal()).thenReturn(peerPrincipal);
         when(mockCert.getIssuerX500Principal()).thenReturn(issuerPrincipal);
 
+        // Create a mock subclass of AbstractClientTlsStrategy
+        final AbstractClientTlsStrategy tlsStrategy = new AbstractClientTlsStrategy(
+                SSLContexts.createDefault(),
+                null, null, SSLBufferMode.STATIC,
+                HostnameVerificationPolicy.BUILTIN,
+                HttpsSupport.getDefaultHostnameVerifier()) {
+            @Override
+            void applyParameters(final SSLEngine sslEngine, final SSLParameters sslParameters, final String[] appProtocols) {
+                // No-op for test
+            }
+
+            @Override
+            TlsDetails createTlsDetails(final SSLEngine sslEngine) {
+                return null;  // No-op for test
+            }
+        };
+
         // Test the verifySession method
-        final SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(SSLContexts.createDefault());
-        factory.verifySession("localhost", mockSession, null);
+        tlsStrategy.verifySession("localhost", mockSession, null);
 
     }
-}
+
+
+}