ClickHouse / ClickHouse/clickhouse-java

[client-v2, jdbc-v2] SQLUtils.enquoteLiteral / enquoteIdentifier do not escape backslashes, corrupting or breaking SQL

Open Beginner friendly
#3,063 0 comments 0 reactions 0 assignees View on GitHub
area:general bug client-api-v2 jdbc jdbc-v2
Dominant language
Java
Stars
1.6k
Forks
636
Avg merge
2d 23h
Merged PRs (30d)
29

Description

## Description

`com.clickhouse.client.api.sql.SQLUtils.enquoteLiteral(String)` escapes **only** the single quote, by doubling it:

```java
// client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java:14-19
public static String enquoteLiteral(String str) {
if (str == null) { throw new IllegalArgumentException("Input string cannot be null"); }
return "'" + str.replace("'", "''") + "'";
}
```

ClickHouse treats the backslash as an escape character inside single-quoted strings, so a value containing a backslash is either **silently corrupted** (`\t` becomes a TAB) or **breaks the statement** (a value ending in `\` escapes the closing quote → `Code: 62 ... Single quoted string is not closed (SYNTAX_ERROR)`).

The same defect exists in `enquoteIdentifier` (`SQLUtils.java:30-38`), which only doubles `"` — ClickHouse also honours backslash escapes inside double-quoted identifiers.

Note the inconsistency inside the very same class: `SQLUtils.escapeSingleQuotes` (line 136) *does* get it right —

```java
public static String escapeSingleQuotes(String x) {
return x.replace("\\", "\\\\").replace("'", "\\'");
}
```

so `PreparedStatementImpl.encodeObject` (which uses `escapeSingleQuotes`) is safe, while `enquoteLiteral` is not. There are two escaping paths and only one of them is correct.

### Affected surfaces

1. **`java.sql.Statement.enquoteLiteral(String)`** — `jdbc-v2/.../StatementImpl.java:508` delegates straight to `SQLUtils.enquoteLiteral`. This is a standard JDBC 4.3 API that callers are told to use to build safe SQL.
2. **`java.sql.Statement.enquoteNCharLiteral(String)`** — `StatementImpl.java:527`, same delegation.
3. **`java.sql.Statement.enquoteIdentifier(String, boolean)`** — `StatementImpl.java:513`.
4. **`DatabaseMetaData.getColumns(...)`** — `jdbc-v2/.../metadata/DatabaseMetaDataImpl.java:1101-1103` builds its `system.columns` query with `SQLUtils.enquoteLiteral` on the caller-supplied `schemaPattern` / `tableNamePattern` / `columnNamePattern`. No API misuse is needed here: a pattern containing a backslash — which is the JDBC-standard escape character for `_` and `%` in metadata patterns — makes the driver's own internal query fail with a syntax error.

Existing coverage (`client-v2/src/test/java/com/clickhouse/client/api/sql/SQLUtilsTest.java` and `jdbc-v2/.../StatementTest.testEnquoteLiteral`) exercises only quote characters, so the gap is not caught.

## ClickHouse server version

`26.7.3.19` (official build), reached over HTTP at `localhost:8123`. Verified against a running server, not code analysis alone.

## Reproduction

`jdbc-v2/src/test/java/com/clickhouse/jdbc/EnquoteBackslashTest.java`:

```java
package com.clickhouse.jdbc;

import org.testng.Assert;
import org.testng.annotations.Test;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Properties;

public class EnquoteBackslashTest {

private static final String URL = "jdbc:ch:http://localhost:8123/default";

private Connection conn() throws SQLException {
Properties p = new Properties();
p.setProperty("user", "default");
p.setProperty("password", "");
return new ConnectionImpl(URL, p);
}

@Test
public void silentCorruption() throws Exception {
try (Connection c = conn(); Statement stmt = c.createStatement()) {
String value = "path like C:\\temp x"; // one real backslash
String quoted = stmt.enquoteLiteral(value);
try (ResultSet rs = stmt.executeQuery("SELECT " + quoted + " AS v")) {
Assert.assertTrue(rs.next());
Assert.assertEquals(rs.getString("v"), value);
}
}
}

@Test
public void brokenStatement() throws Exception {
try (Connection c = conn(); Statement stmt = c.createStatement()) {
String value = "ends with backslash\\";
try (ResultSet rs = stmt.executeQuery("SELECT " + stmt.enquoteLiteral(value))) {
Assert.assertTrue(rs.next());
Assert.assertEquals(rs.getString(1), value);
}
}
}

@Test
public void identifierCorruption() throws Exception {
try (Connection c = conn(); Statement stmt = c.createStatement()) {
String ident = "col\\tname"; // backslash + 't'
try (ResultSet rs = stmt.executeQuery("SELECT 1 AS " + stmt.enquoteIdentifier(ident, true))) {
Assert.assertTrue(rs.next());
Assert.assertEquals(rs.getMetaData().getColumnLabel(1), ident);
}
}
}

@Test
public void metadataPatternBroken() throws Exception {
try (Connection c = conn()) {
try (ResultSet rs = c.getMetaData().getColumns(null, "default", "tbl\\", "%")) {
while (rs.next()) { }
}
}
}
}
```

Run with:

```
mvn -pl jdbc-v2 test -Dtest=EnquoteBackslashTest
```

### Expected

All four pass: the literal round-trips unchanged, the identifier keeps its backslash, and `getColumns` returns an (empty) result set.

### Actual — all four fail

```
Tests run: 4, Failures: 4, Errors: 0, Skipped: 0

silentCorruption:
expected [path like C:\temp x] but found [path like C:emp x]
(the \t was consumed as a TAB escape; length() returns 18 instead of 19)

brokenStatement:
java.sql.SQLException: Code: 62. DB::Exception: Single quoted string is not closed:
Syntax error: failed at position 8 ('ends with backslash\'): 'ends with backslash\'.
(SYNTAX_ERROR) (version 26.7.3.19 (official build))

identifierCorruption:
expected [col\tname] but found [colname]

metadataPatternBroken:
java.sql.SQLException: Code: 62. DB::Exception: Single quoted string is not closed:
Syntax error: failed at position 2827 (' ORDER BY TABLE_SCHEM, TABLE_NAME, ORDINAL_POSITION)
(SYNTAX_ERROR) (version 26.7.3.19 (official build))
```

(`` above is a literal 0x09 byte in the real output.)

## Suggested fix

Escape the backslash before the quote in `client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java`:

- `enquoteLiteral` (line 14): escape `\` to `\\` first, then handle `'`. Reusing the already-correct `escapeSingleQuotes` (line 136) would collapse the two escaping paths into one and keep the class self-consistent.
- `enquoteIdentifier` (line 30): likewise escape `\` to `\\` before doubling `"`.

Worth extending `SQLUtilsTest`'s data providers with backslash cases (embedded `\t`, trailing `\`, `\\`) so the gap stays closed.

## Link

Same class of bug reported for clickhouse-connect: https://github.com/ClickHouse/clickhouse-connect/issues/975 (SQLAlchemy DDL rendered `COMMENT` / `DEFAULT` literals through a generic string type that only doubles quotes, leaving backslashes unescaped).

Contributor guide

Open the contributing guide

Research direction

Start with client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java and compare enquoteLiteral and enquoteIdentifier with escapeSingleQuotes. Run client-v2/src/test/java/com/clickhouse/client/api/sql/SQLUtilsTest.java and jdbc-v2/.../EnquoteBackslashTest.java using the provided Maven command; done means literals and identifiers preserve embedded and trailing backslashes, and metadata patterns no longer fail.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, sql
Domain
api, databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.