diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java index 10c34bb39..e5f7faf79 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java @@ -15,8 +15,13 @@ */ package org.springframework.data.mongodb.repository.query; -import java.util.Collections; +import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.xml.bind.DatatypeConverter; @@ -94,47 +99,67 @@ class ExpressionEvaluatingParameterBinder { return input; } - boolean isCompletlyParameterizedQuery = input.matches("^\\?\\d+$"); - StringBuilder result = new StringBuilder(input); - - for (ParameterBinding binding : bindingContext.getBindings()) { - - String parameter = binding.getParameter(); - int idx = result.indexOf(parameter); - - if (idx == -1) { - continue; - } - - String valueForBinding = getParameterValueForBinding(accessor, bindingContext.getParameters(), binding); - - int start = idx; - int end = idx + parameter.length(); - - // If the value to bind is an object literal we need to remove the quoting around the expression insertion point. - if (valueForBinding.startsWith("{") && !isCompletlyParameterizedQuery) { - - // Is the insertion point actually surrounded by quotes? - char beforeStart = result.charAt(start - 1); - char afterEnd = result.charAt(end); - - if ((beforeStart == '\'' || beforeStart == '"') && (afterEnd == '\'' || afterEnd == '"')) { - - // Skip preceding and following quote - start -= 1; - end += 1; - } - } - - result.replace(start, end, valueForBinding); + if (input.matches("^\\?\\d+$")) { + return getParameterValueForBinding(accessor, bindingContext.getParameters(), + bindingContext.getBindings().iterator().next()); } - return result.toString(); + Matcher matcher = createReplacementPattern(bindingContext.getBindings()).matcher(input); + StringBuffer buffer = new StringBuffer(); + + while (matcher.find()) { + + ParameterBinding binding = bindingContext.getBindingFor(extractPlaceholder(matcher.group())); + 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 (binding.isQuoted()) { + postProcessQuotedBinding(buffer, valueForBinding); + } + } + + matcher.appendTail(buffer); + return buffer.toString(); + } + + /** + * Sanitize String binding by replacing single quoted values with double quotes which prevents potential single quotes + * contained in replacement to interfere with the Json parsing. Also take care of complex objects by removing the + * quotation entirely. + * + * @param buffer the {@link StringBuffer} to operate upon. + * @param valueForBinding the actual binding value. + */ + private void postProcessQuotedBinding(StringBuffer buffer, String valueForBinding) { + + int quotationMarkIndex = buffer.length() - valueForBinding.length() - 1; + char quotationMark = buffer.charAt(quotationMarkIndex); + + while (quotationMark != '\'' && quotationMark != '"') { + + quotationMarkIndex--; + if (quotationMarkIndex < 0) { + throw new IllegalArgumentException("Could not find opening quotes for quoted parameter"); + } + quotationMark = buffer.charAt(quotationMarkIndex); + } + + if (valueForBinding.startsWith("{")) { // remove quotation char before the complex object string + buffer.deleteCharAt(quotationMarkIndex); + } else { + + if (quotationMark == '\'') { + buffer.replace(quotationMarkIndex, quotationMarkIndex + 1, "\""); + } + buffer.append("\""); + } } /** * Returns the serialized value to be used for the given {@link ParameterBinding}. - * + * * @param accessor must not be {@literal null}. * @param parameters * @param binding must not be {@literal null}. @@ -148,7 +173,7 @@ class ExpressionEvaluatingParameterBinder { : accessor.getBindableValue(binding.getParameterIndex()); if (value instanceof String && binding.isQuoted()) { - return (String) value; + return ((String) value).startsWith("{") ? (String) value : ((String) value).replace("\"", "\\\""); } if (value instanceof byte[]) { @@ -167,7 +192,7 @@ class ExpressionEvaluatingParameterBinder { /** * Evaluates the given {@code expressionString}. - * + * * @param expressionString must not be {@literal null} or empty. * @param parameters must not be {@literal null}. * @param parameterValues must not be {@literal null}. @@ -181,6 +206,40 @@ class ExpressionEvaluatingParameterBinder { return expression.getValue(evaluationContext, Object.class); } + /** + * Creates a replacement {@link Pattern} for all {@link ParameterBinding#getParameter() binding parameters} including + * a potentially trailing quotation mark. + * + * @param bindings + * @return + */ + private Pattern createReplacementPattern(List bindings) { + + StringBuilder regex = new StringBuilder(); + for (ParameterBinding binding : bindings) { + regex.append("|"); + regex.append(Pattern.quote(binding.getParameter())); + regex.append("['\"]?"); // potential quotation char (as in { foo : '?0' }). + } + + return Pattern.compile(regex.substring(1)); + } + + /** + * 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}. + * @return + */ + private String extractPlaceholder(String groupName) { + + if (!groupName.endsWith("'") && !groupName.endsWith("\"")) { + return groupName; + } + return groupName.substring(0, groupName.length() - 1); + } + /** * @author Christoph Strobl * @since 1.9 @@ -188,18 +247,18 @@ class ExpressionEvaluatingParameterBinder { static class BindingContext { final MongoParameters parameters; - final List bindings; + final Map bindings; /** * Creates new {@link BindingContext}. - * + * * @param parameters * @param bindings */ public BindingContext(MongoParameters parameters, List bindings) { this.parameters = parameters; - this.bindings = bindings; + this.bindings = mapBindings(bindings); } /** @@ -211,11 +270,28 @@ class ExpressionEvaluatingParameterBinder { /** * Get unmodifiable list of {@link ParameterBinding}s. - * + * * @return never {@literal null}. */ public List getBindings() { - return Collections.unmodifiableList(bindings); + return new ArrayList(bindings.values()); + } + + /** + * Get the concrete {@link ParameterBinding} for a given {@literal placeholder}. + * + * @param placeholder must not be {@literal null}. + * @return + * @throws java.util.NoSuchElementException + * @since 1.10 + */ + ParameterBinding getBindingFor(String placeholder) { + + if (!bindings.containsKey(placeholder)) { + throw new NoSuchElementException(String.format("Could not to find binding for placeholder '%s'.", placeholder)); + } + + return bindings.get(placeholder); } /** @@ -227,5 +303,13 @@ class ExpressionEvaluatingParameterBinder { return parameters; } + private static Map mapBindings(List bindings) { + + Map map = new LinkedHashMap(bindings.size(), 1); + for (ParameterBinding binding : bindings) { + map.put(binding.getParameter(), binding); + } + return map; + } } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java index 41e0c6ef8..bef39f330 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java @@ -54,6 +54,7 @@ import com.mongodb.BasicDBObject; import com.mongodb.BasicDBObjectBuilder; import com.mongodb.DBObject; import com.mongodb.DBRef; +import com.mongodb.util.JSON; /** * Unit tests for {@link StringBasedMongoQuery}. @@ -178,8 +179,8 @@ public class StringBasedMongoQueryUnitTests { @Test public void shouldSupportFindByParameterizedCriteriaAndFields() throws Exception { - ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, new Object[] { - new BasicDBObject("firstname", "first").append("lastname", "last"), Collections.singletonMap("lastname", 1) }); + ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, + new BasicDBObject("firstname", "first").append("lastname", "last"), Collections.singletonMap("lastname", 1)); StringBasedMongoQuery mongoQuery = createQueryForMethod("findByParameterizedCriteriaAndFields", DBObject.class, Map.class); @@ -362,6 +363,96 @@ public class StringBasedMongoQueryUnitTests { assertThat(query.getQueryObject(), is(reference.getQueryObject())); } + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldIgnorePlaceholderPatternInReplacementValue() throws Exception { + + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "argWith?1andText", + "nothing-special"); + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByStringWithWildcardChar", String.class, String.class); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is(JSON.parse("{ \"arg0\" : \"argWith?1andText\" , \"arg1\" : \"nothing-special\"}"))); + } + + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldQuoteStringReplacementCorrectly() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "Matthews', password: 'foo"); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is(not(new BasicDBObjectBuilder().add("lastname", "Matthews").add("password", "foo").get()))); + assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject("lastname", "Matthews', password: 'foo"))); + } + + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldQuoteStringReplacementContainingQuotesCorrectly() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "Matthews\", password: \"foo"); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is(not(new BasicDBObjectBuilder().add("lastname", "Matthews").add("password", "foo").get()))); + assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject("lastname", "Matthews\", password: \"foo"))); + } + + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldQuoteStringReplacementWithQuotationsCorrectly() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, + "\"Dave Matthews\", password: 'foo"); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is((DBObject) new BasicDBObject("lastname", "\"Dave Matthews\", password: 'foo"))); + } + + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldQuoteComplexQueryStringCorreclty() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "{ $ne : \"calamity\" }"); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is((DBObject) new BasicDBObject("lastname", new BasicDBObject("$ne", "calamity")))); + } + + /** + * @see DATAMONGO-1565 + */ + @Test + public void shouldQuotationInQuotedComplexQueryString() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, + "{ $ne : \"\\\"calamity\\\"\" }"); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + assertThat(query.getQueryObject(), + is((DBObject) new BasicDBObject("lastname", new BasicDBObject("$ne", "\"calamity\"")))); + } + private StringBasedMongoQuery createQueryForMethod(String name, Class... parameters) throws Exception { Method method = SampleRepository.class.getMethod(name, parameters); @@ -420,5 +511,8 @@ public class StringBasedMongoQueryUnitTests { @Query("{'id':?#{ [0] ? { $exists :true} : [1] }, 'foo':42, 'bar': ?#{ [0] ? { $exists :false} : [1] }}") List findByQueryWithExpressionAndMultipleNestedObjects(boolean param0, String param1, String param2); + + @Query("{ 'arg0' : ?0, 'arg1' : ?1 }") + List findByStringWithWildcardChar(String arg0, String arg1); } }