From f8fbf7721a23d291346bf500bdeb83ff36ef6559 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Wed, 28 Apr 2021 21:46:10 +0200 Subject: [PATCH] Escape strings with quotes in custom query parameters. (#1793) Original Pull Request #1793 Closes #1790 --- .../query/ElasticsearchStringQuery.java | 36 +--------- .../ReactiveElasticsearchStringQuery.java | 26 +------ .../repository/support/StringQueryUtil.java | 72 +++++++++++++++++++ .../ElasticsearchStringQueryUnitTests.java | 17 ++++- ...tiveElasticsearchStringQueryUnitTests.java | 17 +++++ 5 files changed, 109 insertions(+), 59 deletions(-) create mode 100644 src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index 32733c295..81cc926b0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -15,22 +15,16 @@ */ package org.springframework.data.elasticsearch.repository.query; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.domain.PageRequest; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.SearchHitSupport; import org.springframework.data.elasticsearch.core.SearchHits; -import org.springframework.data.elasticsearch.core.convert.DateTimeConverters; import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates; import org.springframework.data.elasticsearch.core.query.StringQuery; +import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.util.StreamUtils; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; -import org.springframework.util.NumberUtils; /** * ElasticsearchStringQuery @@ -43,11 +37,8 @@ import org.springframework.util.NumberUtils; */ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQuery { - private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)"); private String query; - private final GenericConversionService conversionService = new GenericConversionService(); - public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations, String query) { super(queryMethod, elasticsearchOperations); @@ -104,31 +95,8 @@ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQue } protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) { - String queryString = replacePlaceholders(this.query, parameterAccessor); + String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor); return new StringQuery(queryString); } - private String replacePlaceholders(String input, ParametersParameterAccessor accessor) { - - Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input); - String result = input; - while (matcher.find()) { - - String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)"; - int index = NumberUtils.parseNumber(matcher.group(1), Integer.class); - result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index)); - } - return result; - } - - private String getParameterWithIndex(ParametersParameterAccessor accessor, int index) { - Object parameter = accessor.getBindableValue(index); - if (parameter == null) { - return "null"; - } - if (conversionService.canConvert(parameter.getClass(), String.class)) { - return conversionService.convert(parameter, String.class); - } - return parameter.toString(); - } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java index a20625825..110a2d8ba 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java @@ -15,15 +15,11 @@ */ package org.springframework.data.elasticsearch.repository.query; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations; import org.springframework.data.elasticsearch.core.query.StringQuery; +import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.util.NumberUtils; -import org.springframework.util.ObjectUtils; /** * @author Christoph Strobl @@ -32,7 +28,6 @@ import org.springframework.util.ObjectUtils; */ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsearchRepositoryQuery { - private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)"); private final String query; public ReactiveElasticsearchStringQuery(ReactiveElasticsearchQueryMethod queryMethod, @@ -52,27 +47,10 @@ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsea @Override protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) { - String queryString = replacePlaceholders(this.query, parameterAccessor); + String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor); return new StringQuery(queryString); } - private String replacePlaceholders(String input, ElasticsearchParameterAccessor accessor) { - - Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input); - String result = input; - while (matcher.find()) { - - String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)"; - int index = NumberUtils.parseNumber(matcher.group(1), Integer.class); - result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index)); - } - return result; - } - - private String getParameterWithIndex(ElasticsearchParameterAccessor accessor, int index) { - return ObjectUtils.nullSafeToString(accessor.getBindableValue(index)); - } - @Override boolean isCountQuery() { return queryMethod.hasCountQueryAnnotation(); diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java new file mode 100644 index 000000000..27ad71b47 --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java @@ -0,0 +1,72 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.elasticsearch.repository.support; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.springframework.core.convert.support.GenericConversionService; +import org.springframework.data.repository.query.ParameterAccessor; +import org.springframework.util.NumberUtils; + +/** + * @author Peter-Josef Meisch + */ +final public class StringQueryUtil { + + private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)"); + private static final GenericConversionService conversionService = new GenericConversionService(); + + private StringQueryUtil() {} + + public static String replacePlaceholders(String input, ParameterAccessor accessor) { + + Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input); + String result = input; + while (matcher.find()) { + + String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)"; + int index = NumberUtils.parseNumber(matcher.group(1), Integer.class); + result = result.replaceAll(placeholder, Matcher.quoteReplacement(getParameterWithIndex(accessor, index))); + } + return result; + } + + private static String getParameterWithIndex(ParameterAccessor accessor, int index) { + + Object parameter = accessor.getBindableValue(index); + String parameterValue = "null"; + + // noinspection ConstantConditions + if (parameter != null) { + + if (conversionService.canConvert(parameter.getClass(), String.class)) { + String converted = conversionService.convert(parameter, String.class); + + if (converted != null) { + parameterValue = converted; + } + } else { + parameterValue = parameter.toString(); + } + } + + parameterValue = parameterValue.replaceAll("\"", Matcher.quoteReplacement("\\\"")); + return parameterValue; + + } + +} diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java index 6e63773a1..d1c221b9f 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -37,6 +38,7 @@ import org.springframework.data.elasticsearch.annotations.InnerField; import org.springframework.data.elasticsearch.annotations.MultiField; import org.springframework.data.elasticsearch.annotations.Query; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; +import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter; import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter; import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; @@ -82,6 +84,17 @@ public class ElasticsearchStringQueryUnitTests { .isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)"); } + @Test // #1790 + @DisplayName("should escape Strings in query parameters") + void shouldEscapeStringsInQueryParameters() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByPrefix", "hello \"Stranger\""); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}"); + } + private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args) throws NoSuchMethodException { @@ -90,7 +103,6 @@ public class ElasticsearchStringQueryUnitTests { ElasticsearchStringQuery elasticsearchStringQuery = queryForMethod(queryMethod); return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args)); } - private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) { return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery()); } @@ -110,6 +122,9 @@ public class ElasticsearchStringQueryUnitTests { @Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)") Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5, String arg6, String arg7, String arg8, String arg9, String arg10, String arg11); + + @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") + SearchHits findByPrefix(String prefix); } /** diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java index bf17c3c39..9a24e0af3 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java @@ -29,6 +29,7 @@ import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -41,6 +42,7 @@ import org.springframework.data.elasticsearch.annotations.InnerField; import org.springframework.data.elasticsearch.annotations.MultiField; import org.springframework.data.elasticsearch.annotations.Query; import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations; +import org.springframework.data.elasticsearch.core.SearchHit; import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter; import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter; import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; @@ -119,6 +121,17 @@ public class ReactiveElasticsearchStringQueryUnitTests { .isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)"); } + @Test // #1790 + @DisplayName("should escape Strings in query parameters") + void shouldEscapeStringsInQueryParameters() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByPrefix", "hello \"Stranger\""); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}"); + } + private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args) throws NoSuchMethodException { @@ -163,6 +176,10 @@ public class ReactiveElasticsearchStringQueryUnitTests { @Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)") Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5, String arg6, String arg7, String arg8, String arg9, String arg10, String arg11); + + @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") + Flux> findByPrefix(String prefix); + } /**