Browse Source

Fix DLS using runtime fields and synthetic source (#112341)

Somewhat of a tortured test but applying the same fix from #112260
to synthetic source which was running into the same bug as
a stored field source.
Armin Braun 1 year ago
parent
commit
cefe358b41

+ 5 - 0
docs/changelog/112341.yaml

@@ -0,0 +1,5 @@
+pr: 112341
+summary: Fix DLS using runtime fields and synthetic source
+area: Authorization
+type: bug
+issues: []

+ 10 - 26
server/src/main/java/org/elasticsearch/search/lookup/SyntheticSourceProvider.java

@@ -8,13 +8,14 @@
 
 package org.elasticsearch.search.lookup;
 
-import org.apache.lucene.index.IndexReaderContext;
 import org.apache.lucene.index.LeafReaderContext;
+import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
 import org.elasticsearch.index.fieldvisitor.LeafStoredFieldLoader;
 import org.elasticsearch.index.fieldvisitor.StoredFieldLoader;
 import org.elasticsearch.index.mapper.SourceLoader;
 
 import java.io.IOException;
+import java.util.Map;
 
 // NB This is written under the assumption that individual segments are accessed by a single
 // thread, even if separate segments may be searched concurrently.  If we ever implement
@@ -22,7 +23,7 @@ import java.io.IOException;
 class SyntheticSourceProvider implements SourceProvider {
 
     private final SourceLoader sourceLoader;
-    private volatile SyntheticSourceLeafLoader[] leafLoaders;
+    private final Map<Object, SyntheticSourceLeafLoader> leaves = ConcurrentCollections.newConcurrentMap();
 
     SyntheticSourceProvider(SourceLoader sourceLoader) {
         this.sourceLoader = sourceLoader;
@@ -30,31 +31,14 @@ class SyntheticSourceProvider implements SourceProvider {
 
     @Override
     public Source getSource(LeafReaderContext ctx, int doc) throws IOException {
-        maybeInit(ctx);
-        if (leafLoaders[ctx.ord] == null) {
-            // individual segments are currently only accessed on one thread so there's no need
-            // for locking here.
-            leafLoaders[ctx.ord] = new SyntheticSourceLeafLoader(ctx);
+        final Object id = ctx.id();
+        var provider = leaves.get(id);
+        if (provider == null) {
+            provider = new SyntheticSourceLeafLoader(ctx);
+            var existing = leaves.put(id, provider);
+            assert existing == null : "unexpected source provider [" + existing + "]";
         }
-        return leafLoaders[ctx.ord].getSource(doc);
-    }
-
-    private void maybeInit(LeafReaderContext ctx) {
-        if (leafLoaders == null) {
-            synchronized (this) {
-                if (leafLoaders == null) {
-                    leafLoaders = new SyntheticSourceLeafLoader[findParentContext(ctx).leaves().size()];
-                }
-            }
-        }
-    }
-
-    private IndexReaderContext findParentContext(LeafReaderContext ctx) {
-        if (ctx.parent != null) {
-            return ctx.parent;
-        }
-        assert ctx.isTopLevel;
-        return ctx;
+        return provider.getSource(doc);
     }
 
     private class SyntheticSourceLeafLoader {

+ 37 - 1
x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityRandomTests.java

@@ -144,6 +144,43 @@ public class DocumentLevelSecurityRandomTests extends SecurityIntegTestCase {
                         .endObject()
                 )
         );
+        doTestWithRuntimeFieldsInTestIndex();
+    }
+
+    public void testWithRuntimeFieldsAndSyntheticSource() throws Exception {
+        assertAcked(
+            indicesAdmin().prepareCreate("test")
+                .setMapping(
+                    XContentFactory.jsonBuilder()
+                        .startObject()
+                        .startObject("_source")
+                        .field("mode", "synthetic")
+                        .endObject()
+                        .startObject("runtime")
+                        .startObject("field1")
+                        .field("type", "keyword")
+                        .endObject()
+                        .startObject("field2")
+                        .field("type", "keyword")
+                        .endObject()
+                        .endObject()
+                        .startObject("properties")
+                        .startObject("field1")
+                        .field("type", "text")
+                        .field("store", true)
+                        .endObject()
+                        .startObject("field2")
+                        .field("type", "text")
+                        .field("store", true)
+                        .endObject()
+                        .endObject()
+                        .endObject()
+                )
+        );
+        doTestWithRuntimeFieldsInTestIndex();
+    }
+
+    private void doTestWithRuntimeFieldsInTestIndex() {
         List<IndexRequestBuilder> requests = new ArrayList<>(47);
         for (int i = 1; i <= 42; i++) {
             requests.add(prepareIndex("test").setSource("field1", "value1", "field2", "foo" + i));
@@ -158,5 +195,4 @@ public class DocumentLevelSecurityRandomTests extends SecurityIntegTestCase {
             42L
         );
     }
-
 }