Skip to content

removed default port adjustment #2754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chernser wants to merge 2 commits into from
Closed

removed default port adjustment #2754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d806581
removed default port adjustment
chernser Feb 24, 2026
7879a7d
removed unused imports and duplicate tests
chernser Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 0.9.7

### Breaking changes
- [jdbc] Default port is not added anymore - now port should be explicitly set. Previously `http://localhost` was transformed to `http://localhost:8123`.
Parsing URL with standard tools is not stable because `URI.create()` doesn't guarantee correct port number parsing.
Implementing more complex logic for minor case would add even more complexity. (https://github.com/ClickHouse/clickhouse-java/issues/2753)

## 0.9.6
Release is aimed to address potential security risk in one of the dependencies (see below). We strongly recommend to upgrade.

Expand Down
13 changes: 3 additions & 10 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.clickhouse.client.api.Client;
import com.clickhouse.client.api.ClientConfigProperties;
import com.clickhouse.client.api.http.ClickHouseHttpProto;
import com.clickhouse.data.ClickHouseDataType;
import com.clickhouse.jdbc.Driver;
import com.clickhouse.jdbc.DriverProperties;
Expand Down Expand Up @@ -148,20 +147,14 @@ public String getConnectionUrl() {
* @param ssl - if SSL protocol should be used
* @return URL without JDBC prefix
*/
private static String createConnectionURL(String url, boolean ssl) throws SQLException {
static String createConnectionURL(String url, boolean ssl) throws SQLException {
String adjustedURL = ssl && url.startsWith("http://")
? "https://" + url.substring(7)
: url;
try {
URI tmp = URI.create(adjustedURL);
String asciiString = tmp.toASCIIString();
if (tmp.getPort() < 0) {
String port = ssl || adjustedURL.startsWith("https")
? ":" + ClickHouseHttpProto.DEFAULT_HTTPS_PORT
: ":" + ClickHouseHttpProto.DEFAULT_HTTP_PORT;
asciiString += port;
}
return asciiString;
// URI may incorrectly parse port and logic based on port number is unstable.
return tmp.toASCIIString();
} catch (IllegalArgumentException iae) {
throw new SQLException("Failed to parse URL '" + url + "'", iae);
}
Expand Down
43 changes: 20 additions & 23 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/JdbcConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.clickhouse.client.api.Client;
import com.clickhouse.client.api.ClientConfigProperties;

import com.clickhouse.jdbc.DriverProperties;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
Expand All @@ -23,61 +22,59 @@
public class JdbcConfigurationTest {

private static final JdbcConfigurationTestData[] VALID_URLs = new JdbcConfigurationTestData[] {
new JdbcConfigurationTestData("jdbc:ch://localhost"),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost"),
new JdbcConfigurationTestData("jdbc:clickhouse:http://localhost"),
new JdbcConfigurationTestData("jdbc:clickhouse:https://localhost")
.withExpectedConnectionURL("https://localhost:8443"),
new JdbcConfigurationTestData("jdbc:ch://localhost:8123")
Copy link
Contributor

@mzitnik mzitnik Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only test here the happy path lets add some negative tests

.withExpectedConnectionURL("http://localhost:8123"),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123")
.withExpectedConnectionURL("http://localhost:8123"),
new JdbcConfigurationTestData("jdbc:clickhouse:http://unknown-host.local:8443")
.withExpectedConnectionURL("http://unknown-host.local:8443"),
new JdbcConfigurationTestData("jdbc:clickhouse:http://localhost:8443")
.withExpectedConnectionURL("http://localhost:8443"),
new JdbcConfigurationTestData("jdbc:clickhouse:https://localhost:8123")
.withExpectedConnectionURL("https://localhost:8123"),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8443")
.withAdditionalConnectionParameters(
Map.of(JdbcConfiguration.USE_SSL_PROP, "true"))
.withExpectedConnectionURL("https://localhost:8443"), // ssl should not be passed to client
new JdbcConfigurationTestData("jdbc:clickhouse://[::1]")
new JdbcConfigurationTestData("jdbc:clickhouse://[::1]:8123")
Copy link

@cursor cursor bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate test entries introduced during port migration

Low Severity

The newly added test entry at line 39 (jdbc:clickhouse://[::1]:8123) is an exact duplicate of the pre-existing entry at line 41. Similarly, the entry added at line 77 (jdbc:clickhouse://localhost:8123/?custom_key1=val1) duplicates the pre-existing entry at line 88. Both arose from mechanically replacing no-port test URLs with port-explicit versions without noticing identical entries already existed. The old no-port entries tested distinct scenarios; the replacements add no new coverage.

Additional Locations (1)

Fix in Cursor Fix in Web

.withExpectedConnectionURL("http://[::1]:8123"),
new JdbcConfigurationTestData("jdbc:clickhouse://[::1]:8123")
.withExpectedConnectionURL("http://[::1]:8123"),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8443")
.withExpectedConnectionURL("http://localhost:8443"),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/database")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/database")
.withAdditionalExpectedClientProperties(
Map.of("database", "database")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:42/database")
.withExpectedConnectionURL("http://localhost:42")
.withAdditionalExpectedClientProperties(
Map.of("database", "database")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/data-base")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/data-base")
.withAdditionalExpectedClientProperties(
Map.of("database", "data-base")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/data%20base")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/data%20base")
.withAdditionalExpectedClientProperties(
Map.of("database", "data base")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/data%2Fbase")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/data%2Fbase")
.withAdditionalExpectedClientProperties(
Map.of("database", "data/base")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/☺")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/☺")
.withAdditionalExpectedClientProperties(
Map.of("database", "☺")),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/db?custom_key1=val1&custom_key2=val2")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/db?custom_key1=val1&custom_key2=val2")
.withAdditionalExpectedClientProperties(
Map.of(
"database", "db",
"custom_key1", "val1",
"custom_key2", "val2"
)),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/db?custom_key1=val%201")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/db?custom_key1=val%201")
.withAdditionalExpectedClientProperties(
Map.of(
"database", "db",
"custom_key1", "val 1"
)),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost/?custom_key1=val1")
.withAdditionalExpectedClientProperties(
Map.of(
"custom_key1", "val1"
)),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost?custom_key1=val1")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123/?custom_key1=val1")
.withAdditionalExpectedClientProperties(
Map.of(
"custom_key1", "val1"
Expand All @@ -94,12 +91,12 @@ public class JdbcConfigurationTest {
Map.of(
"custom_key1", "val1"
)),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost?custom_key1=☺")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123?custom_key1=☺")
.withAdditionalExpectedClientProperties(
Map.of(
"custom_key1", "☺"
)),
new JdbcConfigurationTestData("jdbc:clickhouse://localhost?custom_key1=val1,val2")
new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8123?custom_key1=val1,val2")
.withAdditionalExpectedClientProperties(
Map.of(
"custom_key1", "val1,val2"
Expand Down
Loading