Skip to content

Use dynamic max clause count for field expansion limit - #22573

Open
igoralex1992 wants to merge 2 commits into
opensearch-project:mainfrom
igoralex1992:main
Open

Use dynamic max clause count for field expansion limit#22573
igoralex1992 wants to merge 2 commits into
opensearch-project:mainfrom
igoralex1992:main

Conversation

@igoralex1992

Copy link
Copy Markdown

Description

This change makes QueryParserHelper use the current value of
indices.query.bool.max_clause_count when validating field expansion.

Previously, checkForTooManyFields() retrieved the limit from
context.getIndexSettings().getSettings(). However,
indices.query.bool.max_clause_count is a cluster setting and is not
part of the index settings, so this is not the correct source for the
current value.

To address this, SearchService now caches the current value of the
cluster setting and updates it through the existing settings update
consumer. QueryParserHelper retrieves the limit from SearchService
instead of reading it from IndexSettings.

Changes

  • Added a cached fieldExpansionLimit to SearchService.
  • Updated the cached value whenever
    indices.query.bool.max_clause_count changes.
  • Initialized QueryParserHelper with a SearchService instance.
  • Updated field expansion validation to use the cached limit.

Motivation

This ensures that field expansion validation always uses the current
value of indices.query.bool.max_clause_count, including after dynamic
cluster setting updates.

Related Issues

N/A

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 0b0de65.

Hard block: Issues at Medium severity or above will block this PR from merging.

PathLineSeverityDescription
server/src/main/java/org/opensearch/index/search/QueryParserHelper.java55mediumPublic static setter `setSearchService()` introduces a global mutable singleton that any code in the JVM can call before `SearchService` constructor runs. The null-guard only blocks re-assignment, so whichever caller wins the race owns the injected instance. Any code path that can execute before `SearchService` initializes (e.g., a plugin, test harness, or early bootstrap component) could supply a malicious implementation that returns an arbitrarily large `fieldExpansionLimit`, bypassing field-expansion protection.
server/src/main/java/org/opensearch/index/search/QueryParserHelper.java193medium`checkForTooManyFields` now dereferences `searchService` without a null check. If this method is invoked before `SearchService` registers itself (e.g., during index recovery, plugin initialization, or unit testing), a NullPointerException is thrown. This is a reliable crash point that could be triggered by any caller able to initiate a multi-field query before full startup, enabling a targeted availability-disruption.
server/src/main/java/org/opensearch/search/SearchService.java591low`IndexSearcher.setMaxClauseCount` is initialized from the `settings` constructor parameter, while `this.fieldExpansionLimit` is initialized from `clusterService.getSettings()`. These are distinct objects and may carry different effective values at startup, causing `IndexSearcher`'s hard cap and `QueryParserHelper`'s soft field-expansion limit to be inconsistent until the first settings-update event fires.
server/src/main/java/org/opensearch/search/SearchService.java613lowCalling `QueryParserHelper.setSearchService(this)` mid-constructor creates a circular bidirectional dependency (`SearchService` → `QueryParserHelper` → `SearchService`). While `fieldExpansionLimit` is assigned before this call, any future modification that moves field accesses around in `SearchService`'s constructor could expose a partially-constructed object to external callers via the static reference, a classic unsafe-publication pattern.

The table above displays the top 10 most important findings.

Total: 4 | Critical: 0 | High: 0 | Medium: 2 | Low: 2


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@igoralex1992

Copy link
Copy Markdown
Author

Hi!

I'm not the most experienced contributor, but it looks like the current implementation doesn't actually use indices.query.bool.max_clause_count during field expansion validation.

Based on the code, this setting is only reflected in the error message, for example:

"reason": "failed to create query: field expansion matches too many fields, limit: 1024, got: 1334"

Please take a look at this PR. With this change, updates to indices.query.bool.max_clause_count are applied during validation, allowing the field expansion limit to follow the current cluster setting and enabling more complex queries when the limit is increased.

Manual verification:

Before the fix:

Cluster setting updated:
indices.query.bool.max_clause_count: 1024 -> 2048

FIELD EXPANSION DEBUG -> indexLimit: 1024, scopedLimit: 1024

After the fix:

Cluster setting updated:
indices.query.bool.max_clause_count: 1024 -> 2048

MY FIX WORKS -> indexLimit: 1024, scopedLimit: 2048

This confirms that QueryParserHelper now uses the updated cluster setting instead of the stale value from IndexSettings.

[2026-04-30T13:26:38,067][INFO ][o.o.p.PluginsService     ] [test-os] PluginService:onIndexModule index:[test-fields/RXy7Pbk-Q-6ofsUDSLFbyQ]
[2026-04-30T13:26:38,093][INFO ][o.o.c.m.MetadataMappingService] [test-os] [test-fields/RXy7Pbk-Q-6ofsUDSLFbyQ] create_mapping
[2026-04-30T13:27:07,922][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:08,001][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:08,032][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:08,040][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:49,465][INFO ][o.o.c.s.ClusterSettings  ] [test-os] updating [indices.query.bool.max_clause_count] from [1024] to [2048]
[2026-04-30T13:27:49,500][WARN ][o.o.c.r.a.AllocationService] [test-os] Falling back to single shard assignment since batch mode disable or multiple custom allocators set
[2026-04-30T13:27:51,232][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:51,239][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T13:27:51,249][WARN ][o.o.i.s.QueryParserHelper] [test-os] FIELD EXPANSION DEBUG -> fields: 18, indexLimit: 1024, scopedLimit: 1024


[2026-04-30T15:40:41,822][WARN ][o.o.c.r.a.AllocationService] [test-os] Falling back to single shard assignment since batch mode disable or multiple custom allocators set
[2026-04-30T15:40:46,360][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T15:40:46,381][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T15:40:46,385][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T15:40:46,387][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 1024
[2026-04-30T15:40:56,362][INFO ][o.o.c.s.ClusterSettings  ] [test-os] updating [indices.query.bool.max_clause_count] from [1024] to [2048]
[2026-04-30T15:40:56,373][WARN ][o.o.c.r.a.AllocationService] [test-os] Falling back to single shard assignment since batch mode disable or multiple custom allocators set
[2026-04-30T15:40:59,797][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 2048
[2026-04-30T15:40:59,798][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 2048
[2026-04-30T15:40:59,798][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 2048
[2026-04-30T15:40:59,800][WARN ][o.o.i.s.QueryParserHelper] [test-os] MY FIX WORKS -> fields: 0, indexLimit: 1024, scopedLimit: 2048

@igoralex1992

Copy link
Copy Markdown
Author

I forgot one more file, added a commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant