ClickHouse / ClickHouse/clickhouse-java

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

Đang mở Phù hợp với người mới
#3,063 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
area:general bug client-api-v2 jdbc jdbc-v2
Ngôn ngữ chính
Java
Star
1.6k
Fork
636
Merge trung bình
2 ngày 23 giờ
Pull request đã merge (30 ngày)
29

Mô tả

## 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).

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu với client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java và so sánh enquoteLiteral và enquoteIdentifier với escapeSingleQuotes. Chạy client-v2/src/test/java/com/clickhouse/client/api/sql/SQLUtilsTest.java và jdbc-v2/.../EnquoteBackslashTest.java bằng lệnh Maven được cung cấp; được xem là hoàn tất khi các literal và identifier giữ nguyên các dấu gạch chéo ngược nằm bên trong và ở cuối, đồng thời các mẫu siêu dữ liệu không còn thất bại.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
java, sql
Lĩnh vực
api, databases
Loại issue
Lỗi
Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
88/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.