Skip to content

Commit f8fbf77

Browse files
sothawosothawo
authored
Escape strings with quotes in custom query parameters. (#1793)
Original Pull Request #1793 Closes #1790
1 parent a2ca312 commit f8fbf77

File tree

5 files changed

+109
-59
lines changed

5 files changed

+109
-59
lines changed

Diff for: ‎src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java

+2-34
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,16 @@
1515
*/
1616
package org.springframework.data.elasticsearch.repository.query;
1717

18-
import java.util.regex.Matcher;
19-
import java.util.regex.Pattern;
20-
21-
import org.springframework.core.convert.support.GenericConversionService;
2218
import org.springframework.data.domain.PageRequest;
2319
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
2420
import org.springframework.data.elasticsearch.core.SearchHitSupport;
2521
import org.springframework.data.elasticsearch.core.SearchHits;
26-
import org.springframework.data.elasticsearch.core.convert.DateTimeConverters;
2722
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
2823
import org.springframework.data.elasticsearch.core.query.StringQuery;
24+
import org.springframework.data.elasticsearch.repository.support.StringQueryUtil;
2925
import org.springframework.data.repository.query.ParametersParameterAccessor;
3026
import org.springframework.data.util.StreamUtils;
3127
import org.springframework.util.Assert;
32-
import org.springframework.util.ClassUtils;
33-
import org.springframework.util.NumberUtils;
3428

3529
/**
3630
* ElasticsearchStringQuery
@@ -43,11 +37,8 @@
4337
*/
4438
public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQuery {
4539

46-
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
4740
private String query;
4841

49-
private final GenericConversionService conversionService = new GenericConversionService();
50-
5142
public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations,
5243
String query) {
5344
super(queryMethod, elasticsearchOperations);
@@ -104,31 +95,8 @@ public Object execute(Object[] parameters) {
10495
}
10596

10697
protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) {
107-
String queryString = replacePlaceholders(this.query, parameterAccessor);
98+
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
10899
return new StringQuery(queryString);
109100
}
110101

111-
private String replacePlaceholders(String input, ParametersParameterAccessor accessor) {
112-
113-
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
114-
String result = input;
115-
while (matcher.find()) {
116-
117-
String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)";
118-
int index = NumberUtils.parseNumber(matcher.group(1), Integer.class);
119-
result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index));
120-
}
121-
return result;
122-
}
123-
124-
private String getParameterWithIndex(ParametersParameterAccessor accessor, int index) {
125-
Object parameter = accessor.getBindableValue(index);
126-
if (parameter == null) {
127-
return "null";
128-
}
129-
if (conversionService.canConvert(parameter.getClass(), String.class)) {
130-
return conversionService.convert(parameter, String.class);
131-
}
132-
return parameter.toString();
133-
}
134102
}

Diff for: ‎src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java

+2-24
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,11 @@
1515
*/
1616
package org.springframework.data.elasticsearch.repository.query;
1717

18-
import java.util.regex.Matcher;
19-
import java.util.regex.Pattern;
20-
2118
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
2219
import org.springframework.data.elasticsearch.core.query.StringQuery;
20+
import org.springframework.data.elasticsearch.repository.support.StringQueryUtil;
2321
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
2422
import org.springframework.expression.spel.standard.SpelExpressionParser;
25-
import org.springframework.util.NumberUtils;
26-
import org.springframework.util.ObjectUtils;
2723

2824
/**
2925
* @author Christoph Strobl
@@ -32,7 +28,6 @@
3228
*/
3329
public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsearchRepositoryQuery {
3430

35-
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
3631
private final String query;
3732

3833
public ReactiveElasticsearchStringQuery(ReactiveElasticsearchQueryMethod queryMethod,
@@ -52,27 +47,10 @@ public ReactiveElasticsearchStringQuery(String query, ReactiveElasticsearchQuery
5247

5348
@Override
5449
protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) {
55-
String queryString = replacePlaceholders(this.query, parameterAccessor);
50+
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
5651
return new StringQuery(queryString);
5752
}
5853

59-
private String replacePlaceholders(String input, ElasticsearchParameterAccessor accessor) {
60-
61-
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
62-
String result = input;
63-
while (matcher.find()) {
64-
65-
String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)";
66-
int index = NumberUtils.parseNumber(matcher.group(1), Integer.class);
67-
result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index));
68-
}
69-
return result;
70-
}
71-
72-
private String getParameterWithIndex(ElasticsearchParameterAccessor accessor, int index) {
73-
return ObjectUtils.nullSafeToString(accessor.getBindableValue(index));
74-
}
75-
7654
@Override
7755
boolean isCountQuery() {
7856
return queryMethod.hasCountQueryAnnotation();

Diff for: ‎src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright 2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.elasticsearch.repository.support;
17+
18+
import java.util.regex.Matcher;
19+
import java.util.regex.Pattern;
20+
21+
import org.springframework.core.convert.support.GenericConversionService;
22+
import org.springframework.data.repository.query.ParameterAccessor;
23+
import org.springframework.util.NumberUtils;
24+
25+
/**
26+
* @author Peter-Josef Meisch
27+
*/
28+
final public class StringQueryUtil {
29+
30+
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
31+
private static final GenericConversionService conversionService = new GenericConversionService();
32+
33+
private StringQueryUtil() {}
34+
35+
public static String replacePlaceholders(String input, ParameterAccessor accessor) {
36+
37+
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
38+
String result = input;
39+
while (matcher.find()) {
40+
41+
String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)";
42+
int index = NumberUtils.parseNumber(matcher.group(1), Integer.class);
43+
result = result.replaceAll(placeholder, Matcher.quoteReplacement(getParameterWithIndex(accessor, index)));
44+
}
45+
return result;
46+
}
47+
48+
private static String getParameterWithIndex(ParameterAccessor accessor, int index) {
49+
50+
Object parameter = accessor.getBindableValue(index);
51+
String parameterValue = "null";
52+
53+
// noinspection ConstantConditions
54+
if (parameter != null) {
55+
56+
if (conversionService.canConvert(parameter.getClass(), String.class)) {
57+
String converted = conversionService.convert(parameter, String.class);
58+
59+
if (converted != null) {
60+
parameterValue = converted;
61+
}
62+
} else {
63+
parameterValue = parameter.toString();
64+
}
65+
}
66+
67+
parameterValue = parameterValue.replaceAll("\"", Matcher.quoteReplacement("\\\""));
68+
return parameterValue;
69+
70+
}
71+
72+
}

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java

