From e18980e22160cdc2f6f45ff6e5bd238aa33c9467 Mon Sep 17 00:00:00 2001 From: Sanghyuk Jung Date: Sun, 26 Jul 2026 12:51:50 +0900 Subject: [PATCH] Add option to parse named parameters in SQL comments By default, named parameters within SQL comments are ignored. Some database engines interpret hints or routing keys declared in comments, for example "/*@ queryKey(:userId) */ SELECT ...", requiring such parameters to be substituted and bound as well. This commit introduces a parseSqlStatement variant with an allowParametersInComments flag in NamedParameterUtils as well as a corresponding allowParametersInComments property on NamedParameterJdbcTemplate. Closes gh-22255 Signed-off-by: Sanghyuk Jung --- .../NamedParameterJdbcTemplate.java | 43 ++++++++++++- .../core/namedparam/NamedParameterUtils.java | 46 ++++++++++++- .../NamedParameterJdbcTemplateTests.java | 64 +++++++++++++++++++ .../namedparam/NamedParameterUtilsTests.java | 59 +++++++++++++++++ 4 files changed, 208 insertions(+), 4 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java index db76fa820dea..8d1523ad47af 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java @@ -86,6 +86,9 @@ public class NamedParameterJdbcTemplate implements NamedParameterJdbcOperations /** The JdbcTemplate we are wrapping. */ private final JdbcOperations classicJdbcTemplate; + /** Whether named parameters within SQL comments are to be parsed as well. */ + private boolean allowParametersInComments = false; + /** Cache of original SQL String to ParsedSql representation. */ private volatile ConcurrentLruCache parsedSqlCache; @@ -107,7 +110,7 @@ public NamedParameterJdbcTemplate(DataSource dataSource) { public NamedParameterJdbcTemplate(JdbcOperations classicJdbcTemplate) { Assert.notNull(classicJdbcTemplate, "JdbcTemplate must not be null"); this.classicJdbcTemplate = classicJdbcTemplate; - this.parsedSqlCache = new ConcurrentLruCache<>(DEFAULT_CACHE_LIMIT, NamedParameterUtils::parseSqlStatement); + this.parsedSqlCache = createParsedSqlCache(DEFAULT_CACHE_LIMIT); } /** @@ -119,6 +122,7 @@ public NamedParameterJdbcTemplate(JdbcOperations classicJdbcTemplate) { public NamedParameterJdbcTemplate(NamedParameterJdbcTemplate original, JdbcTemplate classicJdbcTemplate) { Assert.notNull(classicJdbcTemplate, "JdbcTemplate must not be null"); this.classicJdbcTemplate = classicJdbcTemplate; + this.allowParametersInComments = original.allowParametersInComments; this.parsedSqlCache = original.parsedSqlCache; } @@ -149,7 +153,7 @@ public JdbcTemplate getJdbcTemplate() { * Default is 256. 0 indicates no caching, always parsing each statement. */ public void setCacheLimit(int cacheLimit) { - this.parsedSqlCache = new ConcurrentLruCache<>(cacheLimit, NamedParameterUtils::parseSqlStatement); + this.parsedSqlCache = createParsedSqlCache(cacheLimit); } /** @@ -159,6 +163,41 @@ public int getCacheLimit() { return this.parsedSqlCache.capacity(); } + /** + * Specify whether named parameters within SQL comments are to be parsed and + * substituted as well, rather than being ignored. Default is {@code false}. + *

Turning this flag on is useful for database engines that interpret hints + * or routing keys declared in comments, for example + * "/*@ queryKey(:userId) */ SELECT ...". + *

NOTE: Even with this flag turned on, string literals and quoted + * identifiers are still skipped. Also, the target driver or engine needs to + * count a placeholder within a comment as an actual bind parameter; otherwise, + * the number of bind values is not going to match. See + * {@link NamedParameterUtils#parseSqlStatement(String, boolean)} for details. + *

Note that changing this setting resets this template's SQL cache, + * retaining the configured {@link #setCacheLimit cache limit}. + * @since 7.1 + * @see NamedParameterUtils#parseSqlStatement(String, boolean) + */ + public void setAllowParametersInComments(boolean allowParametersInComments) { + this.allowParametersInComments = allowParametersInComments; + this.parsedSqlCache = createParsedSqlCache(getCacheLimit()); + } + + /** + * Return whether named parameters within SQL comments are to be parsed as well. + * @since 7.1 + */ + public boolean isAllowParametersInComments() { + return this.allowParametersInComments; + } + + private ConcurrentLruCache createParsedSqlCache(int cacheLimit) { + boolean allowInComments = this.allowParametersInComments; + return new ConcurrentLruCache<>(cacheLimit, + sql -> NamedParameterUtils.parseSqlStatement(sql, allowInComments)); + } + @Override public T execute(String sql, SqlParameterSource paramSource, PreparedStatementCallback action) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java index 3c1902a90619..46e454269e77 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java @@ -51,6 +51,12 @@ public abstract class NamedParameterUtils { */ private static final String[] STOP_SKIP = {"'", "\"", "\n", "*/", "`"}; + /** + * Flags indicating which of the {@link #START_SKIP} entries start a comment + * rather than a quoted identifier or string literal. + */ + private static final boolean[] COMMENT_SKIP = {false, false, true, true, false}; + /** * Set of characters that qualify as parameter separators, * indicating that a parameter name in an SQL String has ended. @@ -77,10 +83,41 @@ public abstract class NamedParameterUtils { /** * Parse the SQL statement and locate any placeholders or named parameters. * Named parameters are substituted for a JDBC placeholder. + *

Named parameters within comments are ignored; see + * {@link #parseSqlStatement(String, boolean)} for an option to include them. * @param sql the SQL statement * @return the parsed statement, represented as {@link ParsedSql} instance */ public static ParsedSql parseSqlStatement(String sql) { + return parseSqlStatement(sql, false); + } + + /** + * Parse the SQL statement and locate any placeholders or named parameters, + * optionally including named parameters within comments. + * Named parameters are substituted for a JDBC placeholder. + *

By default, named parameters within line comments (--) and block + * comments (/* ... */) are ignored, just like named parameters + * within string literals and quoted identifiers. Specifying {@code true} for the + * {@code allowParametersInComments} flag makes named parameters within comments + * subject to regular parsing and substitution, which is useful for database engines + * that interpret hints or routing keys in comments, for example + * "/*@ queryKey(:userId) */ SELECT ...". + *

