From 329d05f61e327160faaf30e9313c25db4ab967fe Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 27 Jun 2019 16:56:15 +0200 Subject: [PATCH] Fix UOE on search requests that match a sparse role query (#43668) Search requests executed through the SecurityIndexSearcherWrapper throw an UnsupportedOperationException if they match a sparse role query. When low level cancellation is activated (which is the default since #42857), the context index searcher creates a weight that doesn't handle #scorer. This change fixes this bug and adds a test to ensure that we check this case. --- .../search/internal/ContextIndexSearcher.java | 7 ++++--- .../SecurityIndexSearcherWrapperUnitTests.java | 8 +++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java index 7c56796f3d24d..49c310ba706b6 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java @@ -152,13 +152,14 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio } @Override - public Scorer scorer(LeafReaderContext context) throws IOException { + public boolean isCacheable(LeafReaderContext ctx) { throw new UnsupportedOperationException(); } @Override - public boolean isCacheable(LeafReaderContext ctx) { - throw new UnsupportedOperationException(); + public Scorer scorer(LeafReaderContext context) throws IOException { + // in case the wrapped searcher (in) uses the scorer directly + return weight.scorer(context); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java index b9eb0241d9a3e..3da3949bad967 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.cache.bitset.BitsetFilterCache; +import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.mapper.FieldNamesFieldMapper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.SeqNoFieldMapper; @@ -52,6 +53,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.aggregations.LeafBucketCollector; +import org.elasticsearch.search.internal.ContextIndexSearcher; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.xpack.core.security.authz.accesscontrol.DocumentSubsetReader.DocumentSubsetDirectoryReader; @@ -537,7 +539,11 @@ public void onRemoval(ShardId shardId, Accountable accountable) { } DocumentSubsetDirectoryReader filteredReader = DocumentSubsetReader.wrap(reader, cache, roleQuery); - IndexSearcher searcher = new SecurityIndexSearcherWrapper.IndexSearcherWrapper(filteredReader); + IndexSearcher wrapSearcher = new SecurityIndexSearcherWrapper.IndexSearcherWrapper(filteredReader); + Engine.Searcher engineSearcher = new Engine.Searcher("test", wrapSearcher, () -> {}); + ContextIndexSearcher searcher = new ContextIndexSearcher(engineSearcher, + wrapSearcher.getQueryCache(), wrapSearcher.getQueryCachingPolicy()); + searcher.setCheckCancelled(() -> {}); // Searching a non-existing term will trigger a null scorer assertEquals(0, searcher.count(new TermQuery(new Term("non_existing_field", "non_existing_value"))));