Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Implement GRPC Nested query ([#19453](https://github.com/opensearch-project/OpenSearch/pull/19453))
- Add sub aggregation support for histogram aggregation using skiplist ([19438](https://github.com/opensearch-project/OpenSearch/pull/19438))
- Optimization in String Terms Aggregation query for Large Bucket Counts([#18732](https://github.com/opensearch-project/OpenSearch/pull/18732))
- New cluster setting search.query.max_query_string_length ([#19491](https://github.com/opensearch-project/OpenSearch/pull/19491))

### Changed
- Refactor `if-else` chains to use `Java 17 pattern matching switch expressions`(([#18965](https://github.com/opensearch-project/OpenSearch/pull/18965))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import static org.opensearch.index.query.QueryBuilders.termQuery;
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
import static org.opensearch.search.SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING;
import static org.opensearch.search.SearchService.SEARCH_MAX_QUERY_STRING_LENGTH;
import static org.opensearch.test.StreamsUtils.copyToStringFromClasspath;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertFailures;
Expand Down Expand Up @@ -767,6 +768,34 @@ public void testDynamicClauseCountUpdate() throws Exception {
);
}

public void testMaxQueryStringLength() throws Exception {
try {
String indexBody = copyToStringFromClasspath("/org/opensearch/search/query/all-query-index.json");
assertAcked(prepareCreate("test").setSource(indexBody, MediaTypeRegistry.JSON));
ensureGreen("test");

assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(SEARCH_MAX_QUERY_STRING_LENGTH.getKey(), 10))
);

SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class, () -> {
client().prepareSearch("test").setQuery(queryStringQuery("foo OR foo OR foo OR foo")).get();
});

assertThat(e.getDetailedMessage(), containsString("Query string length exceeds max allowed length 10"));
} finally {
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH.getKey()))
);
}
}

private void assertHits(SearchHits hits, String... ids) {
assertThat(hits.getTotalHits().value(), equalTo((long) ids.length));
Set<String> hitIds = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ public void apply(Settings value, Settings current, Settings previous) {
SearchService.MAX_AGGREGATION_REWRITE_FILTERS,
SearchService.AGGREGATION_REWRITE_FILTER_SEGMENT_THRESHOLD,
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH,
SearchService.CARDINALITY_AGGREGATION_PRUNING_THRESHOLD,
SearchService.KEYWORD_INDEX_OR_DOC_VALUES_ENABLED,
CreatePitController.PIT_INIT_KEEP_ALIVE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
import org.opensearch.common.lucene.search.Queries;
import org.opensearch.common.regex.Regex;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.Fuzziness;
import org.opensearch.common.util.io.IOUtils;
import org.opensearch.index.IndexSettings;
Expand All @@ -71,6 +72,7 @@
import org.opensearch.index.query.ExistsQueryBuilder;
import org.opensearch.index.query.MultiMatchQueryBuilder;
import org.opensearch.index.query.QueryShardContext;
import org.opensearch.search.SearchService;

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

private final QueryShardContext context;
private final Map<String, Float> fieldsAndWeights;
Expand Down Expand Up @@ -867,6 +871,23 @@ public Query parse(String query) throws ParseException {
if (query.trim().isEmpty()) {
return Queries.newMatchNoDocsQuery("Matching no documents because no terms present");
}
if (query.length() > maxQueryStringLength) {
throw new ParseException(
"Query string length exceeds max allowed length "
+ maxQueryStringLength
+ " ("
+ SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey()
+ "); actual length: "
+ query.length()
);
}
return super.parse(query);
}

/**
* Sets the maximum allowed length for query strings. This should be only called from SearchService on settings updates.
*/
public static void setMaxQueryStringLength(int maxQueryStringLength) {
QueryStringQueryParser.maxQueryStringLength = maxQueryStringLength;
}
}
14 changes: 14 additions & 0 deletions server/src/main/java/org/opensearch/search/SearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
import org.opensearch.index.query.QueryRewriteContext;
import org.opensearch.index.query.QueryShardContext;
import org.opensearch.index.query.Rewriteable;
import org.opensearch.index.search.QueryStringQueryParser;
import org.opensearch.index.shard.IndexEventListener;
import org.opensearch.index.shard.IndexShard;
import org.opensearch.index.shard.SearchOperationListener;
Expand Down Expand Up @@ -369,6 +370,15 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
Setting.Property.Dynamic
);

public static final Setting<Integer> SEARCH_MAX_QUERY_STRING_LENGTH = Setting.intSetting(
"search.query.max_query_string_length",
32_000,
1,
Integer.MAX_VALUE,
Setting.Property.NodeScope,
Setting.Property.Dynamic
);

public static final Setting<Boolean> CLUSTER_ALLOW_DERIVED_FIELD_SETTING = Setting.boolSetting(
"search.derived_field.enabled",
true,
Expand Down Expand Up @@ -524,6 +534,10 @@ public SearchService(
IndexSearcher.setMaxClauseCount(INDICES_MAX_CLAUSE_COUNT_SETTING.get(settings));
clusterService.getClusterSettings().addSettingsUpdateConsumer(INDICES_MAX_CLAUSE_COUNT_SETTING, IndexSearcher::setMaxClauseCount);

QueryStringQueryParser.setMaxQueryStringLength(SEARCH_MAX_QUERY_STRING_LENGTH.get(settings));
clusterService.getClusterSettings()
.addSettingsUpdateConsumer(SEARCH_MAX_QUERY_STRING_LENGTH, QueryStringQueryParser::setMaxQueryStringLength);

allowDerivedField = CLUSTER_ALLOW_DERIVED_FIELD_SETTING.get(settings);
clusterService.getClusterSettings().addSettingsUpdateConsumer(CLUSTER_ALLOW_DERIVED_FIELD_SETTING, this::setAllowDerivedField);

Expand Down
Loading