DATAMONGO-1565 - Ignore placeholder pattern in replacement values for annotated queries.

We now make sure to quote single and double ticks in the replacement values before actually appending them to the query. We also replace single ticks around parameters in the actual raw annotated query by double quotes to make sure they are treated as a single string parameter.
This commit is contained in:
Christoph Strobl
2016-12-15 10:37:16 +01:00
committed by Oliver Gierke
parent dc44c3a455
commit 536dcc14ca
2 changed files with 223 additions and 45 deletions

View File

@@ -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<ParameterBinding> 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<ParameterBinding> bindings;
final Map<String, ParameterBinding> bindings;
/**
* Creates new {@link BindingContext}.
*
*
* @param parameters
* @param bindings
*/
public BindingContext(MongoParameters parameters, List<ParameterBinding> 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<ParameterBinding> getBindings() {
return Collections.unmodifiableList(bindings);
return new ArrayList<ParameterBinding>(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<String, ParameterBinding> mapBindings(List<ParameterBinding> bindings) {
Map<String, ParameterBinding> map = new LinkedHashMap<String, ParameterBinding>(bindings.size(), 1);
for (ParameterBinding binding : bindings) {
map.put(binding.getParameter(), binding);
}
return map;
}
}
}

View File

@@ -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<Person> findByQueryWithExpressionAndMultipleNestedObjects(boolean param0, String param1, String param2);
@Query("{ 'arg0' : ?0, 'arg1' : ?1 }")
List<Person> findByStringWithWildcardChar(String arg0, String arg1);
}
}