Skip to content
Open
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 @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
## [Unreleased 3.x]
### Added
- Use Lucene `pack` method for `half_float` and `usigned_long` when using `ApproximatePointRangeQuery`.
- New cluster setting search.query.max_query_string_length_monitor_only ([#19539](https://github.com/opensearch-project/OpenSearch/pull/19539))
- Add a mapper for context aware segments grouping criteria ([#19233](https://github.com/opensearch-project/OpenSearch/pull/19233))

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
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.search.SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY;
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 @@ -796,6 +797,45 @@ public void testMaxQueryStringLength() throws Exception {
}
}

public void testMaxQueryStringLengthMonitorMode() 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))
);
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.getKey(), true))
);

// query run with the default value of search.query.max_query_string_length which is true
SearchResponse response = client().prepareSearch("test").setQuery(queryStringQuery("foo OR foo OR foo OR foo")).get();

assertNoFailures(response);
} finally {
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH.getKey()))
);
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().putNull(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.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 @@ -579,6 +579,7 @@ public void apply(Settings value, Settings current, Settings previous) {
SearchService.AGGREGATION_REWRITE_FILTER_SEGMENT_THRESHOLD,
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH,
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY,
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 @@ -32,6 +32,8 @@

package org.opensearch.index.search;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
Expand Down Expand Up @@ -97,9 +99,11 @@
* @opensearch.internal
*/
public class QueryStringQueryParser extends XQueryParser {
private static final Logger logger = LogManager.getLogger(QueryStringQueryParser.class);
private static final String EXISTS_FIELD = "_exists_";
@SuppressWarnings("NonFinalStaticField")
private static int maxQueryStringLength = SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.get(Settings.EMPTY);
private static boolean maxQueryStringLengthMonitorMode = SearchService.SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.get(Settings.EMPTY);

private final QueryShardContext context;
private final Map<String, Float> fieldsAndWeights;
Expand Down Expand Up @@ -872,14 +876,24 @@ public Query parse(String query) throws ParseException {
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()
);
if (maxQueryStringLengthMonitorMode) {
// Log a warning and continue
logger.warn(
"Query string length exceeds max allowed length {} ({}); actual length: {}",
maxQueryStringLength,
SearchService.SEARCH_MAX_QUERY_STRING_LENGTH.getKey(),
query.length()
);
} else {
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);
}
Expand All @@ -890,4 +904,12 @@ public Query parse(String query) throws ParseException {
public static void setMaxQueryStringLength(int maxQueryStringLength) {
QueryStringQueryParser.maxQueryStringLength = maxQueryStringLength;
}

/**
* Sets whether the max query string length should be enforced in or not
* @param monitorMode if true, the max query string length will not be enforced
*/
public static void setMaxQueryStringLengthMonitorMode(boolean monitorMode) {
QueryStringQueryParser.maxQueryStringLengthMonitorMode = monitorMode;
}
}
13 changes: 13 additions & 0 deletions server/src/main/java/org/opensearch/search/SearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,13 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
Setting.Property.Dynamic
);

public static final Setting<Boolean> SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY = Setting.boolSetting(
"search.query.max_query_string_length_monitor_only",
false,
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 @@ -537,6 +544,12 @@ public SearchService(
QueryStringQueryParser.setMaxQueryStringLength(SEARCH_MAX_QUERY_STRING_LENGTH.get(settings));
clusterService.getClusterSettings()
.addSettingsUpdateConsumer(SEARCH_MAX_QUERY_STRING_LENGTH, QueryStringQueryParser::setMaxQueryStringLength);
QueryStringQueryParser.setMaxQueryStringLengthMonitorMode(SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY.get(settings));
clusterService.getClusterSettings()
.addSettingsUpdateConsumer(
SEARCH_MAX_QUERY_STRING_LENGTH_MONITOR_ONLY,
QueryStringQueryParser::setMaxQueryStringLengthMonitorMode
);

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