From 5c0be06298ac7f14378fd3c4f7619aa3378c75c8 Mon Sep 17 00:00:00 2001
From: Shinsuke Sugaya <shinsuke@apache.org>
Date: Wed, 10 Mar 2021 07:13:59 +0900
Subject: [PATCH] fix #2542 send escaped query
---
.../java/org/codelibs/fess/Constants.java | 2 +-
.../codelibs/fess/helper/SearchHelper.java | 88 ++++++++++---------
.../fess/util/QueryStringBuilder.java | 36 +++++++-
.../fess/util/QueryStringBuilderTest.java | 20 +++--
4 files changed, 96 insertions(+), 50 deletions(-)
diff --git a/src/main/java/org/codelibs/fess/Constants.java b/src/main/java/org/codelibs/fess/Constants.java
index 881809e8e..2efb39379 100644
--- a/src/main/java/org/codelibs/fess/Constants.java
+++ b/src/main/java/org/codelibs/fess/Constants.java
@@ -251,7 +251,7 @@ public class Constants extends CoreLibConstants {
public static final String FTP = "FTP";
public static final String[] RESERVED =
- { "+", "-", "&&", "||", "!", "(", ")", "{", "}", "[", "]", "^", "~", "*", "?", "\\", ";", ":", "/" };
+ { "\\", "+", "-", "&&", "||", "!", "(", ")", "{", "}", "[", "]", "^", "~", "*", "?", ";", ":", "/" };
public static final Pattern LUCENE_FIELD_RESERVED_PATTERN = Pattern.compile("([+\\-!\\(\\){}\\[\\]^\"~\\\\:\\p{Zs}]|(&&)|(\\|\\|))"); // "*", "?",
diff --git a/src/main/java/org/codelibs/fess/helper/SearchHelper.java b/src/main/java/org/codelibs/fess/helper/SearchHelper.java
index f57d607b3..a9a6d445c 100644
--- a/src/main/java/org/codelibs/fess/helper/SearchHelper.java
+++ b/src/main/java/org/codelibs/fess/helper/SearchHelper.java
@@ -49,6 +49,7 @@ import org.codelibs.fess.entity.SearchRequestParams;
import org.codelibs.fess.entity.SearchRequestParams.SearchRequestType;
import org.codelibs.fess.es.client.SearchEngineClient.SearchConditionBuilder;
import org.codelibs.fess.es.client.SearchEngineClientException;
+import org.codelibs.fess.exception.InvalidQueryException;
import org.codelibs.fess.mylasta.action.FessUserBean;
import org.codelibs.fess.mylasta.direction.FessConfig;
import org.codelibs.fess.util.BooleanFunction;
@@ -81,39 +82,18 @@ public class SearchHelper {
request.setAttribute(Constants.REQUEST_QUERIES, params.getQuery());
});
- final int pageStart = params.getStartPosition();
- final int pageSize = params.getPageSize();
- final String sortField = params.getSort();
- final String query;
- if (StringUtil.isBlank(sortField)) {
- query = ComponentUtil.getQueryStringBuilder().params(params).build();
- } else {
- query = ComponentUtil.getQueryStringBuilder().params(params).build() + " sort:" + sortField;
+ String query = ComponentUtil.getQueryStringBuilder().params(params).sortField(params.getSort()).build();
+ List<Map<String, Object>> documentItems;
+ try {
+ documentItems = searchInternal(query, params, userBean);
+ } catch (final InvalidQueryException e) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Invalid query: {}", query, e);
+ }
+ query = ComponentUtil.getQueryStringBuilder().params(params).sortField(params.getSort()).escape(true).build();
+ documentItems = searchInternal(query, params, userBean);
}
- final FessConfig fessConfig = ComponentUtil.getFessConfig();
- final QueryHelper queryHelper = ComponentUtil.getQueryHelper();
- final List<Map<String, Object>> documentItems =
- ComponentUtil.getSearchEngineClient().search(fessConfig.getIndexDocumentSearchIndex(), searchRequestBuilder -> {
- queryHelper.processSearchPreference(searchRequestBuilder, userBean, query);
- return SearchConditionBuilder.builder(searchRequestBuilder).query(query).offset(pageStart).size(pageSize)
- .facetInfo(params.getFacetInfo()).geoInfo(params.getGeoInfo()).highlightInfo(params.getHighlightInfo())
- .similarDocHash(params.getSimilarDocHash()).responseFields(queryHelper.getResponseFields())
- .searchRequestType(params.getType()).trackTotalHits(params.getTrackTotalHits()).build();
- }, (searchRequestBuilder, execTime, searchResponse) -> {
- searchResponse.ifPresent(r -> {
- if (r.getTotalShards() != r.getSuccessfulShards() && fessConfig.isQueryTimeoutLogging()) {
- // partial results
- final StringBuilder buf = new StringBuilder(1000);
- buf.append("[SEARCH TIMEOUT] {\"exec_time\":").append(execTime)//
- .append(",\"request\":").append(searchRequestBuilder.toString())//
- .append(",\"response\":").append(r.toString()).append('}');
- logger.warn(buf.toString());
- }
- });
- final QueryResponseList queryResponseList = ComponentUtil.getQueryResponseList();
- queryResponseList.init(searchResponse, pageStart, pageSize);
- return queryResponseList;
- });
+
data.setDocumentItems(documentItems);
// search
@@ -142,7 +122,7 @@ public class SearchHelper {
}
data.setExecTime(execTime);
- final String queryId = queryHelper.generateId();
+ final String queryId = ComponentUtil.getQueryHelper().generateId();
data.setPageSize(queryResponseList.getPageSize());
data.setCurrentPageNumber(queryResponseList.getCurrentPageNumber());
@@ -160,10 +140,12 @@ public class SearchHelper {
data.setRequestedTime(requestedTime);
data.setQueryId(queryId);
+ final FessConfig fessConfig = ComponentUtil.getFessConfig();
+
// search log
if (fessConfig.isSearchLog()) {
- ComponentUtil.getSearchLogHelper().addSearchLog(params, DfTypeUtil.toLocalDateTime(requestedTime), queryId, query, pageStart,
- pageSize, queryResponseList);
+ ComponentUtil.getSearchLogHelper().addSearchLog(params, DfTypeUtil.toLocalDateTime(requestedTime), queryId, query,
+ params.getStartPosition(), params.getPageSize(), queryResponseList);
}
// favorite
@@ -173,6 +155,34 @@ public class SearchHelper {
}
+ protected List<Map<String, Object>> searchInternal(final String query, final SearchRequestParams params,
+ final OptionalThing<FessUserBean> userBean) {
+ final FessConfig fessConfig = ComponentUtil.getFessConfig();
+ final QueryHelper queryHelper = ComponentUtil.getQueryHelper();
+ return ComponentUtil.getSearchEngineClient().search(fessConfig.getIndexDocumentSearchIndex(), searchRequestBuilder -> {
+ queryHelper.processSearchPreference(searchRequestBuilder, userBean, query);
+ return SearchConditionBuilder.builder(searchRequestBuilder).query(query).offset(params.getStartPosition())
+ .size(params.getPageSize()).facetInfo(params.getFacetInfo()).geoInfo(params.getGeoInfo())
+ .highlightInfo(params.getHighlightInfo()).similarDocHash(params.getSimilarDocHash())
+ .responseFields(queryHelper.getResponseFields()).searchRequestType(params.getType())
+ .trackTotalHits(params.getTrackTotalHits()).build();
+ }, (searchRequestBuilder, execTime, searchResponse) -> {
+ searchResponse.ifPresent(r -> {
+ if (r.getTotalShards() != r.getSuccessfulShards() && fessConfig.isQueryTimeoutLogging()) {
+ // partial results
+ final StringBuilder buf = new StringBuilder(1000);
+ buf.append("[SEARCH TIMEOUT] {\"exec_time\":").append(execTime)//
+ .append(",\"request\":").append(searchRequestBuilder.toString())//
+ .append(",\"response\":").append(r.toString()).append('}');
+ logger.warn(buf.toString());
+ }
+ });
+ final QueryResponseList queryResponseList = ComponentUtil.getQueryResponseList();
+ queryResponseList.init(searchResponse, params.getStartPosition(), params.getPageSize());
+ return queryResponseList;
+ });
+ }
+
public long scrollSearch(final SearchRequestParams params, final BooleanFunction<Map<String, Object>> cursor,
final OptionalThing<FessUserBean> userBean) {
LaRequestUtil.getOptionalRequest().ifPresent(request -> {
@@ -181,13 +191,7 @@ public class SearchHelper {
});
final int pageSize = params.getPageSize();
- final String sortField = params.getSort();
- final String query;
- if (StringUtil.isBlank(sortField)) {
- query = ComponentUtil.getQueryStringBuilder().params(params).build();
- } else {
- query = ComponentUtil.getQueryStringBuilder().params(params).build() + " sort:" + sortField;
- }
+ final String query = ComponentUtil.getQueryStringBuilder().params(params).sortField(params.getSort()).build();
final FessConfig fessConfig = ComponentUtil.getFessConfig();
return ComponentUtil.getSearchEngineClient().<Map<String, Object>> scrollSearch(fessConfig.getIndexDocumentSearchIndex(),
searchRequestBuilder -> {
diff --git a/src/main/java/org/codelibs/fess/util/QueryStringBuilder.java b/src/main/java/org/codelibs/fess/util/QueryStringBuilder.java
index 8bbbf4c2f..30c7f12f3 100644
--- a/src/main/java/org/codelibs/fess/util/QueryStringBuilder.java
+++ b/src/main/java/org/codelibs/fess/util/QueryStringBuilder.java
@@ -22,6 +22,7 @@ import java.util.Map;
import java.util.stream.Collectors;
import org.codelibs.core.lang.StringUtil;
+import org.codelibs.fess.Constants;
import org.codelibs.fess.entity.SearchRequestParams;
import org.codelibs.fess.helper.RelatedQueryHelper;
import org.codelibs.fess.mylasta.direction.FessConfig;
@@ -36,6 +37,10 @@ public class QueryStringBuilder {
private SearchRequestParams params;
+ private boolean escape = false;
+
+ private String sortField;
+
protected String quote(final String value) {
if (value.split("\\s").length > 1) {
return new StringBuilder().append('"').append(value.replace('"', ' ')).append('"').toString();
@@ -43,6 +48,19 @@ public class QueryStringBuilder {
return value;
}
+ protected String escapeQuery(final String value) {
+ if (!escape) {
+ return value;
+ }
+
+ String newValue = value;
+ for (int i = 0; i < Constants.RESERVED.length; i++) {
+ final String replacement = Constants.RESERVED[i].replaceAll("(.)", "\\\\$1");
+ newValue = newValue.replace(Constants.RESERVED[i], replacement);
+ }
+ return newValue;
+ }
+
public String build() {
final FessConfig fessConfig = ComponentUtil.getFessConfig();
final int maxQueryLength = fessConfig.getQueryMaxLengthAsInteger();
@@ -50,7 +68,7 @@ public class QueryStringBuilder {
final String query = buildBaseQuery();
if (StringUtil.isNotBlank(query)) {
- queryBuf.append(query);
+ queryBuf.append(escapeQuery(query));
}
stream(params.getExtraQueries())
@@ -80,7 +98,11 @@ public class QueryStringBuilder {
}
}));
- return queryBuf.toString().trim();
+ String baseQuery = queryBuf.toString().trim();
+ if (StringUtil.isBlank(sortField)) {
+ return baseQuery;
+ }
+ return baseQuery + " sort:" + sortField;
}
protected void appendQuery(final StringBuilder queryBuf, final String query) {
@@ -183,4 +205,14 @@ public class QueryStringBuilder {
this.params = params;
return this;
}
+
+ public QueryStringBuilder sortField(final String sortField) {
+ this.sortField = sortField;
+ return this;
+ }
+
+ public QueryStringBuilder escape(final boolean escape) {
+ this.escape = escape;
+ return this;
+ }
}
diff --git a/src/test/java/org/codelibs/fess/util/QueryStringBuilderTest.java b/src/test/java/org/codelibs/fess/util/QueryStringBuilderTest.java
index 19c38bfcf..e7c95d48e 100644
--- a/src/test/java/org/codelibs/fess/util/QueryStringBuilderTest.java
+++ b/src/test/java/org/codelibs/fess/util/QueryStringBuilderTest.java
@@ -28,7 +28,7 @@ import org.codelibs.fess.unit.UnitFessTestCase;
public class QueryStringBuilderTest extends UnitFessTestCase {
public void test_query() {
- assertEquals("", getQuery("", new String[0], Collections.emptyMap(), Collections.emptyMap()));
+ assertEquals("", getQuery("", new String[0], Collections.emptyMap(), Collections.emptyMap(), false));
}
public void test_conditions_q() {
@@ -69,16 +69,26 @@ public class QueryStringBuilderTest extends UnitFessTestCase {
assertEquals("NOT aaa NOT bbb", getAsQuery("111", Collections.singletonMap(k, new String[] { "aaa bbb" })));
}
+ public void test_escape() {
+ assertEquals("\\/", getQuery("/", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ assertEquals("aaa\\/bbb", getQuery("aaa/bbb", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ assertEquals("\\\\", getQuery("\\", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ assertEquals("\\\\\\\\", getQuery("\\\\", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ assertEquals("aaa \\&\\& bbb", getQuery("aaa && bbb", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ assertEquals("\\\\\\+\\-\\&\\&\\|\\|\\!\\(\\)\\{\\}\\[\\]\\^\\~\\*\\?\\;\\:\\/",
+ getQuery("\\+-&&||!(){}[]^~*?;:/", new String[0], Collections.emptyMap(), Collections.emptyMap(), true));
+ }
+
private String getAsQuery(final String query, final Map<String, String[]> conditions) {
- return getQuery(query, new String[0], Collections.emptyMap(), conditions);
+ return getQuery(query, new String[0], Collections.emptyMap(), conditions, false);
}
private String getAsQuery(final Map<String, String[]> conditions) {
- return getQuery("", new String[0], Collections.emptyMap(), conditions);
+ return getQuery("", new String[0], Collections.emptyMap(), conditions, false);
}
private String getQuery(final String query, final String[] extraQueries, final Map<String, String[]> fields,
- final Map<String, String[]> conditions) {
+ final Map<String, String[]> conditions, final boolean escape) {
return new QueryStringBuilder().params(new SearchRequestParams() {
@Override
@@ -156,6 +166,6 @@ public class QueryStringBuilderTest extends UnitFessTestCase {
return new HighlightInfo();
}
- }).build();
+ }).escape(escape).build();
}
}
--
GitLab