From 1f90772ecfe0067a298de47e1586433925eb7ffa Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:33:39 +0000 Subject: [PATCH] Fix clickhouse-jdbc: skip heredoc literals when extracting ? placeholders JdbcParameterizedQuery.parse() skipped quoted strings and --/ comments but had no notion of a heredoc literal ($$...$$ / $tag$...$tag$), so the contents of a heredoc were parsed as SQL: a '?' inside one was counted as a bind parameter, a ';' was rejected as a multi-statement query, a '\'' broke the scan with "Missing quote: '", and a ':' could be mistaken for the delimiter of the ? : ternary operator, silently dropping a real placeholder. A heredoc is now skipped as an opaque token, both in the placeholder scan and in the ternary lookahead (including inside brackets, where a bracket or quote in a heredoc body used to end the enclosing bracket or string). A '$' only opens a heredoc when it does not continue an identifier (a$b, a$x$), its tag contains word characters only, and a matching closing tag exists - otherwise it stays an ordinary character, matching the server lexer. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/3035 --- CHANGELOG.md | 7 + .../jdbc/JdbcParameterizedQuery.java | 131 +++++++++++++++++- .../jdbc/ClickHousePreparedStatementTest.java | 15 ++ .../jdbc/JdbcParameterizedQueryTest.java | 55 ++++++++ 4 files changed, 207 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..062dcb0d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,13 @@ ### Bug Fixes +- **[clickhouse-jdbc]** Fixed a heredoc string literal (`$$...$$` / `$tag$...$tag$`) being scanned as SQL when + `PreparedStatement` placeholders are extracted, so a `?` inside a heredoc was counted as a bind parameter, a `;` + was rejected as a multi-statement query, a `'` broke the scan with `Missing quote: '`, and a `:` could be read as + the delimiter of a ternary operator and drop a real placeholder. A heredoc is now skipped as an opaque literal, so + a statement such as `select $$a?b$$ as s, ? as n` has exactly one parameter. A dollar sign that does not open a + heredoc (an identifier such as `a$x$`, or an unterminated `$$`) keeps its previous meaning. + (https://github.com/ClickHouse/clickhouse-java/issues/3035) - **[client-v2]** Fixed LZ4 input streams not closing their underlying HTTP response stream. Closing an LZ4 stream returned by `QueryResponse.getInputStream()` now releases the wrapped transport stream, including after a partial read. (https://github.com/ClickHouse/clickhouse-java/issues/2985) diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java index 40c9192e4..e9823edcc 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java @@ -1,7 +1,9 @@ package com.clickhouse.jdbc; +import java.util.ArrayDeque; import java.util.Collection; import java.util.Collections; +import java.util.Deque; import java.util.Iterator; import com.clickhouse.client.ClickHouseConfig; @@ -41,7 +43,7 @@ protected String parse() { if (ClickHouseUtils.isQuote(ch)) { i = ClickHouseUtils.skipQuotedString(originalQuery, i, len, ch) - 1; } else if (ch == '?') { - int idx = ClickHouseUtils.skipContentsUntil(originalQuery, i + 2, len, '?', ':'); + int idx = skipUntilTernaryDelimiter(originalQuery, i + 2, len); if (idx < len && originalQuery.charAt(idx - 1) == ':' && originalQuery.charAt(idx) != ':' && originalQuery.charAt(idx - 2) != ':') { i = idx - 1; @@ -59,6 +61,8 @@ protected String parse() { i = ClickHouseUtils.skipSingleLineComment(originalQuery, i + 2, len) - 1; } else if (ch == '/' && nextCh == '*') { i = ClickHouseUtils.skipMultiLineComment(originalQuery, i + 2, len) - 1; + } else if (ch == '$') { + i = skipHeredoc(originalQuery, i, len) - 1; } } } @@ -66,6 +70,131 @@ protected String parse() { return partIndex < len ? originalQuery.substring(partIndex, len) : null; } + /** + * Skips quoted strings, brackets, comments and heredocs until seeing a + * {@code ?} or {@code :}, the delimiters of a ternary operator. Same as + * {@link ClickHouseUtils#skipContentsUntil(String, int, int, char...)} except + * that a heredoc is skipped as an opaque token, so that its contents cannot be + * mistaken for a ternary operator's delimiter. + * + * @param query non-null string to scan + * @param startIndex start index + * @param len end index, usually length of the given string + * @return index next to the delimiter, or {@code len} when there is none + */ + private static int skipUntilTernaryDelimiter(String query, int startIndex, int len) { + for (int i = startIndex; i < len; i++) { + char ch = query.charAt(i); + if (ch == '?' || ch == ':') { + return i + 1; + } else if (ClickHouseUtils.isQuote(ch)) { + i = ClickHouseUtils.skipQuotedString(query, i, len, ch) - 1; + } else if (ClickHouseUtils.isOpenBracket(ch)) { + i = skipBrackets(query, i, len, ch) - 1; + } else if (i + 1 < len) { + char nextCh = query.charAt(i + 1); + if (ch == '-' && nextCh == ch) { + i = ClickHouseUtils.skipSingleLineComment(query, i + 2, len) - 1; + } else if (ch == '/' && nextCh == '*') { + i = ClickHouseUtils.skipMultiLineComment(query, i + 2, len) - 1; + } else if (ch == '$') { + i = skipHeredoc(query, i, len) - 1; + } + } + } + + return len; + } + + /** + * Skips brackets and the content inside. Same as + * {@link ClickHouseUtils#skipBrackets(String, int, int, char)} except that a + * heredoc is skipped as an opaque token, so that a bracket or quote inside it + * does not end the enclosing bracket or string. + * + * @param query non-null string to scan + * @param startIndex start index, optionally index of the opening bracket + * @param len end index, usually length of the given string + * @param bracket the opening bracket + * @return index next to the matching close bracket + * @throws IllegalArgumentException when the bracket is not closed + */ + private static int skipBrackets(String query, int startIndex, int len, char bracket) { + char closeBracket = ClickHouseUtils.getCloseBracket(bracket); + + Deque stack = new ArrayDeque<>(); + for (int i = startIndex + (startIndex < len && query.charAt(startIndex) == bracket ? 1 : 0); i < len; i++) { + char ch = query.charAt(i); + if (ClickHouseUtils.isQuote(ch)) { + i = ClickHouseUtils.skipQuotedString(query, i, len, ch) - 1; + } else if (ClickHouseUtils.isOpenBracket(ch)) { + stack.push(closeBracket); + closeBracket = ClickHouseUtils.getCloseBracket(ch); + } else if (ch == closeBracket) { + if (stack.isEmpty()) { + return i + 1; + } else { + closeBracket = stack.pop(); + } + } else if (i + 1 < len) { + char nextCh = query.charAt(i + 1); + if (ch == '-' && nextCh == ch) { + i = ClickHouseUtils.skipSingleLineComment(query, i + 2, len) - 1; + } else if (ch == '/' && nextCh == '*') { + i = ClickHouseUtils.skipMultiLineComment(query, i + 2, len) - 1; + } else if (ch == '$') { + i = skipHeredoc(query, i, len) - 1; + } + } + } + + throw new IllegalArgumentException( + ClickHouseUtils.format("Missing '%s' for '%s' at position %d", closeBracket, bracket, startIndex)); + } + + /** + * Skips a heredoc (dollar quoted string) like {@code $$...$$} or + * {@code $tag$...$tag$}, where the tag may only contain word characters. When + * there is no heredoc at {@code startIndex} the dollar sign is treated as an + * ordinary character, because it is also a valid identifier character: a dollar + * sign that follows a word character continues an identifier (e.g. {@code a$b} + * or {@code a$x$}) instead of opening a heredoc, and a dollar sign without a + * matching closing tag does not open one either. + * + * @param query non-null string to scan + * @param startIndex index of the dollar sign that may open a heredoc + * @param len end index, usually length of the given string + * @return index next to the closing tag, or {@code startIndex + 1} when there is + * no heredoc + */ + private static int skipHeredoc(String query, int startIndex, int len) { + if (startIndex > 0 && isWordChar(query.charAt(startIndex - 1))) { + return startIndex + 1; + } + + int tagEndIndex = query.indexOf('$', startIndex + 1); + if (tagEndIndex < 0 || tagEndIndex >= len) { + return startIndex + 1; + } + + for (int i = startIndex + 1; i < tagEndIndex; i++) { + if (!isWordChar(query.charAt(i))) { + return startIndex + 1; + } + } + + String tag = query.substring(startIndex, tagEndIndex + 1); + int closingTagIndex = query.indexOf(tag, tagEndIndex + 1); + if (closingTagIndex < 0 || closingTagIndex + tag.length() > len) { + return startIndex + 1; + } + return closingTagIndex + tag.length(); + } + + private static boolean isWordChar(char ch) { + return ch == '_' || (ch >= '0' && ch <= '9') || (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z'); + } + @Override public void apply(StringBuilder builder, Collection params) { if (!hasParameter()) { diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java index 3226471b2..f44652bc4 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java @@ -665,6 +665,21 @@ public void testReadWriteString() throws SQLException { } } + @Test(groups = "integration") + public void testQueryWithHeredocLiteral() throws SQLException { + try (ClickHouseConnection conn = newConnection(new Properties()); + PreparedStatement ps = conn.prepareStatement("select $$a?b$$ as s, ? as n")) { + Assert.assertEquals(ps.getParameterMetaData().getParameterCount(), 1); + ps.setInt(1, 42); + try (ResultSet rs = ps.executeQuery()) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getString(1), "a?b"); + Assert.assertEquals(rs.getInt(2), 42); + Assert.assertFalse(rs.next()); + } + } + } + @Test(groups = "integration") public void testInsertQueryDateTime64() throws SQLException { try (ClickHouseConnection conn = newConnection(new Properties()); diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/JdbcParameterizedQueryTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/JdbcParameterizedQueryTest.java index b6111a431..5bc18e61f 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/JdbcParameterizedQueryTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/JdbcParameterizedQueryTest.java @@ -7,6 +7,7 @@ import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class JdbcParameterizedQueryTest { @@ -66,4 +67,58 @@ public void testParseJdbcQueries() { q.apply(builder, 1, new StringBuilder("Int8")); Assert.assertEquals(builder.toString(), "select 1::Int8"); } + + @Test(groups = "unit", dataProvider = "heredocQueryProvider") + public void testParseQueriesWithHeredoc(String sql, int parameters, String substituted) { + JdbcParameterizedQuery q = JdbcParameterizedQuery.of(config, sql); + Assert.assertEquals(q.getParameters().size(), parameters, "Parameter count mismatch for: " + sql); + + StringBuilder builder = new StringBuilder(); + q.apply(builder, "X", "Y"); + Assert.assertEquals(builder.toString(), substituted); + } + + @DataProvider(name = "heredocQueryProvider") + private static Object[][] getHeredocQueries() { + return new Object[][] { + // a heredoc is an opaque literal, so its contents are not parameters + { "select $$a?b$$, ?", 1, "select $$a?b$$, X" }, + { "select $tag$ ? $tag$, ?", 1, "select $tag$ ? $tag$, X" }, + { "select $1$?$1$, ?", 1, "select $1$?$1$, X" }, + { "select $$?$$", 0, "select $$?$$" }, + { "select $$$$, ?", 1, "select $$$$, X" }, + { "select $$a$b$$, ?", 1, "select $$a$b$$, X" }, + { "select $$-- ?$$, ?", 1, "select $$-- ?$$, X" }, + { "select $$/* ? $$, ?", 1, "select $$/* ? $$, X" }, + { "select $$it's$$, ?", 1, "select $$it's$$, X" }, + { "select $$a;b$$, ?", 1, "select $$a;b$$, X" }, + { "select ?, $$a:b$$", 1, "select X, $$a:b$$" }, + { "select ?, lower($$it's$$)", 1, "select X, lower($$it's$$)" }, + { "select ?, position($$)$$, $$:$$)", 1, "select X, position($$)$$, $$:$$)" }, + { "select 1 ? $$a:b$$ : 2, ?", 1, "select 1 ? $$a:b$$ : 2, X" }, + { "select $_a1$ ? $_a1$, ?", 1, "select $_a1$ ? $_a1$, X" }, + { "$$?$$ as v, ?", 1, "$$?$$ as v, X" }, + { "insert into t values ($$a?b$$, ?)", 1, "insert into t values ($$a?b$$, X)" }, + // a dollar sign that does not open a heredoc stays an ordinary character + { "select ? as a$x$, ? as b$x$", 2, "select X as a$x$, Y as b$x$" }, + { "select ? as a$b, ?", 2, "select X as a$b, Y" }, + { "select $$ ? , ?", 2, "select $$ X , Y" }, + { "select '$$?$$' as v, ?", 1, "select '$$?$$' as v, X" }, + { "select -- $$?$$\n?", 1, "select -- $$?$$\nX" }, + { "select /* $$?$$ */ ?", 1, "select /* $$?$$ */ X" }, + { "select 1 ? 'a' : 'b', ?", 1, "select 1 ? 'a' : 'b', X" }, + }; + } + + @Test(groups = "unit") + public void testParseInvalidQueriesWithHeredoc() { + Assert.assertThrows(IllegalArgumentException.class, + () -> JdbcParameterizedQuery.of(config, "select $$a$$; select ?")); + Assert.assertThrows(IllegalArgumentException.class, + () -> JdbcParameterizedQuery.of(config, "select $$a$$ as v; select 2")); + Assert.assertThrows(IllegalArgumentException.class, + () -> JdbcParameterizedQuery.of(config, "select ?, f($$a$$")); + Assert.assertThrows(IllegalArgumentException.class, + () -> JdbcParameterizedQuery.of(config, "select ?, 'a")); + } }