DATAMONGO-1575 - Escape Strings correctly.

Use regex groups and parameter index values for replacement in string based queries.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.mongodb.repository.query;
 
+import com.mongodb.DBObject;
+import lombok.EqualsAndHashCode;
 import lombok.Value;
 
 import java.util.ArrayList;
@@ -110,16 +112,23 @@ class ExpressionEvaluatingParameterBinder {
 		Matcher matcher = createReplacementPattern(bindingContext.getBindings()).matcher(input);
 		StringBuffer buffer = new StringBuffer();
 
+		int parameterIndex = 0;
 		while (matcher.find()) {
 
-			ParameterBinding binding = bindingContext.getBindingFor(extractPlaceholder(matcher.group()));
+			Placeholder placeholder = extractPlaceholder(parameterIndex++, matcher);
+			ParameterBinding binding = bindingContext.getBindingFor(placeholder);
 			String valueForBinding = getParameterValueForBinding(accessor, bindingContext.getParameters(), binding);
 
 			// appendReplacement does not like unescaped $ sign and others, so we need to quote that stuff first
 			matcher.appendReplacement(buffer, Matcher.quoteReplacement(valueForBinding));
+			if (StringUtils.hasText(placeholder.getSuffix())) {
+				buffer.append(placeholder.getSuffix());
+			}
 
-			if (binding.isQuoted()) {
-				postProcessQuotedBinding(buffer, valueForBinding);
+			if (binding.isQuoted() || placeholder.isQuoted()) {
+				postProcessQuotedBinding(buffer, valueForBinding,
+						!binding.isExpression() ? accessor.getBindableValue(binding.getParameterIndex()) : null,
+						binding.isExpression());
 			}
 		}
 
@@ -135,7 +144,7 @@ class ExpressionEvaluatingParameterBinder {
 	 * @param buffer the {@link StringBuffer} to operate upon.
 	 * @param valueForBinding the actual binding value.
 	 */
-	private void postProcessQuotedBinding(StringBuffer buffer, String valueForBinding) {
+	private void postProcessQuotedBinding(StringBuffer buffer, String valueForBinding, Object raw, boolean isExpression) {
 
 		int quotationMarkIndex = buffer.length() - valueForBinding.length() - 1;
 		char quotationMark = buffer.charAt(quotationMarkIndex);
@@ -151,7 +160,8 @@ class ExpressionEvaluatingParameterBinder {
 			quotationMark = buffer.charAt(quotationMarkIndex);
 		}
 
-		if (valueForBinding.startsWith("{")) { // remove quotation char before the complex object string
+		if (valueForBinding.startsWith("{") && (raw instanceof DBObject || isExpression)) { // remove quotation char before
+																																												// the complex object string
 
 			buffer.deleteCharAt(quotationMarkIndex);
 
@@ -181,7 +191,12 @@ class ExpressionEvaluatingParameterBinder {
 				: accessor.getBindableValue(binding.getParameterIndex());
 
 		if (value instanceof String && binding.isQuoted()) {
-			return ((String) value).startsWith("{") ? (String) value : ((String) value).replace("\"", "\\\"");
+
+			if (binding.isExpression() && ((String) value).startsWith("{")) {
+				return (String) value;
+			}
+
+			return ((String) value).replace("\\", "\\\\").replace("\"", "\\\"");
 		}
 
 		if (value instanceof byte[]) {
@@ -228,8 +243,8 @@ class ExpressionEvaluatingParameterBinder {
 		for (ParameterBinding binding : bindings) {
 
 			regex.append("|");
-			regex.append(Pattern.quote(binding.getParameter()));
-			regex.append("['\"]?"); // potential quotation char (as in { foo : '?0' }).
+			regex.append("(" + Pattern.quote(binding.getParameter()) + ")");
+			regex.append("(\\W?['\"])?"); // potential quotation char (as in { foo : '?0' }).
 		}
 
 		return Pattern.compile(regex.substring(1));
@@ -239,14 +254,35 @@ class ExpressionEvaluatingParameterBinder {
 	 * Extract the placeholder stripping any trailing trailing quotation mark that might have resulted from the
 	 * {@link #createReplacementPattern(List) pattern} used.
 	 *
-	 * @param groupName The actual {@link Matcher#group() group}.
+	 * @param matcher The actual {@link Matcher#group() group}.
 	 * @return
 	 */
-	private Placeholder extractPlaceholder(String groupName) {
+	private Placeholder extractPlaceholder(int parameterIndex, Matcher matcher) {
 
-		return !groupName.endsWith("'") && !groupName.endsWith("\"") ? //
-				Placeholder.of(groupName, false) : //
-				Placeholder.of(groupName.substring(0, groupName.length() - 1), true);
+		if (matcher.groupCount() > 1) {
+
+			String suffix = matcher.group(parameterIndex * 2 + 2);
+			String rawPlaceholder = matcher.group(parameterIndex * 2 + 1);
+
+			if (!StringUtils.hasText(rawPlaceholder)) {
+
+				rawPlaceholder = matcher.group();
+				suffix = ""+rawPlaceholder.charAt(rawPlaceholder.length()-1);
+				if(rawPlaceholder.endsWith("'")) {
+					rawPlaceholder = rawPlaceholder.substring(0, rawPlaceholder.length()-1);
+				}
+			}
+
+			if (StringUtils.hasText(suffix)) {
+
+				boolean quoted= (suffix.endsWith("'") || suffix.endsWith("\""));
+
+				return Placeholder.of(parameterIndex, rawPlaceholder, quoted,
+						quoted ? suffix.substring(0, suffix.length() - 1) : suffix);
+			}
+		}
+
+		return Placeholder.of(parameterIndex, matcher.group(), false, null);
 	}
 
 	/**
@@ -317,8 +353,9 @@ class ExpressionEvaluatingParameterBinder {
 
 			Map<Placeholder, ParameterBinding> map = new LinkedHashMap<Placeholder, ParameterBinding>(bindings.size(), 1);
 
+			int parameterIndex = 0;
 			for (ParameterBinding binding : bindings) {
-				map.put(Placeholder.of(binding.getParameter(), binding.isQuoted()), binding);
+				map.put(Placeholder.of(parameterIndex++, binding.getParameter(), binding.isQuoted(), null), binding);
 			}
 
 			return map;
@@ -332,10 +369,13 @@ class ExpressionEvaluatingParameterBinder {
 	 * @since 1.9
 	 */
 	@Value(staticConstructor = "of")
+	@EqualsAndHashCode(exclude = { "quoted", "suffix" })
 	static class Placeholder {
 
+		private int parameterIndex;
 		private final String parameter;
 		private final boolean quoted;
+		private final String suffix;
 
 		/*
 		 * (non-Javadoc)
@@ -343,7 +383,9 @@ class ExpressionEvaluatingParameterBinder {
 		 */
 		@Override
 		public String toString() {
-			return quoted ? String.format("'%s'", parameter) : parameter;
+			return quoted ? String.format("'%s'", parameter + (suffix != null ? suffix : ""))
+					: parameter + (suffix != null ? suffix : "");
 		}
+
 	}
 }

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java

@@ -23,6 +23,7 @@ import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import javax.xml.bind.DatatypeConverter;
 
@@ -394,7 +395,7 @@ public class StringBasedMongoQueryUnitTests {
 				is((DBObject) new BasicDBObject("lastname", "\"Dave Matthews\", password: 'foo")));
 	}
 
-	@Test // DATAMONGO-1565
+	@Test // DATAMONGO-1565, DATAMONGO-1575
 	public void shouldQuoteComplexQueryStringCorreclty() throws Exception {
 
 		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class);
@@ -402,10 +403,10 @@ public class StringBasedMongoQueryUnitTests {
 
 		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
 		assertThat(query.getQueryObject(),
-				is((DBObject) new BasicDBObject("lastname", new BasicDBObject("$ne", "calamity"))));
+				is((DBObject) new BasicDBObject("lastname",  "{ $ne : \"calamity\" }")));
 	}
 
-	@Test // DATAMONGO-1565
+	@Test // DATAMONGO-1565, DATAMONGO-1575
 	public void shouldQuotationInQuotedComplexQueryString() throws Exception {
 
 		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class);
@@ -414,7 +415,33 @@ public class StringBasedMongoQueryUnitTests {
 
 		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
 		assertThat(query.getQueryObject(),
-				is((DBObject) new BasicDBObject("lastname", new BasicDBObject("$ne", "\"calamity\""))));
+				is((DBObject) new BasicDBObject("lastname", "{ $ne : \"\\\"calamity\\\"\" }")));
+	}
+
+	/**
+	 * @see DATAMONGO-1575
+	 */
+	@Test
+	public void shouldTakeBsonParameterAsIs() throws Exception {
+
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByWithBsonArgument", DBObject.class);
+		ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, new BasicDBObject("$regex", "^calamity$"));
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
+		assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject("arg0", Pattern.compile("^calamity$"))));
+	}
+
+	/**
+	 * @see DATAMONGO-1575
+	 */
+	@Test
+	public void shouldReplaceParametersInInQuotedExpressionOfNestedQueryOperator() throws Exception {
+
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameRegex", String.class);
+		ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity");
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
+		assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject("lastname", Pattern.compile("^(calamity)"))));
 	}
 
 	private StringBasedMongoQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
@@ -437,6 +464,9 @@ public class StringBasedMongoQueryUnitTests {
 		@Query("{ 'lastname' : '?0' }")
 		Person findByLastnameQuoted(String lastname);
 
+		@Query("{ 'lastname' : { '$regex' : '^(?0)'} }")
+		Person findByLastnameRegex(String lastname);
+
 		@Query("{ 'address' : ?0 }")
 		Person findByAddress(Address address);
 
@@ -484,5 +514,8 @@ public class StringBasedMongoQueryUnitTests {
 
 		@Query("{ 'arg0' : ?0, 'arg1' : ?1 }")
 		List<Person> findByStringWithWildcardChar(String arg0, String arg1);
+
+		@Query("{ 'arg0' : ?0 }")
+		List<Person> findByWithBsonArgument(DBObject arg0);
 	}
 }

src/main/asciidoc/reference/mongo-repositories.adoc

@@ -366,7 +366,9 @@ public interface PersonRepository extends MongoRepository<Person, String>
 }
 ----
 
-The placeholder ?0 lets you substitute the value from the method arguments into the JSON query string.
+The placeholder `?0` lets you substitute the value from the method arguments into the JSON query string.
+
+NOTE: `String` parameter values are escaped during the binding process, which means that it is not possible to add MongoDB specific operators via the argument.
 
 You can also use the filter property to restrict the set of properties that will be mapped into the Java object. For example,
 
@@ -382,6 +384,64 @@ public interface PersonRepository extends MongoRepository<Person, String>
 
 This will return only the firstname, lastname and Id properties of the Person objects. The age property, a java.lang.Integer, will not be set and its value will therefore be null.
 
+[[mongodb.repositories.queries.json-spel]]
+=== JSON based queries with SpEL expressions
+
+Query strings and field definitions can be used together with SpEL expressions to create dynamic queries at runtime.
+SpEL expressions can provide predicate values and can be used to extend predicates with subdocuments.
+
+Expressions expose method arguments through an array that contains all arguments. The the following query uses `[0]`
+to declare the predicate value for `lastname` that is equivalent to the `?0` parameter binding.
+
+[source,java]
+----
+public interface PersonRepository extends MongoRepository<Person, String>
+
+  @Query("{'lastname': ?#{[0]} }")
+  List<Person> findByQueryWithExpression(String param0);
+
+}
+----
+
+Expressions can be used to invoke functions, evaluate conditionals and construct values. SpEL expressions
+reveal in conjunction with JSON a side-effect as Map-like declarations inside of SpEL read like JSON.
+
+[source,java]
+----
+public interface PersonRepository extends MongoRepository<Person, String>
+
+  @Query("{'id': ?#{ [0] ? {$exists :true} : [1] }}")
+  List<Person> findByQueryWithExpressionAndNestedObject(boolean param0, String param1);
+
+}
+----
+
+SpEL in query strings can be a powerful way to enhance queries and can accept a broad range of unwanted arguments.
+You should make sure to sanitize strings before passing these to the query to avoid unwanted changes to your query.
+
+Expression support is extensible through the Query SPI `org.springframework.data.repository.query.spi.EvaluationContextExtension`
+than can contribute properties, functions and customize the root object. Extensions are retrieved from the application context
+at the time of SpEL evaluation when the query is build.
+
+[source,java]
+----
+public class SampleEvaluationContextExtension extends EvaluationContextExtensionSupport {
+
+  @Override
+  public String getExtensionId() {
+    return "security";
+  }
+
+  @Override
+  public Map<String, Object> getProperties() {
+    return Collections.singletonMap("principal", SecurityContextHolder.getCurrent().getPrincipal());
+  }
+}
+----
+
+NOTE: Bootstrapping `MongoRepositoryFactory` yourself is not application context-aware and requires further configuration
+to pick up Query SPI extensions.
+
 [[mongodb.repositories.queries.type-safe]]
 === Type-safe Query methods