+16-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Map;
2626

2727
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.DisplayName;
2829
import org.junit.jupiter.api.Test;
2930
import org.junit.jupiter.api.extension.ExtendWith;
3031
import org.mockito.Mock;
@@ -37,6 +38,7 @@
3738
import org.springframework.data.elasticsearch.annotations.MultiField;
3839
import org.springframework.data.elasticsearch.annotations.Query;
3940
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
41+
import org.springframework.data.elasticsearch.core.SearchHits;
4042
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
4143
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
4244
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
@@ -82,6 +84,17 @@ public void shouldReplaceRepeatedParametersCorrectly() throws Exception {
8284
.isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)");
8385
}
8486

87+
@Test // #1790
88+
@DisplayName("should escape Strings in query parameters")
89+
void shouldEscapeStringsInQueryParameters() throws Exception {
90+
91+
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByPrefix", "hello \"Stranger\"");
92+
93+
assertThat(query).isInstanceOf(StringQuery.class);
94+
assertThat(((StringQuery) query).getSource())
95+
.isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}");
96+
}
97+
8598
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args)
8699
throws NoSuchMethodException {
87100

@@ -90,7 +103,6 @@ private org.springframework.data.elasticsearch.core.query.Query createQuery(Stri
90103
ElasticsearchStringQuery elasticsearchStringQuery = queryForMethod(queryMethod);
91104
return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
92105
}
93-
94106
private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) {
95107
return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery());
96108
}
@@ -110,6 +122,9 @@ private interface SampleRepository extends Repository<Person, String> {
110122
@Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)")
111123
Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5,
112124
String arg6, String arg7, String arg8, String arg9, String arg10, String arg11);
125+
126+
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
127+
SearchHits<Book> findByPrefix(String prefix);
113128
}
114129

115130
/**

Diff for: ‎src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java

+17
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import org.junit.jupiter.api.BeforeEach;
3131
import org.junit.jupiter.api.Disabled;
32+
import org.junit.jupiter.api.DisplayName;
3233
import org.junit.jupiter.api.Test;
3334
import org.junit.jupiter.api.extension.ExtendWith;
3435
import org.mockito.Mock;
@@ -41,6 +42,7 @@
4142
import org.springframework.data.elasticsearch.annotations.MultiField;
4243
import org.springframework.data.elasticsearch.annotations.Query;
4344
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
45+
import org.springframework.data.elasticsearch.core.SearchHit;
4446
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
4547
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
4648
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
@@ -119,6 +121,17 @@ public void shouldReplaceRepeatedParametersCorrectly() throws Exception {
119121
.isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)");
120122
}
121123

124+
@Test // #1790
125+
@DisplayName("should escape Strings in query parameters")
126+
void shouldEscapeStringsInQueryParameters() throws Exception {
127+
128+
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByPrefix", "hello \"Stranger\"");
129+
130+
assertThat(query).isInstanceOf(StringQuery.class);
131+
assertThat(((StringQuery) query).getSource())
132+
.isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}");
133+
}
134+
122135
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args)
123136
throws NoSuchMethodException {
124137

@@ -163,6 +176,10 @@ Person findWithQuiteSomeParameters(String arg0, String arg1, String arg2, String
163176
@Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)")
164177
Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5,
165178
String arg6, String arg7, String arg8, String arg9, String arg10, String arg11);
179+
180+
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
181+
Flux<SearchHit<Book>> findByPrefix(String prefix);
182+
166183
}
167184

168185
/**

0 commit comments

Comments
 (0)