Skip to content

Commit 3677d8f

Browse files
nibixnibix
authored
Introduced new setting search.query.max_query_string_length (#19491)
* Introduced new setting search.query.max_query_string_length Signed-off-by: Nils Bandener <nils.bandener@eliatra.com> * Added changelog Signed-off-by: Nils Bandener <nils.bandener@eliatra.com> * Initialize with proper default to fix unit tests Signed-off-by: Nils Bandener <nils.bandener@eliatra.com> * Use setting default for static initialization value Signed-off-by: Nils Bandener <nils.bandener@eliatra.com> --------- Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>
1 parent 32b6873 commit 3677d8f

File tree

5 files changed

+66
-0
lines changed

5 files changed

+66
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4646
- Implement GRPC Nested query ([#19453](https://github.com/opensearch-project/OpenSearch/pull/19453))
4747
- Add sub aggregation support for histogram aggregation using skiplist ([19438](https://github.com/opensearch-project/OpenSearch/pull/19438))
4848
- Optimization in String Terms Aggregation query for Large Bucket Counts([#18732](https://github.com/opensearch-project/OpenSearch/pull/18732))
49+
- New cluster setting search.query.max_query_string_length ([#19491](https://github.com/opensearch-project/OpenSearch/pull/19491))
4950

5051
### Changed
5152
- Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965))

‎server/src/internalClusterTest/java/org/opensearch/search/query/SimpleQueryStringIT.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import static org.opensearch.index.query.QueryBuilders.termQuery;
8181
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
8282
import static org.opensearch.search.SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING;
83+
import static org.opensearch.search.SearchService.SEARCH_MAX_QUERY_STRING_LENGTH;
8384
import static org.opensearch.test.StreamsUtils.copyToStringFromClasspath;
8485
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
8586
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertFailures;
@@ -767,6 +768,34 @@ public void testDynamicClauseCountUpdate() throws Exception {
767768
);
768769
}
769770

771+
public void testMaxQueryStringLength() throws Exception {
772+
try {
773+
String indexBody = copyToStringFromClasspath("/org/opensearch/search/query/all-query-index.json");
774+
assertAcked(prepareCreate("test").setSource(indexBody, MediaTypeRegistry.JSON));
775+
ensureGreen("test");
776+
777+
assertAcked(
778+
client().admin()
779+
.cluster()
780+
.prepareUpdateSettings()
781+
.setTransientSettings(Settings.builder().put(SEARCH_MAX_QUERY_STRING_LENGTH.getKey(), 10))
782+
);
783+
784+
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> {
785+
client().prepareSearch("test").setQuery(queryStringQuery("foo OR foo OR foo OR foo")).get();
786+
});
787+
788+
assertThat(e.getDetailedMessage(), containsString("Query string length exceeds max allowed length 10"));
789+
} finally {
790+
assertAcked(
791+
client().admin()
792+
.cluster()
793+
.prepareUpdateSettings()
794+
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH.getKey()))
795+
);
796+
}
797+
}
798+
770799
private void assertHits(SearchHits hits, String... ids) {
771800
assertThat(hits.getTotalHits().value(), equalTo((long) ids.length));
772801
Set<String> hitIds = new HashSet<>();

‎server/src/main/java/org/opensearch/common/settings/ClusterSettings.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ public void apply(Settings value, Settings current, Settings previous) {
570570
SearchService.MAX_AGGREGATION_REWRITE_FILTERS,
571571
SearchService.AGGREGATION_REWRITE_FILTER_SEGMENT_THRESHOLD,
572572
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
573+
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH,
573574
SearchService.CARDINALITY_AGGREGATION_PRUNING_THRESHOLD,
574575
SearchService.KEYWORD_INDEX_OR_DOC_VALUES_ENABLED,
575576
CreatePitController.PIT_INIT_KEEP_ALIVE,

‎server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
6161
import org.opensearch.common.lucene.search.Queries;
6262
import org.opensearch.common.regex.Regex;
63+
import org.opensearch.common.settings.Settings;
6364
import org.opensearch.common.unit.Fuzziness;
6465
import org.opensearch.common.util.io.IOUtils;
6566
import org.opensearch.index.IndexSettings;
@@ -71,6 +72,7 @@
7172
import org.opensearch.index.query.ExistsQueryBuilder;
7273
import org.opensearch.index.query.MultiMatchQueryBuilder;
7374
import org.opensearch.index.query.QueryShardContext;
75+
import org.opensearch.search.SearchService;
7476

7577
import java.io.IOException;
7678
import java.time.ZoneId;
@@ -96,6 +98,8 @@
9698
*/
9799
public class QueryStringQueryParser extends XQueryParser {
98100
private static final String EXISTS_FIELD = "_exists_";
101+
@SuppressWarnings("NonFinalStaticField")
102+
private static int maxQueryStringLength = SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.get(Settings.EMPTY);
99103

100104
private final QueryShardContext context;
101105
private final Map<String, Float> fieldsAndWeights;
@@ -867,6 +871,23 @@ public Query parse(String query) throws ParseException {
867871
if (query.trim().isEmpty()) {
868872
return Queries.newMatchNoDocsQuery("Matching no documents because no terms present");
869873
}
874+
if (query.length() > maxQueryStringLength) {
875+
throw new ParseException(
876+
"Query string length exceeds max allowed length "
877+
+ maxQueryStringLength
878+
+ " ("
879+
+ SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey()
880+
+ "); actual length: "
881+
+ query.length()
882+
);
883+
}
870884
return super.parse(query);
871885
}
886+
887+
/**
888+
* Sets the maximum allowed length for query strings. This should be only called from SearchService on settings updates.
889+
*/
890+
public static void setMaxQueryStringLength(int maxQueryStringLength) {
891+
QueryStringQueryParser.maxQueryStringLength = maxQueryStringLength;
892+
}
872893
}

‎server/src/main/java/org/opensearch/search/SearchService.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
import org.opensearch.index.query.QueryRewriteContext;
9494
import org.opensearch.index.query.QueryShardContext;
9595
import org.opensearch.index.query.Rewriteable;
96+
import org.opensearch.index.search.QueryStringQueryParser;
9697
import org.opensearch.index.shard.IndexEventListener;
9798
import org.opensearch.index.shard.IndexShard;
9899
import org.opensearch.index.shard.SearchOperationListener;
@@ -369,6 +370,15 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
369370
Setting.Property.Dynamic
370371
);
371372

373+
public static final Setting<Integer> SEARCH_MAX_QUERY_STRING_LENGTH = Setting.intSetting(
374+
"search.query.max_query_string_length",
375+
32_000,
376+
1,
377+
Integer.MAX_VALUE,
378+
Setting.Property.NodeScope,
379+
Setting.Property.Dynamic
380+
);
381+
372382
public static final Setting<Boolean> CLUSTER_ALLOW_DERIVED_FIELD_SETTING = Setting.boolSetting(
373383
"search.derived_field.enabled",
374384
true,
@@ -524,6 +534,10 @@ public SearchService(
524534
IndexSearcher.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
525535
clusterService.getClusterSettings().addSettingsUpdateConsumer(INDICES_MAX_CLAUSE_COUNT_SETTING, IndexSearcher::setMaxClauseCount);
526536

537+
QueryStringQueryParser.setMaxQueryStringLength(SEARCH_MAX_QUERY_STRING_LENGTH.get(settings));
538+
clusterService.getClusterSettings()
539+
.addSettingsUpdateConsumer(SEARCH_MAX_QUERY_STRING_LENGTH, QueryStringQueryParser::setMaxQueryStringLength);
540+
527541
allowDerivedField = CLUSTER_ALLOW_DERIVED_FIELD_SETTING.get(settings);
528542
clusterService.getClusterSettings().addSettingsUpdateConsumer(CLUSTER_ALLOW_DERIVED_FIELD_SETTING, this::setAllowDerivedField);
529543

0 commit comments

Comments
 (0)