Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.
Retrospective finding from a historical scan of PR #74212 (merged 2025-01-06). Confirmed on current codebase — close with a note if already fixed.
Describe what's wrong
clickhouse-client --port 9440 does NOT auto-enable TLS. The client sends plaintext native protocol instead of TLS ClientHello, causing connection failures or data leaks when connecting to a TLS-only port.
Root cause: ConnectionParameters.cpp:70: commit dce8775 ('SSH Authentication') refactored the constructor, renaming parameters and adding StrongTypedef wrappers, but dropped the third argument (port_) from the enableSecureConnection() call. Changed from enableSecureConnection(config, connection_host, connection_port) to enableSecureConnection(config, host_).
Affected locations:
src/Client/ConnectionParameters.cpp:70 — enableSecureConnection(config, host_) missing port_ argument
Impact: Users connecting with --port 9440 without --secure get plaintext connections instead of TLS. This defeats the entire purpose of PR #74212's auto-detection feature. Connection to TLS-only servers fails with unhelpful errors, and connections to servers accepting both protocols would transmit credentials in plaintext.
Does it reproduce on most recent release?
Yes — confirmed on current master (commit a56315c84236).
How to reproduce
Run a TCP listener on port 9440, then connect clickhouse client with --port 9440 (no --secure). Inspect first bytes: 0x16 0x03 = TLS (correct), 0x00 0x11 ClickHouse = native protocol (bug).
Expected behavior
Client should send TLS ClientHello (starting with 0x16 0x03) when connecting to port 9440 without explicit --secure flag
Error message and/or stacktrace
Client sends native protocol handshake (bytes: 0011436c69636b486f75736520636c69656e741a) to port 9440 instead of TLS ClientHello (0x16 0x03)
Additional context
Suggested fix: Change line 70 from enableSecureConnection(config, host_) to enableSecureConnection(config, host_, port_)
Analysis details: Confidence HIGH | Severity P1 | Testability: INTEGRATION_TEST
Found during automated review of PR #74212.
Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.
Retrospective finding from a historical scan of PR #74212 (merged 2025-01-06). Confirmed on current codebase — close with a note if already fixed.
Describe what's wrong
clickhouse-client --port 9440 does NOT auto-enable TLS. The client sends plaintext native protocol instead of TLS ClientHello, causing connection failures or data leaks when connecting to a TLS-only port.
Root cause: ConnectionParameters.cpp:70: commit dce8775 ('SSH Authentication') refactored the constructor, renaming parameters and adding StrongTypedef wrappers, but dropped the third argument (port_) from the enableSecureConnection() call. Changed from enableSecureConnection(config, connection_host, connection_port) to enableSecureConnection(config, host_).
Affected locations:
src/Client/ConnectionParameters.cpp:70— enableSecureConnection(config, host_) missing port_ argumentImpact: Users connecting with --port 9440 without --secure get plaintext connections instead of TLS. This defeats the entire purpose of PR #74212's auto-detection feature. Connection to TLS-only servers fails with unhelpful errors, and connections to servers accepting both protocols would transmit credentials in plaintext.
Does it reproduce on most recent release?
Yes — confirmed on current
master(commita56315c84236).How to reproduce
Expected behavior
Error message and/or stacktrace
Additional context
Suggested fix: Change line 70 from enableSecureConnection(config, host_) to enableSecureConnection(config, host_, port_)
Analysis details: Confidence HIGH | Severity P1 | Testability:
INTEGRATION_TESTFound during automated review of PR #74212.