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

Abierto Apto para principiantes
#3,063 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
88/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
java, sql
Área
api, databases

Línea de trabajo

Comienza con client-v2/src/main/java/com/clickhouse/client/api/sql/SQLUtils.java y compara enquoteLiteral y enquoteIdentifier con escapeSingleQuotes. Ejecuta client-v2/src/test/java/com/clickhouse/client/api/sql/SQLUtilsTest.java y jdbc-v2/.../EnquoteBackslashTest.java usando el comando Maven proporcionado; se considera terminado cuando los literales y los identificadores conservan las barras invertidas incrustadas y finales, y los patrones de metadatos ya no fallan.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

area:general bug client-api-v2 jdbc jdbc-v2

Description

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

// 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 —

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:

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:<TAB>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 [col<TAB>name]

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

(<TAB> 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).

Lenguaje dominante
Java
Estrellas
1.6k
Forks
637
Merge medio
2 d 12 h
PR fusionados (30 d)
28

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de ClickHouse/clickhouse-java

Todos los issues de ClickHouse/clickhouse-java

Issues similares

Más issues de Java

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.