NOTE: Even with the flag set to {@code true}, string literals and quoted + * identifiers are still skipped. As a consequence, an apostrophe within a + * comment (as in "/* don't touch */") is going to be interpreted + * as the start of a string literal, potentially skipping over subsequent named + * parameters. Also, since a named parameter within a comment gets substituted with a + * JDBC placeholder like any other, the target driver or engine needs to count such a + * placeholder as an actual bind parameter; otherwise, the number of bind values is + * not going to match. + * @param sql the SQL statement + * @param allowParametersInComments whether to parse named parameters + * within SQL comments as well, rather than ignoring them + * @return the parsed statement, represented as {@link ParsedSql} instance + * @since 7.1 + */ + public static ParsedSql parseSqlStatement(String sql, boolean allowParametersInComments) { Assert.notNull(sql, "SQL must not be null"); Set namedParameters = new HashSet<>(); @@ -97,7 +134,7 @@ public static ParsedSql parseSqlStatement(String sql) { while (i < statement.length) { int skipToPosition = i; while (i < statement.length) { - skipToPosition = skipCommentsAndQuotes(statement, i); + skipToPosition = skipCommentsAndQuotes(statement, i, !allowParametersInComments); if (i == skipToPosition) { break; } @@ -223,10 +260,15 @@ private static int addNewNamedParameter(Set namedParameters, int namedPa * Skip over comments and quoted names present in an SQL statement. * @param statement character array containing SQL statement * @param position current position of statement + * @param skipComments whether to skip over comments as well, + * rather than just over quoted names and string literals * @return next position to process after any comments or quotes are skipped */ - private static int skipCommentsAndQuotes(char[] statement, int position) { + private static int skipCommentsAndQuotes(char[] statement, int position, boolean skipComments) { for (int i = 0; i < START_SKIP.length; i++) { + if (!skipComments && COMMENT_SKIP[i]) { + continue; + } if (statement[position] == START_SKIP[i].charAt(0)) { boolean match = true; for (int j = 1; j < START_SKIP[i].length(); j++) { diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplateTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplateTests.java index e2f6387965ec..1c8c3f6be914 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplateTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplateTests.java @@ -80,6 +80,13 @@ class NamedParameterJdbcTemplateTests { private static final String UPDATE_ARRAY_PARAMETERS_PARSED = "update customer set type = array[?, ?, ?] where id = ?"; + private static final String SELECT_NAMED_PARAMETERS_IN_COMMENT = + "/*@ queryKey(:id) */ select id, forename from custmr where id = :id and country = :country"; + private static final String SELECT_NAMED_PARAMETERS_IN_COMMENT_PARSED = + "/*@ queryKey(?) */ select id, forename from custmr where id = ? and country = ?"; + private static final String SELECT_NAMED_PARAMETERS_IN_COMMENT_PARSED_WITHOUT_COMMENT = + "/*@ queryKey(:id) */ select id, forename from custmr where id = ? and country = ?"; + private static final String[] COLUMN_NAMES = new String[] {"id", "forename"}; @@ -147,6 +154,63 @@ void execute() throws SQLException { verify(connection).close(); } + @Test // gh-22255 + void executeWithParametersInCommentsIgnoredByDefault() throws SQLException { + given(preparedStatement.executeUpdate()).willReturn(1); + + assertThat(namedParameterTemplate.isAllowParametersInComments()).isFalse(); + + params.put("id", 1); + params.put("country", "UK"); + namedParameterTemplate.execute(SELECT_NAMED_PARAMETERS_IN_COMMENT, params, + (PreparedStatementCallback) ps -> { + ps.executeUpdate(); + return "result"; + }); + + verify(connection).prepareStatement(SELECT_NAMED_PARAMETERS_IN_COMMENT_PARSED_WITHOUT_COMMENT); + verify(preparedStatement).setObject(1, 1); + verify(preparedStatement).setString(2, "UK"); + verify(preparedStatement).close(); + verify(connection).close(); + } + + @Test // gh-22255 + void executeWithParametersInCommentsAllowed() throws SQLException { + given(preparedStatement.executeUpdate()).willReturn(1); + + namedParameterTemplate.setAllowParametersInComments(true); + assertThat(namedParameterTemplate.isAllowParametersInComments()).isTrue(); + + params.put("id", 1); + params.put("country", "UK"); + namedParameterTemplate.execute(SELECT_NAMED_PARAMETERS_IN_COMMENT, params, + (PreparedStatementCallback) ps -> { + ps.executeUpdate(); + return "result"; + }); + + verify(connection).prepareStatement(SELECT_NAMED_PARAMETERS_IN_COMMENT_PARSED); + verify(preparedStatement).setObject(1, 1); + verify(preparedStatement).setObject(2, 1); + verify(preparedStatement).setString(3, "UK"); + verify(preparedStatement).close(); + verify(connection).close(); + } + + @Test // gh-22255 + void setAllowParametersInCommentsResetsParsedSqlCacheRetainingCacheLimit() { + namedParameterTemplate.setCacheLimit(10); + assertThat(namedParameterTemplate.getParsedSql(SELECT_NAMED_PARAMETERS_IN_COMMENT) + .getTotalParameterCount()).isEqualTo(2); + + namedParameterTemplate.setAllowParametersInComments(true); + + assertThat(namedParameterTemplate.getCacheLimit()).isEqualTo(10); + assertThat(namedParameterTemplate.getParsedSql(SELECT_NAMED_PARAMETERS_IN_COMMENT) + .getTotalParameterCount()).isEqualTo(3); + } + @Disabled("SPR-16340") @Test void executeArray() throws SQLException { diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterUtilsTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterUtilsTests.java index 8a2a942367a6..8c54422638d7 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterUtilsTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/namedparam/NamedParameterUtilsTests.java @@ -178,6 +178,65 @@ void parseSqlContainingComments() { assertThat(NamedParameterUtils.substituteNamedParameters(parsedSql4, new MapSqlParameterSource(parameters))).isEqualTo("/*+ HINT */ xxx /* comment :a ? */ ? yyyy ? ? ? zzzzz /* :xx XX*"); } + @Test // gh-22255 + void parseSqlContainingParametersInBlockCommentWhenAllowed() { + String sql = "/*@ queryKey(:cafeId) */ select * from cafe where cafe_id = :cafeId"; + ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement(sql, true); + assertThat(parsedSql.getParameterNames()).containsExactly("cafeId", "cafeId"); + assertThat(parsedSql.getTotalParameterCount()).isEqualTo(2); + assertThat(parsedSql.getNamedParameterCount()).isEqualTo(1); + + MapSqlParameterSource paramSource = new MapSqlParameterSource("cafeId", 42); + assertThat(NamedParameterUtils.substituteNamedParameters(parsedSql, paramSource)).isEqualTo( + "/*@ queryKey(?) */ select * from cafe where cafe_id = ?"); + assertThat(NamedParameterUtils.buildValueArray(parsedSql, paramSource, null)).containsExactly(42, 42); + } + + @Test // gh-22255 + void parseSqlContainingParametersInLineCommentWhenAllowed() { + String sql = "select * from cafe where cafe_id = :cafeId -- filtered by :status\n"; + ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement(sql, true); + assertThat(parsedSql.getParameterNames()).containsExactly("cafeId", "status"); + assertThat(NamedParameterUtils.substituteNamedParameters(parsedSql, null)).isEqualTo( + "select * from cafe where cafe_id = ? -- filtered by ?\n"); + } + + @Test // gh-22255 + void parseSqlContainingCommentsWhenParametersInCommentsAllowed() { + String sql1 = "/*+ HINT */ xxx /* comment */ :a yyyy :b :c :a zzzzz -- :xx XX\n"; + ParsedSql parsedSql1 = NamedParameterUtils.parseSqlStatement(sql1, true); + assertThat(NamedParameterUtils.substituteNamedParameters(parsedSql1, null)).isEqualTo( + "/*+ HINT */ xxx /* comment */ ? yyyy ? ? ? zzzzz -- ? XX\n"); + assertThat(parsedSql1.getParameterNames()).containsExactly("a", "b", "c", "a", "xx"); + + // Non-terminated comment must not trip up the parser + String sql2 = "/*+ HINT */ xxx /* comment */ :a yyyy :b :c :a zzzzz /* :xx XX*"; + ParsedSql parsedSql2 = NamedParameterUtils.parseSqlStatement(sql2, true); + assertThat(NamedParameterUtils.substituteNamedParameters(parsedSql2, null)).isEqualTo( + "/*+ HINT */ xxx /* comment */ ? yyyy ? ? ? zzzzz /* ? XX*"); + } + + @Test // gh-22255 + void parseSqlContainingCommentsIgnoresParametersInCommentsByDefault() { + String sql = "/*@ queryKey(:cafeId) */ select * from cafe where cafe_id = :cafeId"; + ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement(sql); + assertThat(parsedSql.getParameterNames()).containsExactly("cafeId"); + assertThat(substituteNamedParameters(parsedSql)).isEqualTo( + "/*@ queryKey(:cafeId) */ select * from cafe where cafe_id = ?"); + } + + @ParameterizedTest // gh-22255 + @ValueSource(strings = { + "SELECT ':foo'':doo', :xxx FROM DUAL", + "SELECT \":foo\"\":doo\", :xxx FROM DUAL", + "SELECT `:foo``:doo`, :xxx FROM DUAL" + }) + void parseSqlStatementWithParametersInsideQuotesWhenParametersInCommentsAllowed(String sql) { + ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement(sql, true); + assertThat(parsedSql.getTotalParameterCount()).isEqualTo(1); + assertThat(parsedSql.getParameterNames()).containsExactly("xxx"); + } + @Test // SPR-4612 void parseSqlStatementWithPostgresCasting() { String expectedSql = "select 'first name' from artists where id = ? and birth_date=?::timestamp";