From 950f6311c52e025cc4803899d943c2ad058150cb Mon Sep 17 00:00:00 2001 From: piyush15102003 Date: Wed, 30 Sep 2026 16:09:27 +0530 Subject: [PATCH] Bind metadata patterns instead of concatenating them into SQL Closes #3169. getSchemas(String, String), getPrimaryKeys and getFunctions in jdbc-v2 DatabaseMetaDataImpl built their WHERE clause by concatenating the caller's pattern into the SQL text as "... LIKE '" + pattern + "'". A single quote in the argument closed the literal early, so a name that ClickHouse accepts (for example a database created as `a'b`) failed with Code 62 SYNTAX_ERROR, and the remainder of the argument was parsed as SQL - getSchemas(null, "nomatch' OR '1'='1") returned every database instead of none. Bind each pattern as a PreparedStatement parameter, the way the sibling getTables already does. PreparedStatementImpl encodes String parameters with SQLUtils.escapeSingleQuotes, which escapes the backslash before the quote, so this path is correct for backslashes too and does not depend on the enquoteLiteral fix in #3063. The LIKE / ILIKE operators, the null to "%" default and the JDBC search escape are unchanged. Because the PreparedStatement is now closed with the try-with-resources block, the rows are returned through DetachedResultSet, matching getTables and getTableTypes. This also stops the three methods leaking the Statement they used to create and never close. Add a regression test to DatabaseMetaDataTest with a @DataProvider over the three methods, covering a database and table whose names contain a single quote (must be found) and the pattern "nomatch' OR '1'='1" (must return no rows). --- .../jdbc/metadata/DatabaseMetaDataImpl.java | 31 +++++-- .../jdbc/metadata/DatabaseMetaDataTest.java | 84 +++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java index 482237e03..336cbaf03 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java @@ -1262,10 +1262,16 @@ public ResultSet getPrimaryKeys(String catalog, String schema, String table) thr "FROM system.tables " + "ARRAY JOIN arrayZip(splitByChar(',', primary_key), arrayEnumerate(splitByChar(',', primary_key))) as c " + "WHERE system.tables.primary_key <> '' " + - "AND system.tables.database ILIKE '" + (schema == null ? "%" : schema) + "' " + - "AND system.tables.name ILIKE '" + (table == null ? "%" : table) + "' " + + "AND system.tables.database ILIKE ? " + + "AND system.tables.name ILIKE ? " + "ORDER BY COLUMN_NAME"; - return createStatement().executeQuery(sql); + try (PreparedStatement stmt = prepareStatement(sql)) { + stmt.setString(1, schema == null ? "%" : schema); + stmt.setString(2, table == null ? "%" : table); + try (ResultSet rs = stmt.executeQuery()) { + return DetachedResultSet.createFromResultSet(rs, connection.getDefaultCalendar(), Collections.emptyList()); + } + } } catch (Exception e) { throw ExceptionUtils.toSqlState(e); } @@ -1808,9 +1814,13 @@ public RowIdLifetime getRowIdLifetime() throws SQLException { @Override public ResultSet getSchemas(String catalog, String schemaPattern) throws SQLException { // TODO: handle useCatalogs == true and return schema catalog name - try { - return createStatement().executeQuery("SELECT name AS TABLE_SCHEM, " + catalogPlaceholder + " AS TABLE_CATALOG FROM system.databases " + - "WHERE name LIKE '" + (schemaPattern == null ? "%" : schemaPattern) + "'"); + String sql = "SELECT name AS TABLE_SCHEM, " + catalogPlaceholder + " AS TABLE_CATALOG FROM system.databases " + + "WHERE name LIKE ?"; + try (PreparedStatement stmt = prepareStatement(sql)) { + stmt.setString(1, schemaPattern == null ? "%" : schemaPattern); + try (ResultSet rs = stmt.executeQuery()) { + return DetachedResultSet.createFromResultSet(rs, connection.getDefaultCalendar(), Collections.emptyList()); + } } catch (Exception e) { throw ExceptionUtils.toSqlState(e); } @@ -1861,9 +1871,12 @@ public ResultSet getFunctions(String catalog, String schemaPattern, String funct "CAST(" + java.sql.DatabaseMetaData.functionResultUnknown + " AS Int16) AS FUNCTION_TYPE, " + "name AS SPECIFIC_NAME " + "FROM system.functions " + - "WHERE name LIKE '" + (functionNamePattern == null ? "%" : functionNamePattern) + "'"; - try { - return createStatement().executeQuery(sql); + "WHERE name LIKE ?"; + try (PreparedStatement stmt = prepareStatement(sql)) { + stmt.setString(1, functionNamePattern == null ? "%" : functionNamePattern); + try (ResultSet rs = stmt.executeQuery()) { + return DetachedResultSet.createFromResultSet(rs, connection.getDefaultCalendar(), Collections.emptyList()); + } } catch (Exception e) { throw ExceptionUtils.toSqlState(e); } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java index 08dd650ae..7badf2c7b 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java @@ -10,6 +10,7 @@ import com.clickhouse.jdbc.internal.JdbcUtils; import org.testng.Assert; import org.testng.SkipException; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.ByteArrayOutputStream; @@ -607,6 +608,89 @@ public void testGetPrimaryKeys() throws Exception { } } + /** + * A {@link DatabaseMetaData} lookup that takes a schema pattern and an object-name pattern. + * Used to run the same assertions over the metadata methods that filter on caller-supplied + * patterns. + */ + @FunctionalInterface + private interface MetaDataPatternCall { + ResultSet apply(DatabaseMetaData dbmd, String schemaPattern, String namePattern) throws SQLException; + } + + @DataProvider(name = "metadataPatternArguments") + public static Object[][] metadataPatternArguments() { + final String schemaWithQuote = QUOTED_NAME_SCHEMA; + final String tableWithQuote = QUOTED_NAME_TABLE; + final String injection = "nomatch' OR '1'='1"; + + MetaDataPatternCall getSchemas = (dbmd, schemaPattern, namePattern) -> dbmd.getSchemas(null, schemaPattern); + MetaDataPatternCall getPrimaryKeys = (dbmd, schemaPattern, namePattern) -> dbmd.getPrimaryKeys(null, schemaPattern, namePattern); + MetaDataPatternCall getFunctions = (dbmd, schemaPattern, namePattern) -> dbmd.getFunctions(null, null, namePattern); + + return new Object[][] { + // A name that legally contains a single quote must be matched, not rejected as a syntax error. + { "getSchemas", getSchemas, schemaWithQuote, null, 1 }, + { "getPrimaryKeys", getPrimaryKeys, schemaWithQuote, tableWithQuote, 1 }, + // No function name contains a quote, so the only thing to assert is that it does not throw. + { "getFunctions", getFunctions, null, "to'Int", 0 }, + // The pattern must stay a value: it must not close the literal and inject a predicate. + { "getSchemas", getSchemas, injection, null, 0 }, + { "getPrimaryKeys", getPrimaryKeys, injection, null, 0 }, + { "getFunctions", getFunctions, null, injection, 0 }, + }; + } + + /** + * Regression test: these methods used to concatenate the caller's pattern straight into the SQL + * text, so a pattern containing a single quote either failed with SYNTAX_ERROR or was parsed as + * SQL and changed the result set. + */ + @Test(groups = { "integration" }, dataProvider = "metadataPatternArguments") + public void testMetadataPatternIsUsedAsValueNotSql(String method, MetaDataPatternCall call, + String schemaPattern, String namePattern, + int expectedRows) throws Exception { + if ("getFunctions".equals(method) && ClickHouseVersion.of(getServerVersion()).check("(,23.8]")) { + throw new SkipException("getFunctions is broken before 23.9 - see testGetFunctions"); + } + + try (Connection conn = getJdbcConnection()) { + createQuotedNameFixture(conn); + try { + DatabaseMetaData dbmd = conn.getMetaData(); + int rows = 0; + try (ResultSet rs = call.apply(dbmd, schemaPattern, namePattern)) { + while (rs.next()) { + rows++; + } + } + assertEquals(rows, expectedRows, method + " returned the wrong number of rows for schemaPattern=" + + schemaPattern + ", namePattern=" + namePattern); + } finally { + dropQuotedNameFixture(conn); + } + } + } + + /** Deliberately contains a single quote, and no {@code _} so that LIKE matches it exactly. */ + private static final String QUOTED_NAME_SCHEMA = "mdquotea'b"; + + private static final String QUOTED_NAME_TABLE = "t'1"; + + private static void createQuotedNameFixture(Connection conn) throws SQLException { + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate("DROP DATABASE IF EXISTS `mdquotea'b`"); + stmt.executeUpdate("CREATE DATABASE `mdquotea'b`"); + stmt.executeUpdate("CREATE TABLE `mdquotea'b`.`t'1` (id Int32, v String) ENGINE MergeTree ORDER BY id"); + } + } + + private static void dropQuotedNameFixture(Connection conn) throws SQLException { + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate("DROP DATABASE IF EXISTS `mdquotea'b`"); + } + } + @Test(groups = { "integration" }) public void testGetSchemas() throws Exception { try (Connection conn = getJdbcConnection()) {