Skip to content

Commit 675b8f5

Browse files
client-v2: drop blank tokens from ssl_cipher_suites before enabling them
A leading, trailing or doubled comma in ssl_cipher_suites (or a raw List option) leaves an empty or whitespace-only token in the parsed list. The value was only checked for emptiness (zero elements), so a blank token reached SSLSocket#setEnabledCipherSuites, which throws IllegalArgumentException ("invalid null or empty string elements") and breaks the TLS handshake even when valid suites are also present. Blank tokens are now dropped and each surviving name is trimmed; a list that reduces to nothing is treated as "no restriction" (JVM defaults), matching the empty/unset behaviour. Addresses the Cursor Bugbot review comment on PR #2919.
1 parent d17cf3d commit 675b8f5

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,17 @@ public CloseableHttpClient createHttpClient(boolean initSslContext, Map<String,
290290
boolean trustAllHostnames = sslMode == SSLMode.TRUST || sslMode == SSLMode.VERIFY_CA;
291291
boolean hasSNI = socketSNI != null && !socketSNI.trim().isEmpty();
292292
List<String> cipherSuites = ClientConfigProperties.SSL_CIPHER_SUITES.getOrDefault(configuration);
293-
String[] enabledCipherSuites = cipherSuites == null || cipherSuites.isEmpty()
294-
? null : cipherSuites.toArray(new String[0]);
293+
// Drop blank tokens (e.g. from a leading, trailing or doubled comma in ssl_cipher_suites) and trim
294+
// each name: a null, empty or whitespace-only entry is rejected by the SSL socket and breaks the
295+
// handshake even when valid suites are also present.
296+
String[] enabledCipherSuites = cipherSuites == null ? null
297+
: cipherSuites.stream()
298+
.filter(s -> s != null && !s.trim().isEmpty())
299+
.map(String::trim)
300+
.toArray(String[]::new);
301+
if (enabledCipherSuites != null && enabledCipherSuites.length == 0) {
302+
enabledCipherSuites = null;
303+
}
295304
if (hasSNI || trustAllHostnames || enabledCipherSuites != null) {
296305
// Skip hostname verification only for trust-all modes or when a custom SNI is used (the
297306
// connection hostname would not match the certificate); otherwise a null verifier makes the

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,45 @@ public void testCreateHttpClientEmptyCipherSuitesTreatedAsNoRestriction() {
221221
+ "defaults), so the plain SSLConnectionSocketFactory is used");
222222
}
223223

224+
/**
225+
* Malformed cipher-suite input - blank tokens from a leading, trailing or doubled comma, and
226+
* whitespace-padded names - must be sanitized before reaching the SSL socket: a null, empty or
227+
* whitespace-only entry is rejected by {@code SSLSocket#setEnabledCipherSuites} ("invalid null or empty
228+
* string elements") and breaks the handshake even when valid suites are also present. The blanks must be
229+
* dropped and the surviving names trimmed, preserving order.
230+
*/
231+
@Test
232+
public void testCreateHttpClientDropsBlankAndWhitespaceCipherTokens() {
233+
Map<String, Object> config = new HashMap<>();
234+
config.put(ClientConfigProperties.SSL_CIPHER_SUITES.getKey(),
235+
Arrays.asList("", "TLS_AES_256_GCM_SHA384", "", " ", " TLS_AES_128_GCM_SHA256 "));
236+
237+
List<List<?>> calls = captureCustomFactoryConstruction(config);
238+
239+
assertEquals(calls.size(), 1, "configured (non-blank) cipher suites must still build the custom factory");
240+
List<?> args = calls.get(0);
241+
assertEquals((String[]) args.get(3),
242+
new String[]{"TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256"},
243+
"blank tokens (leading/doubled comma, whitespace-only) must be dropped and surviving names "
244+
+ "trimmed before being forwarded to the SSL socket factory");
245+
}
246+
247+
/**
248+
* Boundary case: a cipher-suite list that contains only blank tokens (e.g. from a property that is just
249+
* commas/whitespace) has no usable suite, so - like an empty or unset list - it must be treated as "no
250+
* restriction" (JVM defaults) rather than forwarding an all-blank array that would fail every handshake.
251+
*/
252+
@Test
253+
public void testCreateHttpClientBlankOnlyCipherSuitesTreatedAsNoRestriction() {
254+
Map<String, Object> config = new HashMap<>();
255+
config.put(ClientConfigProperties.SSL_CIPHER_SUITES.getKey(), Arrays.asList("", " "));
256+
257+
List<List<?>> calls = captureCustomFactoryConstruction(config);
258+
259+
assertEquals(calls.size(), 0, "a cipher-suite list of only blank tokens must be treated as no "
260+
+ "restriction (JVM defaults), so the plain SSLConnectionSocketFactory is used");
261+
}
262+
224263
/**
225264
* Builds an {@link HttpAPIClientHelper} and invokes {@link HttpAPIClientHelper#createHttpClient} with SSL
226265
* enabled while intercepting every {@link CustomSSLConnectionFactory} construction, returning the

0 commit comments

Comments
 (0)