Prevent synthetic field loaders accessing stored fields from using stale data (#112173) (#112257)

(cherry picked from commit 38adbb0)

# Conflicts:
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml

Author: lkts

docs/changelog/112173.yaml

@@ -0,0 +1,7 @@
+pr: 112173
+summary: Prevent synthetic field loaders accessing stored fields from using stale
+  data
+area: Mapping
+type: bug
+issues:
+ - 112156

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java

@@ -447,7 +447,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
                 "field [" + fullPath() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
             );
         }
-        return new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), leafName(), null) {
+        return new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), leafName()) {
             @Override
             protected void write(XContentBuilder b, Object value) throws IOException {
                 b.value((String) value);

plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java

@@ -584,7 +584,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
             );
         }
         if (fieldType.stored()) {
-            return new StringStoredFieldFieldLoader(fullPath(), leafName(), null) {
+            return new StringStoredFieldFieldLoader(fullPath(), leafName()) {
                 @Override
                 protected void write(XContentBuilder b, Object value) throws IOException {
                     b.value((String) value);

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml

@@ -1204,3 +1204,54 @@ nested object with stored array:
   - match: { hits.hits.1._source.nested_array_stored.0.b.1.c: 100 }
   - match: { hits.hits.1._source.nested_array_stored.1.b.0.c: 20 }
   - match: { hits.hits.1._source.nested_array_stored.1.b.1.c: 200 }
+
+---
+# 112156
+stored field under object with store_array_source:
+  - requires:
+      cluster_features: ["mapper.track_ignored_source"]
+      reason: requires tracking ignored source
+
+  - do:
+      indices.create:
+        index: test
+        body:
+          settings:
+            index:
+              sort.field: "name"
+              sort.order: "asc"
+          mappings:
+            _source:
+              mode: synthetic
+            properties:
+              name:
+                type: keyword
+              obj:
+                store_array_source: true
+                properties:
+                  foo:
+                    type: keyword
+                    store: true
+
+  - do:
+      bulk:
+        index: test
+        refresh: true
+        body:
+          - '{ "create": { } }'
+          - '{ "name": "B", "obj": null }'
+          - '{ "create": { } }'
+          - '{ "name": "A", "obj": [ { "foo": "hello_from_the_other_side" } ] }'
+
+  - match: { errors: false }
+
+  - do:
+      search:
+        index: test
+        sort: name
+
+  - match:  { hits.total.value: 2 }
+  - match: { hits.hits.0._source.name: A }
+  - match: { hits.hits.0._source.obj: [ { "foo": "hello_from_the_other_side" } ] }
+  - match: { hits.hits.1._source.name: B }
+  - match: { hits.hits.1._source.obj: null }

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java

@@ -859,7 +859,7 @@ protected String contentType() {
 
         @Override
         public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
-            return new StringStoredFieldFieldLoader(fullPath(), leafName(), null) {
+            return new StringStoredFieldFieldLoader(fullPath(), leafName()) {
                 @Override
                 protected void write(XContentBuilder b, Object value) throws IOException {
                     BytesRef ref = (BytesRef) value;

server/src/main/java/org/elasticsearch/index/mapper/CompositeSyntheticFieldLoader.java

@@ -15,6 +15,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
@@ -28,29 +29,44 @@
  * stored in a different field in case of ignore_malformed being enabled.
  */
 public class CompositeSyntheticFieldLoader implements SourceLoader.SyntheticFieldLoader {
-    private final String fieldName;
+    private final String leafFieldName;
     private final String fullFieldName;
-    private final SyntheticFieldLoaderLayer[] parts;
-    private boolean hasValue;
+    private final Collection<Layer> parts;
+    private boolean storedFieldLoadersHaveValues;
+    private boolean docValuesLoadersHaveValues;
 
-    public CompositeSyntheticFieldLoader(String fieldName, String fullFieldName, SyntheticFieldLoaderLayer... parts) {
-        this.fieldName = fieldName;
+    public CompositeSyntheticFieldLoader(String leafFieldName, String fullFieldName, Layer... parts) {
+        this(leafFieldName, fullFieldName, Arrays.asList(parts));
+    }
+
+    public CompositeSyntheticFieldLoader(String leafFieldName, String fullFieldName, Collection<Layer> parts) {
+        this.leafFieldName = leafFieldName;
         this.fullFieldName = fullFieldName;
         this.parts = parts;
-        this.hasValue = false;
+        this.storedFieldLoadersHaveValues = false;
+        this.docValuesLoadersHaveValues = false;
     }
 
     @Override
     public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
-        return Arrays.stream(parts).flatMap(SyntheticFieldLoaderLayer::storedFieldLoaders).map(e -> Map.entry(e.getKey(), values -> {
-            hasValue = true;
-            e.getValue().load(values);
+        return parts.stream().flatMap(Layer::storedFieldLoaders).map(e -> Map.entry(e.getKey(), new StoredFieldLoader() {
+            @Override
+            public void advanceToDoc(int docId) {
+                storedFieldLoadersHaveValues = false;
+                e.getValue().advanceToDoc(docId);
+            }
+
+            @Override
+            public void load(List<Object> newValues) {
+                storedFieldLoadersHaveValues = true;
+                e.getValue().load(newValues);
+            }
         }));
     }
 
     @Override
     public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
-        var loaders = new ArrayList<DocValuesLoader>(parts.length);
+        var loaders = new ArrayList<DocValuesLoader>(parts.size());
         for (var part : parts) {
             var partLoader = part.docValuesLoader(leafReader, docIdsInLeaf);
             if (partLoader != null) {
@@ -63,38 +79,40 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
         }
 
         return docId -> {
+            this.docValuesLoadersHaveValues = false;
+
             boolean hasDocs = false;
             for (var loader : loaders) {
                 hasDocs |= loader.advanceToDoc(docId);
             }
 
-            this.hasValue |= hasDocs;
+            this.docValuesLoadersHaveValues = hasDocs;
             return hasDocs;
         };
     }
 
     @Override
     public boolean hasValue() {
-        return hasValue;
+        return storedFieldLoadersHaveValues || docValuesLoadersHaveValues;
     }
 
     @Override
     public void write(XContentBuilder b) throws IOException {
-        var totalCount = Arrays.stream(parts).mapToLong(SyntheticFieldLoaderLayer::valueCount).sum();
+        var totalCount = parts.stream().mapToLong(Layer::valueCount).sum();
 
         if (totalCount == 0) {
             return;
         }
 
         if (totalCount == 1) {
-            b.field(fieldName);
+            b.field(leafFieldName);
             for (var part : parts) {
                 part.write(b);
             }
             return;
         }
 
-        b.startArray(fieldName);
+        b.startArray(leafFieldName);
         for (var part : parts) {
             part.write(b);
         }
@@ -113,9 +131,9 @@ public String fieldName() {
      * Note that the contract of {@link SourceLoader.SyntheticFieldLoader#write(XContentBuilder)}
      * is slightly different here since it only needs to write field values without encompassing object or array.
      */
-    public interface SyntheticFieldLoaderLayer extends SourceLoader.SyntheticFieldLoader {
+    public interface Layer extends SourceLoader.SyntheticFieldLoader {
         /**
-         * Number of values that this loader will write.
+         * Number of values that this loader will write for a given document.
          * @return
          */
         long valueCount();
@@ -125,12 +143,30 @@ public interface SyntheticFieldLoaderLayer extends SourceLoader.SyntheticFieldLo
      * Layer that loads malformed values stored in a dedicated field with a conventional name.
      * @see IgnoreMalformedStoredValues
      */
-    public static class MalformedValuesLayer implements SyntheticFieldLoaderLayer {
+    public static class MalformedValuesLayer extends StoredFieldLayer {
+        public MalformedValuesLayer(String fieldName) {
+            super(IgnoreMalformedStoredValues.name(fieldName));
+        }
+
+        @Override
+        protected void writeValue(Object value, XContentBuilder b) throws IOException {
+            if (value instanceof BytesRef r) {
+                XContentDataHelper.decodeAndWrite(b, r);
+            } else {
+                b.value(value);
+            }
+        }
+    }
+
+    /**
+     * Layer that loads field values from a provided stored field.
+     */
+    public abstract static class StoredFieldLayer implements Layer {
         private final String fieldName;
         private List<Object> values;
 
-        public MalformedValuesLayer(String fieldName) {
-            this.fieldName = IgnoreMalformedStoredValues.name(fieldName);
+        public StoredFieldLayer(String fieldName) {
+            this.fieldName = fieldName;
             this.values = emptyList();
         }
 
@@ -141,7 +177,17 @@ public long valueCount() {
 
         @Override
         public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
-            return Stream.of(Map.entry(fieldName, values -> this.values = values));
+            return Stream.of(Map.entry(fieldName, new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
+                @Override
+                public void advanceToDoc(int docId) {
+                    values = emptyList();
+                }
+
+                @Override
+                public void load(List<Object> newValues) {
+                    values = newValues;
+                }
+            }));
         }
 
         @Override
@@ -157,15 +203,15 @@ public boolean hasValue() {
         @Override
         public void write(XContentBuilder b) throws IOException {
             for (Object v : values) {
-                if (v instanceof BytesRef r) {
-                    XContentDataHelper.decodeAndWrite(b, r);
-                } else {
-                    b.value(v);
-                }
+                writeValue(v, b);
             }
-            values = emptyList();
         }
 
+        /**
+         * Write a value read from stored field using appropriate format.
+         */
+        protected abstract void writeValue(Object value, XContentBuilder b) throws IOException;
+
         @Override
         public String fieldName() {
             return fieldName;

server/src/main/java/org/elasticsearch/index/mapper/IgnoreMalformedStoredValues.java

@@ -107,7 +107,17 @@ private static class Stored extends IgnoreMalformedStoredValues {
 
         @Override
         public Stream<Map.Entry<String, SourceLoader.SyntheticFieldLoader.StoredFieldLoader>> storedFieldLoaders() {
-            return Stream.of(Map.entry(name(fieldName), values -> this.values = values));
+            return Stream.of(Map.entry(name(fieldName), new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
+                @Override
+                public void advanceToDoc(int docId) {
+                    values = emptyList();
+                }
+
+                @Override
+                public void load(List<Object> newValues) {
+                    values = newValues;
+                }
+            }));
         }
 
         @Override

server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java

@@ -47,6 +47,7 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.time.ZoneId;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -625,7 +626,9 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
                 "field [" + fullPath() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
             );
         }
-        return new SortedSetDocValuesSyntheticFieldLoader(fullPath(), leafName(), null, ignoreMalformed) {
+
+        var layers = new ArrayList<CompositeSyntheticFieldLoader.Layer>();
+        layers.add(new SortedSetDocValuesSyntheticFieldLoaderLayer(fullPath()) {
             @Override
             protected BytesRef convert(BytesRef value) {
                 byte[] bytes = Arrays.copyOfRange(value.bytes, value.offset, value.offset + value.length);
@@ -637,6 +640,12 @@ protected BytesRef preserve(BytesRef value) {
                 // No need to copy because convert has made a deep copy
                 return value;
             }
-        };
+        });
+
+        if (ignoreMalformed) {
+            layers.add(new CompositeSyntheticFieldLoader.MalformedValuesLayer(fullPath()));
+        }
+
+        return new CompositeSyntheticFieldLoader(leafName(), fullPath(), layers);
     }
 }