From 0559c38c9c04e91ee23c776affb008294f1642ea Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 25 Aug 2023 09:31:30 +0200 Subject: [PATCH] Correctly validate mixed parameter bind marker usage. We now inspect individual parameters instead of the resulting query whether the query contains JDBC-style bind markers. Previously, we inspected the final (rewritten) query which might have contained question marks in literals leading to improper validation failures. Closes #3125 --- .../jpa/repository/query/StringQuery.java | 27 ++++++++++--------- .../query/StringQueryUnitTests.java | 15 +++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index ad1a25cb97..afa68ff51c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -15,6 +15,8 @@ */ package org.springframework.data.jpa.repository.query; +import static java.util.regex.Pattern.*; + import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -39,8 +41,6 @@ import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; -import static java.util.regex.Pattern.*; - /** * Encapsulation of a JPA query String. Offers access to parameters as bindings. The internal query String is cleaned * from decorated parameters like {@literal %:lastname%} and the matching bindings take care of applying the decorations @@ -244,13 +244,6 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St int currentIndex = 0; boolean usesJpaStyleParameters = false; - if (JDBC_STYLE_PARAM.matcher(resultingQuery).find()) { - queryMeta.usesJdbcStyleParameters = true; - } - - if (NUMBERED_STYLE_PARAM.matcher(resultingQuery).find() || NAMED_STYLE_PARAM.matcher(resultingQuery).find()) { - usesJpaStyleParameters = true; - } while (matcher.find()) { @@ -262,6 +255,19 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St String parameterName = parameterIndexString != null ? null : matcher.group(NAMED_PARAMETER_GROUP); Integer parameterIndex = getParameterIndex(parameterIndexString); + String match = matcher.group(0); + if (JDBC_STYLE_PARAM.matcher(match).find()) { + queryMeta.usesJdbcStyleParameters = true; + } + + if (NUMBERED_STYLE_PARAM.matcher(match).find() || NAMED_STYLE_PARAM.matcher(match).find()) { + usesJpaStyleParameters = true; + } + + if (usesJpaStyleParameters && queryMeta.usesJdbcStyleParameters) { + throw new IllegalArgumentException("Mixing of ? parameters and other forms like ?1 is not supported"); + } + String typeSource = matcher.group(COMPARISION_TYPE_GROUP); Assert.isTrue(parameterIndexString != null || parameterName != null, () -> String.format("We need either a name or an index; Offending query string: %s", query)); @@ -273,9 +279,6 @@ private String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(St parameterIndex = expressionParameterIndex; } - if (usesJpaStyleParameters && queryMeta.usesJdbcStyleParameters) { - throw new IllegalArgumentException("Mixing of ? parameters and other forms like ?1 is not supported"); - } BindingIdentifier queryParameter; if (parameterIndex != null) { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java index cde06804a0..f50f7b03d5 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java @@ -656,6 +656,7 @@ void questionMarkInStringLiteral() { softly.assertThat(query.getQueryString()).isEqualTo(queryString); softly.assertThat(query.hasParameterBindings()).isFalse(); softly.assertThat(query.getParameterBindings()).hasSize(0); + softly.assertThat(query.usesJdbcStyleParameters()).isFalse(); softly.assertAll(); } @@ -697,6 +698,20 @@ void isNotDefaultProjection() { softly.assertAll(); } + @Test // GH-3125 + void questionMarkInStringLiteralWithParameters() { + + String queryString = "SELECT CAST(REGEXP_SUBSTR(itp.template_as_txt, '(?<=templateId\\\\\\\\=)(\\\\\\\\d+)(?:\\\\\\\\R)') AS INT) AS templateId FROM foo itp WHERE bar = ?1 AND baz = 1"; + StringQuery query = new StringQuery(queryString, false); + + softly.assertThat(query.getQueryString()).isEqualTo(queryString); + softly.assertThat(query.hasParameterBindings()).isTrue(); + softly.assertThat(query.getParameterBindings()).hasSize(1); + softly.assertThat(query.usesJdbcStyleParameters()).isFalse(); + + softly.assertAll(); + } + @Test // DATAJPA-1652 void usingPipesWithNamedParameter() {