diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e55a893..ca4088730 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,18 @@ ### Bug Fixes +- **[client-v2]** Fixed scalar `String` query parameters containing a tab (`0x09`), newline + (`0x0a`) or backslash being mishandled through the server's `param_` interface. A `{name:String}` + parameter value is parsed by the server with `deserializeTextEscaped`, which treated a raw tab or + newline as a field delimiter (failing the query with `BAD_QUERY_PARAMETER: ... isn't parsed completely`) + and a raw backslash as the start of an escape sequence (silently corrupting the value, e.g. `C:\temp` + became `C:emp`). `Client.query(sql, params, ...)` now escapes the backslash, tab and newline in a + scalar `String` parameter so any value round-trips; every other character the server reads verbatim β€” + including the single quote and carriage return β€” is left unchanged, so `Identifier` values and + pre-formatted `Array`/`Map` literals passed as a `String` still round-trip. The JDBC driver (`jdbc-v2`), + which inlines parameters as SQL literals and already escaped the backslash and single quote, is + unchanged and covered by a new regression test. (https://github.com/ClickHouse/clickhouse-java/issues/2781) + - **[client-v2]** Fixed binary array decoding for nullable element types so `Array(Nullable(Float64))` and similar columns now return boxed arrays such as `Double[]` instead of `Object[]`. This keeps null-supporting arrays aligned with their element type while preserving the existing `Object[]` fallback for Variant/Dynamic/Geometry arrays. (https://github.com/ClickHouse/clickhouse-java/issues/2846) - **[client-v2]** Fixed `Float32`/`Float64` columns throwing `ClassCastException` when a value of a diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java index acd1372b6..aea3578ad 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java @@ -249,11 +249,57 @@ public String convertParameterToString(Object value) { if (isParameterContainer(value)) { return convertParameterContainer(value); } - // Scalars (and null) are passed through unquoted: the server reads a scalar parameter value - // verbatim, so quoting it here would break parsing (e.g. Date, numbers, Identifier). + if (value instanceof CharSequence) { + // A scalar String parameter is read by the server with deserializeTextEscaped: a raw tab + // (0x09) or newline (0x0a) is treated as a field delimiter (failing the query with + // BAD_QUERY_PARAMETER) and a raw backslash starts an escape sequence (silently corrupting + // the value). Escape those three characters so any String value round-trips. + return escapeStringParameter((CharSequence) value); + } + // Other scalars (and null) have no escapable characters and are read verbatim by the server, + // so they are passed through unquoted (e.g. Date, numbers, Identifier). return String.valueOf(value); } + /** + * Escapes a scalar {@code String} parameter value so it survives the server's + * {@code param_} interface, which parses a {@code {name:String}} value with + * {@code deserializeTextEscaped}. Only the three characters that reader treats as structural are + * escaped: the backslash (it introduces an escape sequence, so a raw one silently corrupts the + * value), and the tab and newline (TSV field/row delimiters, so a raw one aborts the parse with + * {@code BAD_QUERY_PARAMETER}). Every other byte - carriage return, NUL, the single quote, UTF-8 + * multi-byte sequences, etc. - is read verbatim by the server, so it is emitted unchanged. + * Escaping only this minimal set leaves a value that needs no escaping completely untouched, so + * {@code Identifier} values (which the server backtick-escapes itself) and pre-formatted + * {@code Array}/{@code Map} literals passed as a {@code String} still round-trip. + */ + private String escapeStringParameter(CharSequence value) { + final int len = value.length(); + StringBuilder sb = null; // created lazily; a value with nothing to escape allocates nothing + for (int i = 0; i < len; i++) { + char c = value.charAt(i); + String escaped; + switch (c) { + case '\\': escaped = "\\\\"; break; + case '\t': escaped = "\\t"; break; + case '\n': escaped = "\\n"; break; + default: escaped = null; + } + if (escaped == null) { + if (sb != null) { + sb.append(c); + } + } else { + if (sb == null) { + sb = new StringBuilder(len + 8); + sb.append(value, 0, i); + } + sb.append(escaped); + } + } + return sb == null ? value.toString() : sb.toString(); + } + private boolean isParameterContainer(Object value) { return value instanceof Collection || value instanceof Map || (value != null && value.getClass().isArray()); diff --git a/client-v2/src/test/java/com/clickhouse/client/api/internal/DataTypeConverterTest.java b/client-v2/src/test/java/com/clickhouse/client/api/internal/DataTypeConverterTest.java index ce2573581..e8d7a1370 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/internal/DataTypeConverterTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/api/internal/DataTypeConverterTest.java @@ -178,6 +178,23 @@ public static Object[][] queryParameters() { {new BigDecimal("1.50"), "1.50"}, {null, "null"}, + // --- Scalar String special characters: the server parses a {name:String} value with + // deserializeTextEscaped, so the client escapes the three characters that reader treats + // as structural. A raw tab or newline previously failed with BAD_QUERY_PARAMETER and a + // raw backslash silently corrupted the value. --- + {"hello\tworld", "hello\\tworld"}, + {"line1\nline2", "line1\\nline2"}, + {"a\\tb", "a\\\\tb"}, + {"x\ty\nz\\w", "x\\ty\\nz\\\\w"}, + // Contrast: characters the server reads verbatim are NOT escaped. Over-escaping (e.g. the + // single quote) would corrupt values that are already valid server text, such as a + // pre-formatted Array literal or an Identifier passed as a String. + {"", ""}, // empty string: boundary + {"a\rb", "a\rb"}, // carriage return: not a delimiter + {"O'Brien", "O'Brien"}, // single quote: not a delimiter + {"['COLLATIONS','ENGINES']", "['COLLATIONS','ENGINES']"}, // pre-formatted Array literal + {"`db`.`tbl`", "`db`.`tbl`"}, // Identifier-style value + // --- Array/List with String/temporal leaves: single-quoted so the server's array // text parser accepts them (previously emitted e.g. [2026-05-13] -> HTTP 400). --- {Arrays.asList(LocalDate.of(2026, 5, 13), LocalDate.of(2026, 5, 14)), "['2026-05-13','2026-05-14']"}, diff --git a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java index e8230716c..b0e3e121a 100644 --- a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java @@ -1741,6 +1741,40 @@ public void testContainerQueryParamsQuoteInnerValues(String clickHouseType, Obje Assert.assertEquals(records.get(0).getString("v"), expected); } + @DataProvider(name = "scalarStringQueryParameters") + Object[][] scalarStringQueryParameters() { + // Scalar {p:String} values that must round-trip byte-for-byte through the server's + // param_ interface. A raw tab/newline previously failed with BAD_QUERY_PARAMETER and a + // raw backslash was silently corrupted; the client now emits the TSV "escaped" form. + return new Object[][]{ + {"plain value"}, + {""}, // empty string: boundary + {"hello\tworld"}, // tab: rejected with BAD_QUERY_PARAMETER before the fix + {"line1\nline2"}, // newline: rejected with BAD_QUERY_PARAMETER before the fix + {"C:\\temp"}, // backslash before 't': silently corrupted to a tab before the fix + {"carriage\rreturn"}, // CR: read verbatim, must survive round-trip + {"quote'inside"}, // single quote: read verbatim, must survive round-trip + {"mix\tof\nall\\the'specials"}, + {"unicode Γ©δΈ­πŸ˜€"}, // multi-byte UTF-8 must be untouched + {"a\u0000b"}, // NUL: read verbatim, must survive round-trip + }; + } + + @Test(groups = {"integration"}, dataProvider = "scalarStringQueryParameters") + public void testScalarStringQueryParamsRoundTrip(String value) { + // A scalar {p:String} parameter containing a tab, newline or backslash must round-trip + // unchanged. Before the fix the value was sent raw, so the server's deserializeTextEscaped + // treated a tab/newline as a field delimiter (BAD_QUERY_PARAMETER) or read a backslash as an + // escape sequence, corrupting the value. The trailing constant column detects a parameter + // parse that consumes the wrong number of bytes. + Map params = Collections.singletonMap("p", value); + List records = client.queryAll("SELECT {p:String} AS v, 'END' AS tail", params); + + Assert.assertEquals(records.size(), 1); + Assert.assertEquals(records.get(0).getString("v"), value); + Assert.assertEquals(records.get(0).getString("tail"), "END"); + } + @Test(groups = {"integration"}) public void testExecuteQueryParam() throws ExecutionException, InterruptedException, TimeoutException { diff --git a/docs/features.md b/docs/features.md index b720d3931..e69a54460 100644 --- a/docs/features.md +++ b/docs/features.md @@ -39,7 +39,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t Compatibility-sensitive traits: - Named parameter typing is part of the contract: placeholders are written as `{name:Type}` and the supplied value must match the expected ClickHouse textual representation for that type. -- String query parameters are expected to round-trip correctly for ordinary text, Unicode, slashes, dashes, and leading or trailing spaces. +- String query parameters are expected to round-trip correctly for ordinary text, Unicode, slashes, dashes, leading or trailing spaces, and characters the server's escaped-text parameter parser treats as structural β€” a backslash, tab, or newline β€” which the client escapes before sending. - Runtime authentication changes are compatibility-sensitive: after `updateUserAndPassword()`, `updateAccessToken()`, or `updateBearerToken()`, subsequent requests from the same `Client` are expected to use the updated credentials. The authentication method itself is fixed at construction time; calling a runtime updater that does not match the configured method throws `ClientMisconfigurationException`. - String escaping behavior in `SQLUtils` is compatibility-sensitive: `enquoteLiteral()` uses SQL-style doubled single quotes, while `escapeSingleQuotes()` escapes both backslashes and single quotes with backslashes. - Identifier quoting behavior is stable API for helper callers: identifiers are double-quoted, embedded double quotes are doubled, and optional quoting keeps simple identifiers unchanged. diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java index 706e95815..14c19f7a9 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java @@ -175,6 +175,36 @@ public void testSetString() throws Exception { } } + @DataProvider(name = "specialCharacterStrings") + Object[][] specialCharacterStrings() { + return new Object[][] { + {"plain value"}, + {"tab\tinside"}, + {"newline\ninside"}, + {"C:\\temp"}, + {"quote'inside"}, + {"mixed\t'quote'\nand\\backslash"}, + {"unicode Γ©δΈ­πŸ˜€"}, + }; + } + + @Test(groups = { "integration" }, dataProvider = "specialCharacterStrings") + public void testSetStringWithSpecialCharacters(String value) throws Exception { + // A String bound with setString is inlined into the SQL as a single-quoted literal, so the + // backslash and single quote must be escaped (a tab or newline is a valid literal character). + // The value must round-trip byte-for-byte. + try (Connection conn = getJdbcConnection()) { + try (PreparedStatement stmt = conn.prepareStatement("SELECT ?")) { + stmt.setString(1, value); + try (ResultSet rs = stmt.executeQuery()) { + assertTrue(rs.next()); + assertEquals(rs.getString(1), value); + assertFalse(rs.next()); + } + } + } + } + @Test(groups = { "integration" }) public void testSetBytes() throws Exception { // see com.clickhouse.jdbc.JdbcDataTypeTests.testStringsUsedAsBytes