DATAMONGO-1575 - Escape Strings correctly.

Use regex groups and parameter index values for replacement in string based queries.
This commit is contained in:
Christoph Strobl
2016-12-22 11:21:30 +01:00
committed by Mark Paluch
parent 0e0b8d5f79
commit 18e6b9cfa7
3 changed files with 159 additions and 30 deletions

View File

@@ -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 : "");
}
}
}

View File

@@ -24,12 +24,13 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
import javax.xml.bind.DatatypeConverter;
import org.bson.BSON;
import org.bson.BsonRegularExpression;
import org.bson.Document;
import org.bson.conversions.Bson;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -46,7 +47,6 @@ import org.springframework.data.mongodb.core.query.BasicQuery;
import org.springframework.data.mongodb.repository.Address;
import org.springframework.data.mongodb.repository.Person;
import org.springframework.data.mongodb.repository.Query;
import org.springframework.data.mongodb.test.util.IsBsonObject;
import org.springframework.data.projection.ProjectionFactory;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.Repository;
@@ -54,9 +54,6 @@ import org.springframework.data.repository.core.support.DefaultRepositoryMetadat
import org.springframework.data.repository.query.DefaultEvaluationContextProvider;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import com.mongodb.DBRef;
import org.bson.Document;
/**
* Unit tests for {@link StringBasedMongoQuery}.
*
@@ -381,7 +378,7 @@ public class StringBasedMongoQueryUnitTests {
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
assertThat(query.getQueryObject(),
is(not(new Document().append("lastname", "Matthews").append("password", "foo"))));
assertThat(query.getQueryObject(), is( new Document("lastname", "Matthews\", password: \"foo")));
assertThat(query.getQueryObject(), is(new Document("lastname", "Matthews\", password: \"foo")));
}
@Test // DATAMONGO-1565
@@ -392,22 +389,20 @@ public class StringBasedMongoQueryUnitTests {
"\"Dave Matthews\", password: 'foo");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
assertThat(query.getQueryObject(),
is(new Document("lastname", "\"Dave Matthews\", password: 'foo")));
assertThat(query.getQueryObject(), is(new Document("lastname", "\"Dave Matthews\", password: 'foo")));
}
@Test // DATAMONGO-1565
@Test // DATAMONGO-1565, DATAMONGO-1575
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( new Document("lastname", new Document("$ne", "calamity"))));
assertThat(query.getQueryObject(), is(new Document("lastname", "{ $ne : \"calamity\" }")));
}
@Test // DATAMONGO-1565
@Test // DATAMONGO-1565, DATAMONGO-1575
public void shouldQuotationInQuotedComplexQueryString() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class);
@@ -416,8 +411,34 @@ public class StringBasedMongoQueryUnitTests {
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
assertThat(query.getQueryObject(),
is(new Document("lastname", new Document("$ne", "\"calamity\""))));
assertThat(query.getQueryObject(), is(new Document("lastname", "{ $ne : \"\\\"calamity\\\"\" }")));
}
/**
* @see DATAMONGO-1575
*/
@Test
public void shouldTakeBsonParameterAsIs() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByWithBsonArgument", Document.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter,
new Document("$regex", "^calamity$"));
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(), is(new Document("arg0", new BsonRegularExpression("^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(new Document("lastname", new BsonRegularExpression("^(calamity)"))));
}
private StringBasedMongoQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
@@ -440,6 +461,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);
@@ -487,5 +511,8 @@ public class StringBasedMongoQueryUnitTests {
@Query("{ 'arg0' : ?0, 'arg1' : ?1 }")
List<Person> findByStringWithWildcardChar(String arg0, String arg1);
@Query("{ 'arg0' : ?0 }")
List<Person> findByWithBsonArgument(Document arg0);
}
}

View File

@@ -368,7 +368,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,
@@ -384,6 +386,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