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
spring-projects · Aug 25, 2023 · 0559c38 · 0559c38
1 parent a9279c7
commit 0559c38
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 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
@@ -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/...jpa/src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java b/...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() {