refactor(client-v2): parse ssl_cipher_suites in the config layer

Move the ssl_cipher_suites string->list sanitization (drop blank tokens
from a leading/trailing/doubled comma, trim each surviving name) out of
HttpAPIClientHelper transport code and into a ClientConfigProperties
SSL_CIPHER_SUITES parseValue override, mirroring CLIENT_RETRY_ON_FAILURE.
The transport layer now consumes an already-sanitized list (an unset or
empty list means "no restriction"), keeping transport code free of
config parsing per the repo architecture convention. Relocate the
sanitization coverage to ClientConfigPropertiesTest, and replace the
CustomSSLConnectionFactory super(...) block comment with an inline
null /* supportedProtocols */.

Addresses @chernser review feedback on #2919.

Author: polyglotAI-bot

client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java

@@ -211,10 +211,28 @@ public Object parseValue(String value) {
      * support); when unset, the transport defaults are used (Apache HttpClient enables the JVM's default
      * suites minus those it considers weak).
      * <p>
+     * The value is parsed into a sanitized list here, in the config-parsing layer (not in transport code):
+     * blank tokens produced by a leading, trailing or doubled comma are dropped and each surviving name is
+     * trimmed, so consumers receive a ready-to-use list. A null, empty or whitespace-only entry would
+     * otherwise be rejected by the SSL socket and break the handshake even alongside valid suites.
+     * <p>
      * Appended at the end of the enum on purpose: adding a constant in the middle would shift the ordinal
      * of every following constant (see {@code docs/changes_checklist.md}).
      */
-    SSL_CIPHER_SUITES("ssl_cipher_suites", List.class),
+    SSL_CIPHER_SUITES("ssl_cipher_suites", List.class) {
+        @Override
+        @SuppressWarnings("unchecked")
+        public Object parseValue(String value) {
+            List<String> suites = (List<String>) super.parseValue(value);
+            if (suites == null) {
+                return null;
+            }
+            return suites.stream()
+                    .filter(s -> s != null && !s.trim().isEmpty())
+                    .map(String::trim)
+                    .collect(Collectors.toList());
+        }
+    },
     ;
 
     private static final Logger LOG = LoggerFactory.getLogger(ClientConfigProperties.class);

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

