ClickHouse / ClickHouse/clickhouse-java

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

Open
#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 16h
Merged PRs (30d)
28

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

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.