Fix client-v2: escape special characters in scalar String query parameters - #2962
Merged
Merged
Conversation
…eters
A scalar {name:String} query parameter is parsed by the server with
deserializeTextEscaped, so a raw tab (0x09) or newline (0x0a) was treated as a
field delimiter (failing with BAD_QUERY_PARAMETER) and a raw backslash started
an escape sequence (silently corrupting the value, e.g. C:\temp became
C:<tab>emp). DataTypeConverter.convertParameterToString now escapes the
backslash, tab and newline in a scalar String parameter so any value round-trips
through the param_<name> interface; every other character the server reads
verbatim (carriage return, NUL, the single quote, ...) is left unchanged, so
Identifier values and pre-formatted Array/Map literals passed as a String still
round-trip. The JDBC driver (jdbc-v2) inlines parameters as SQL literals and
already escaped the backslash and single quote; it is unchanged and covered by a
new regression test.
Fixes: #2781
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
Member
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 834be8f. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes client-v2 named scalar {p:String} query parameters so values containing literal tab/newline/backslash round-trip correctly through ClickHouse’s param_<name> interface (parsed as TSV “escaped” text via deserializeTextEscaped).
Changes:
- Escape scalar
CharSequenceparameter values inclient-v2(\→\\, tab →\t, newline →\n) while leaving other scalars and all container formatting unchanged. - Add unit + integration coverage to lock correct round-trip behavior for special-character strings in
client-v2, plus a JDBCsetString()characterization/regression test. - Document the compatibility contract in
docs/features.mdand add a changelog entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
client-v2/src/main/java/com/clickhouse/client/api/internal/DataTypeConverter.java |
Escapes scalar string parameters for the HTTP param_* path while keeping container formatting as-is. |
client-v2/src/test/java/com/clickhouse/client/api/internal/DataTypeConverterTest.java |
Unit tests for scalar-string escaping vs. “do not escape” contrast cases. |
client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java |
Integration round-trip test for special-character scalar {p:String} parameters with a sentinel column. |
jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java |
Integration test ensuring PreparedStatement#setString round-trips special characters when inlined as SQL literals. |
docs/features.md |
Extends the documented stability contract for string query parameters to include backslash/tab/newline handling. |
CHANGELOG.md |
Adds a bug-fix entry describing the corrected behavior and scope. |
The fix changes client-v2 behavior only; jdbc-v2 is unchanged (its parameter path already escapes backslash and single quote), so tag the CHANGELOG entry [client-v2] to match the repo convention and the entry's own text, per Copilot review on #2962.
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
|
chernser
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2781.
A scalar
Stringquery parameter is parsed by the server (verified against ClickHouse26.5.1) withdeserializeTextEscaped— the TSV "escaped" text format — behind theparam_<name>interface.client-v2sent the value raw, so:0x09) or newline (0x0a) was read as a TSV field/row delimiter, aborting the parse withBAD_QUERY_PARAMETER: ... isn't parsed completely: only N of M bytes was parsed;C:\tempcame back asC:<tab>emp).The fix escapes a scalar
Stringparameter into the exact form that reader reverses. Empirically,deserializeTextEscapedonly treats backslash, tab and newline as structural — carriage return, NUL, the single quote, bell, vtab and UTF-8 multi-byte sequences all round-trip verbatim — so only those three characters are escaped. Escaping the minimal set is important: it leaves any value that needs no escaping completely untouched, soIdentifierparameter values (which the server backtick-escapes itself and must not be pre-escaped by the client) and pre-formattedArray/Mapliterals passed as aString(the pre-#2897workaround, still exercised bytestQueryParamsWithArrays) continue to round-trip.The formatting lives in the value-converter (
DataTypeConverter), per the repo's architecture convention that parameter formatting belongs on the converters, not in transport code.jdbc-v2was verified already-correct and is unchanged. ItsPreparedStatementinlines parameters as SQL string literals viaSQLUtils.escapeSingleQuotes, which already escapes the backslash and single quote (a tab/newline is a valid literal character inside'...'). A characterization test locks that behavior.Changes
client-v2DataTypeConverter.convertParameterToString(Object): a scalarCharSequencevalue is now escaped via a new privateescapeStringParameterhelper (escapes\, tab, newline; lazily allocates only when something needs escaping). Non-text scalars (numbers, temporals,null) and containers are unchanged.docs/features.md: extended the client-v2 String-parameter round-trip contract to cover backslash/tab/newline.CHANGELOG.md: added a Bug Fixes entry.client-v2DataTypeConverterTest— unit cases for tab/newline/backslash/combination that are escaped, plus contrast cases (empty string, carriage return, single quote, a pre-formattedArrayliteral, anIdentifier-style value) that stay unescaped.client-v2QueryTests#testScalarStringQueryParamsRoundTrip— end-to-end round-trip throughClient.queryAll(sql, params)against a live server (tab/newline/backslash/CR/quote/NUL/empty/UTF-8), with a trailing sentinel column to catch a mis-counted parse.jdbc-v2PreparedStatementTest#testSetStringWithSpecialCharacters— round-trip throughPreparedStatement.setString.Test
New unit + integration tests fail on
mainand pass with the fix:BAD_QUERY_PARAMETERon the tab/newline cases, and the unit test's tab/newline/backslash cases fail with the raw (unescaped) value.testQueryParamsWithArrays,testContainerQueryParamsQuoteInnerValuesandtestExecuteQueryParam(Identifierparam) tests still pass — confirming no regression for container or identifier parameters.Pre-PR validation gate
client.query("SELECT {p:String}", {p:"a\tb"})→BAD_QUERY_PARAMETERonmain)deserializeTextEscapedmisinterprets)@DataProvider; CHANGELOG +docs/features.mdupdated)Client.queryAll)