@@ -289,18 +289,11 @@ public CloseableHttpClient createHttpClient(boolean initSslContext, Map<String,
             // set because the connection hostname will not match the certificate.
             boolean trustAllHostnames = sslMode == SSLMode.TRUST || sslMode == SSLMode.VERIFY_CA;
             boolean hasSNI = socketSNI != null && !socketSNI.trim().isEmpty();
+            // ssl_cipher_suites is parsed and sanitized in ClientConfigProperties (blank tokens dropped,
+            // names trimmed); an unset or empty list means "no restriction" so the transport defaults apply.
             List<String> cipherSuites = ClientConfigProperties.SSL_CIPHER_SUITES.getOrDefault(configuration);
-            // Drop blank tokens (e.g. from a leading, trailing or doubled comma in ssl_cipher_suites) and trim
-            // each name: a null, empty or whitespace-only entry is rejected by the SSL socket and breaks the
-            // handshake even when valid suites are also present.
-            String[] enabledCipherSuites = cipherSuites == null ? null
-                    : cipherSuites.stream()
-                            .filter(s -> s != null && !s.trim().isEmpty())
-                            .map(String::trim)
-                            .toArray(String[]::new);
-            if (enabledCipherSuites != null && enabledCipherSuites.length == 0) {
-                enabledCipherSuites = null;
-            }
+            String[] enabledCipherSuites = cipherSuites == null || cipherSuites.isEmpty() ? null
+                    : cipherSuites.toArray(new String[0]);
             if (hasSNI || trustAllHostnames || enabledCipherSuites != null) {
                 // Skip hostname verification only for trust-all modes or when a custom SNI is used (the
                 // connection hostname would not match the certificate); otherwise a null verifier makes the
@@ -1097,10 +1090,7 @@ public CustomSSLConnectionFactory(String defaultSNI, SSLContext sslContext, Host
 
         public CustomSSLConnectionFactory(String defaultSNI, SSLContext sslContext, HostnameVerifier hostnameVerifier,
                                           String[] supportedCipherSuites) {
-            // supportedProtocols is left as null (transport defaults apply); supportedCipherSuites, when
-            // provided, restricts the cipher suites the base factory enables on each socket. A null
-            // hostnameVerifier makes the base factory fall back to its default verifier.
-            super(sslContext, null, supportedCipherSuites, hostnameVerifier);
+            super(sslContext, null /* supportedProtocols */, supportedCipherSuites, hostnameVerifier);
             this.defaultSNI = defaultSNI == null || defaultSNI.trim().isEmpty() ? null : new SNIHostName(defaultSNI);
         }
 

client-v2/src/test/java/com/clickhouse/client/api/ClientConfigPropertiesTest.java

@@ -2,8 +2,13 @@
 
 
 import org.testng.Assert;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 
@@ -31,4 +36,44 @@ public void testToKeyValuePairs() {
 //        Assert.assertEquals(map.get("key1"), "value1");
 //        Assert.assertEquals(map.get("key2"), "value2");
     }
+
+    @DataProvider(name = "sslCipherSuites")
+    public static Object[][] sslCipherSuites() {
+        return new Object[][]{
+                // raw ssl_cipher_suites value -> expected parsed list
+                {"TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256",
+                        Arrays.asList("TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256")},
+                {"TLS_AES_128_GCM_SHA256", Collections.singletonList("TLS_AES_128_GCM_SHA256")},
+                // each surviving name is trimmed
+                {" TLS_AES_256_GCM_SHA384 , TLS_AES_128_GCM_SHA256 ",
+                        Arrays.asList("TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256")},
+                // blank tokens from a leading, doubled or trailing comma (and whitespace-only) are dropped
+                {",TLS_AES_256_GCM_SHA384,, TLS_AES_128_GCM_SHA256 ,",
+                        Arrays.asList("TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256")},
+                // no usable suite -> empty list (treated downstream as "no restriction")
+                {"", Collections.emptyList()},
+                {"   ", Collections.emptyList()},
+                {",, ,", Collections.emptyList()},
+        };
+    }
+
+    @Test(groups = {"unit"}, dataProvider = "sslCipherSuites")
+    public void testSslCipherSuitesParsedAndSanitized(String raw, List<String> expected) {
+        Assert.assertEquals(ClientConfigProperties.SSL_CIPHER_SUITES.parseValue(raw), expected);
+    }
+
+    @Test(groups = {"unit"})
+    public void testSslCipherSuitesUnsetParsesToNull() {
+        Assert.assertNull(ClientConfigProperties.SSL_CIPHER_SUITES.parseValue(null));
+    }
+
+    @Test(groups = {"unit"})
+    public void testParseConfigMapSanitizesSslCipherSuites() {
+        Map<String, String> raw = new HashMap<>();
+        raw.put(ClientConfigProperties.SSL_CIPHER_SUITES.getKey(),
+                ",TLS_AES_256_GCM_SHA384,, TLS_AES_128_GCM_SHA256 ,");
+        Map<String, Object> parsed = ClientConfigProperties.parseConfigMap(raw);
+        Assert.assertEquals(parsed.get(ClientConfigProperties.SSL_CIPHER_SUITES.getKey()),
+                Arrays.asList("TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256"));
+    }
 }

client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java

@@ -228,45 +228,6 @@ public void testCreateHttpClientEmptyCipherSuitesTreatedAsNoRestriction() {
                 + "(transport defaults), so the plain SSLConnectionSocketFactory is used");
     }
 
-    /**
-     * Malformed cipher-suite input - blank tokens from a leading, trailing or doubled comma, and
-     * whitespace-padded names - must be sanitized before reaching the SSL socket: a null, empty or
-     * whitespace-only entry is rejected by {@code SSLSocket#setEnabledCipherSuites} ("invalid null or empty
-     * string elements") and breaks the handshake even when valid suites are also present. The blanks must be
-     * dropped and the surviving names trimmed, preserving order.
-     */
-    @Test
-    public void testCreateHttpClientDropsBlankAndWhitespaceCipherTokens() {
-        Map<String, Object> config = new HashMap<>();
-        config.put(ClientConfigProperties.SSL_CIPHER_SUITES.getKey(),
-                Arrays.asList("", "TLS_AES_256_GCM_SHA384", "", "   ", " TLS_AES_128_GCM_SHA256 "));
-
-        List<List<?>> calls = captureCustomFactoryConstruction(config);
-
-        assertEquals(calls.size(), 1, "configured (non-blank) cipher suites must still build the custom factory");
-        List<?> args = calls.get(0);
-        assertEquals((String[]) args.get(3),
-                new String[]{"TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256"},
-                "blank tokens (leading/doubled comma, whitespace-only) must be dropped and surviving names "
-                        + "trimmed before being forwarded to the SSL socket factory");
-    }
-
-    /**
-     * Boundary case: a cipher-suite list that contains only blank tokens (e.g. from a property that is just
-     * commas/whitespace) has no usable suite, so - like an empty or unset list - it must be treated as "no
-     * restriction" (transport defaults) rather than forwarding an all-blank array that would fail every handshake.
-     */
-    @Test
-    public void testCreateHttpClientBlankOnlyCipherSuitesTreatedAsNoRestriction() {
-        Map<String, Object> config = new HashMap<>();
-        config.put(ClientConfigProperties.SSL_CIPHER_SUITES.getKey(), Arrays.asList("", "   "));
-
-        List<List<?>> calls = captureCustomFactoryConstruction(config);
-
-        assertEquals(calls.size(), 0, "a cipher-suite list of only blank tokens must be treated as no "
-                + "restriction (transport defaults), so the plain SSLConnectionSocketFactory is used");
-    }
-
     @Test
     public void testExecuteRequestThrowsConnectExceptionOn502() throws Exception {
         Map<String, Object> configuration = new HashMap<>();