Synthetic source error on script loads (#88334)

* Source Lookup refactor with error on script synthetic source load Refactors SourceLookup into a static source lookup used for most cases where we access the source again after the fetch phase, and a re-loading lookup used by scripts. The re-loading lookup now also fails with an error when we are using synthetic source preventing silent failures or non-sensical behavior from scripts.
elastic · Aug 26, 2022 · db5ddb3 · db5ddb3
1 parent 6f5fa14
commit db5ddb3
Show file tree

Hide file tree

Showing 63 changed files with 520 additions and 220 deletions.
diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
@@ -40,6 +40,7 @@
 import org.elasticsearch.script.ScoreScript;
 import org.elasticsearch.script.ScriptModule;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -88,7 +89,8 @@ public class ScriptScoreBenchmark {
     private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();
     private final SearchLookup lookup = new SearchLookup(
         fieldTypes::get,
-        (mft, lookup, fdo) -> mft.fielddataBuilder(FieldDataContext.noRuntimeFields("benchmark")).build(fieldDataCache, breakerService)
+        (mft, lookup, fdo) -> mft.fielddataBuilder(FieldDataContext.noRuntimeFields("benchmark")).build(fieldDataCache, breakerService),
+        new SourceLookup.ReaderSourceProvider()
     );
 
     @Param({ "expression", "metal", "painless_cast", "painless_def" })

diff --git a/...ain/java/org/elasticsearch/benchmark/search/fetch/subphase/FetchSourcePhaseBenchmark.java b/...ain/java/org/elasticsearch/benchmark/search/fetch/subphase/FetchSourcePhaseBenchmark.java
@@ -79,8 +79,7 @@ private BytesReference buildBigExample(String extraText) throws IOException {
 
     @Benchmark
     public BytesReference filterObjects() throws IOException {
-        SourceLookup lookup = new SourceLookup();
-        lookup.setSource(sourceBytes);
+        SourceLookup lookup = new SourceLookup(new SourceLookup.BytesSourceProvider(sourceBytes));
         Object value = lookup.filter(fetchContext);
         return FetchSourcePhase.objectToBytes(value, XContentType.JSON, Math.min(1024, lookup.internalSourceRef().length()));
     }

diff --git a/docs/changelog/88334.yaml b/docs/changelog/88334.yaml
@@ -0,0 +1,5 @@
+pr: 88334
+summary: Synthetic source error on script loads
+area: Geo
+type: bug
+issues: []
diff --git a/...ression/src/test/java/org/elasticsearch/script/expression/ExpressionFieldScriptTests.java b/...ression/src/test/java/org/elasticsearch/script/expression/ExpressionFieldScriptTests.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.script.FieldScript;
 import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -49,7 +50,11 @@ public void setUp() throws Exception {
         when(fieldData.load(any())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(field -> field.equals("field") ? fieldType : null, (ignored, _lookup, fdt) -> fieldData);
+        lookup = new SearchLookup(
+            field -> field.equals("field") ? fieldType : null,
+            (ignored, _lookup, fdt) -> fieldData,
+            new SourceLookup.ReaderSourceProvider()
+        );
     }
 
     private FieldScript.LeafFactory compile(String expression) {

diff --git a/...on/src/test/java/org/elasticsearch/script/expression/ExpressionNumberSortScriptTests.java b/...on/src/test/java/org/elasticsearch/script/expression/ExpressionNumberSortScriptTests.java
@@ -17,6 +17,7 @@
 import org.elasticsearch.script.NumberSortScript;
 import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -50,7 +51,11 @@ public void setUp() throws Exception {
         when(fieldData.load(any())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(field -> field.equals("field") ? fieldType : null, (ignored, _lookup, fdt) -> fieldData);
+        lookup = new SearchLookup(
+            field -> field.equals("field") ? fieldType : null,
+            (ignored, _lookup, fdt) -> fieldData,
+            new SourceLookup.ReaderSourceProvider()
+        );
     }
 
     private NumberSortScript.LeafFactory compile(String expression) {

diff --git a/...ssion/src/test/java/org/elasticsearch/script/expression/ExpressionTermsSetQueryTests.java b/...ssion/src/test/java/org/elasticsearch/script/expression/ExpressionTermsSetQueryTests.java
@@ -16,6 +16,7 @@
 import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.script.TermsSetQueryScript;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -49,7 +50,11 @@ public void setUp() throws Exception {
         when(fieldData.load(any())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(field -> field.equals("field") ? fieldType : null, (ignored, _lookup, fdt) -> fieldData);
+        lookup = new SearchLookup(
+            field -> field.equals("field") ? fieldType : null,
+            (ignored, _lookup, fdt) -> fieldData,
+            new SourceLookup.ReaderSourceProvider()
+        );
     }
 
     private TermsSetQueryScript.LeafFactory compile(String expression) {

diff --git a/...rcolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java b/...rcolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java
@@ -22,6 +22,7 @@
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightPhase;
 import org.elasticsearch.search.fetch.subphase.highlight.Highlighter;
 import org.elasticsearch.search.fetch.subphase.highlight.SearchHighlightContext;
+import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -86,7 +87,7 @@ public void process(HitContext hit) throws IOException {
                                 percolatorLeafReaderContext,
                                 slot
                             );
-                            subContext.sourceLookup().setSource(document);
+                            subContext.sourceLookup().setSourceProvider(new SourceLookup.BytesSourceProvider(document));
                             // force source because MemoryIndex does not store fields
                             SearchHighlightContext highlight = new SearchHighlightContext(fetchContext.highlight().fields(), true);
                             FetchSubPhaseProcessor processor = highlightPhase.getProcessor(fetchContext, highlight, query);

diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java
@@ -29,6 +29,7 @@
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.lookup.FieldLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
@@ -142,7 +143,7 @@ static Object fieldsScript(Map<String, Object> vars, String fieldName) {
 
         static Object sourceScript(Map<String, Object> vars, String path) {
             @SuppressWarnings("unchecked")
-            Map<String, Object> source = (Map<String, Object>) vars.get("_source");
+            Map<String, Object> source = ((SourceLookup) vars.get("_source")).source();
             return XContentMapValues.extractValue(path, source);
         }
 

diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/stats/SearchStatsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/stats/SearchStatsIT.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.test.ESIntegTestCase;
 
 import java.util.Collection;
@@ -61,7 +62,7 @@ public static class CustomScriptPlugin extends MockScriptPlugin {
         @Override
         protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
             return Collections.singletonMap("_source.field", vars -> {
-                Map<?, ?> src = (Map) vars.get("_source");
+                Map<?, ?> src = ((SourceLookup) vars.get("_source")).source();
                 return src.get("field");
             });
         }

diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
@@ -332,8 +332,7 @@ public static GetResult extractGetResult(
 
         BytesReference sourceFilteredAsBytes = sourceAsBytes;
         if (request.fetchSource().includes().length > 0 || request.fetchSource().excludes().length > 0) {
-            SourceLookup sourceLookup = new SourceLookup();
-            sourceLookup.setSource(source);
+            SourceLookup sourceLookup = new SourceLookup(new SourceLookup.MapSourceProvider(source));
             Object value = sourceLookup.filter(request.fetchSource());
             try {
                 final int initialCapacity = sourceAsBytes != null ? Math.min(1024, sourceAsBytes.length()) : 1024;

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -24,6 +24,7 @@
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentParserConfiguration;
@@ -144,7 +145,8 @@ private static void executeIndexTimeScripts(DocumentParserContext context) {
             context.mappingLookup().indexTimeLookup()::get,
             (ft, lookup, fto) -> ft.fielddataBuilder(
                 new FieldDataContext(context.indexSettings().getIndex().getName(), lookup, context.mappingLookup()::sourcePaths, fto)
-            ).build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService())
+            ).build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()),
+            new SourceLookup.ReaderSourceProvider()
         );
         // field scripts can be called both by the loop at the end of this method and via
         // the document reader, so to ensure that we don't run them multiple times we

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
+import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -470,4 +471,18 @@ public void validateDoesNotShadow(String name) {
             throw new MapperParsingException("Field [" + name + "] attempted to shadow a time_series_metric");
         }
     }
+
+    /**
+     * Returns a SourceProvider describing how to read the source. If using synthetic source, returns the null source provider
+     * expecting the source to either be provided later by a fetch phase or not be accessed at all (as in scripts).
+     * @return
+     */
+    public SourceLookup.SourceProvider getSourceProvider() {
+        SourceFieldMapper sourceMapper = (SourceFieldMapper) getMapper("_source");
+        if (sourceMapper == null || sourceMapper.isSynthetic() == false) {
+            return new SourceLookup.ReaderSourceProvider();
+        } else {
+            return new SourceLookup.NullSourceProvider();
+        }
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NestedValueFetcher.java b/server/src/main/java/org/elasticsearch/index/mapper/NestedValueFetcher.java
@@ -50,8 +50,7 @@ public List<Object> fetchValues(SourceLookup lookup, List<Object> includedValues
         for (Object entry : nestedValues) {
             // add this one entry only to the stub and use this as source lookup
             stub.put(nestedFieldName, entry);
-            SourceLookup nestedSourceLookup = new SourceLookup();
-            nestedSourceLookup.setSource(filteredSource);
+            SourceLookup nestedSourceLookup = new SourceLookup(new SourceLookup.MapSourceProvider(filteredSource));
 
             Map<String, DocumentField> fetchResult = nestedFieldFetcher.fetch(nestedSourceLookup);
 

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java
@@ -83,7 +83,7 @@ public Set<String> requiredStoredFields() {
     };
 
     /**
-     * Load {@code _source} from doc vales.
+     * Load {@code _source} from doc values.
      */
     class Synthetic implements SourceLoader {
         private final SyntheticFieldLoader loader;

diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java
@@ -59,6 +59,7 @@
 import org.elasticsearch.search.NestedDocuments;
 import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.transport.RemoteClusterAware;
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentParserConfiguration;
@@ -481,12 +482,16 @@ public boolean containsBrokenAnalysis(String field) {
      */
     public SearchLookup lookup() {
         if (this.lookup == null) {
+            SourceLookup.SourceProvider sourceProvider = mappingLookup == null
+                ? new SourceLookup.ReaderSourceProvider()
+                : mappingLookup.getSourceProvider();
             this.lookup = new SearchLookup(
                 this::getFieldType,
                 (fieldType, searchLookup, fielddataOperation) -> indexFieldDataLookup.apply(
                     fieldType,
                     new FieldDataContext(fullyQualifiedIndex.getName(), searchLookup, this::sourcePath, fielddataOperation)
-                )
+                ),
+                sourceProvider
             );
         }
         return this.lookup;

diff --git a/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java b/server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java
@@ -272,8 +272,7 @@ private static Fields generateTermVectors(
         }
         if (source != null) {
             MappingLookup mappingLookup = indexShard.mapperService().mappingLookup();
-            SourceLookup sourceLookup = new SourceLookup();
-            sourceLookup.setSource(source);
+            SourceLookup sourceLookup = new SourceLookup(new SourceLookup.MapSourceProvider(source));
             for (String field : fields) {
                 if (values.containsKey(field) == false) {
                     SourceValueFetcher valueFetcher = SourceValueFetcher.toString(mappingLookup.sourcePaths(field));

diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
@@ -383,11 +383,12 @@ private static HitContext prepareNonNestedHitContext(
             if (source != null) {
                 // Store the loaded source on the hit context so that fetch subphases can access it.
                 // Also make it available to scripts by storing it on the shared SearchLookup instance.
-                hitContext.sourceLookup().setSource(source);
+                SourceLookup.BytesSourceProvider sourceBytes = new SourceLookup.BytesSourceProvider(source);
+                hitContext.sourceLookup().setSourceProvider(sourceBytes);
 
                 SourceLookup scriptSourceLookup = context.getSearchExecutionContext().lookup().source();
                 scriptSourceLookup.setSegmentAndDocument(subReaderContext, subDocId);
-                scriptSourceLookup.setSource(source);
+                scriptSourceLookup.setSourceProvider(sourceBytes);
             }
             return hitContext;
         }
@@ -486,8 +487,7 @@ private static HitContext prepareNestedHitContext(
                 }
             }
 
-            hitContext.sourceLookup().setSource(nestedSourceAsMap);
-            hitContext.sourceLookup().setSourceContentType(rootSourceContentType);
+            hitContext.sourceLookup().setSourceProvider(new SourceLookup.MapSourceProvider(nestedSourceAsMap, rootSourceContentType));
         }
         return hitContext;
     }

diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java
@@ -31,7 +31,7 @@ public HitContext(SearchHit hit, LeafReaderContext context, int docId) {
             this.hit = hit;
             this.readerContext = context;
             this.docId = docId;
-            this.sourceLookup = new SourceLookup();
+            this.sourceLookup = new SourceLookup(new SourceLookup.ReaderSourceProvider());
             sourceLookup.setSegmentAndDocument(context, docId);
         }
 

diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java
@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 /**
@@ -172,15 +173,20 @@ public Map<String, DocumentField> fetch(SourceLookup sourceLookup) throws IOExce
                 documentFields.put(field, docField);
             }
         }
-        collectUnmapped(documentFields, sourceLookup, "", 0);
+        collectUnmapped(documentFields, () -> sourceLookup.source(), "", 0);
         return documentFields;
     }
 
-    private void collectUnmapped(Map<String, DocumentField> documentFields, Map<String, Object> source, String parentPath, int lastState) {
+    private void collectUnmapped(
+        Map<String, DocumentField> documentFields,
+        Supplier<Map<String, Object>> source,
+        String parentPath,
+        int lastState
+    ) {
         // lookup field patterns containing wildcards
         if (this.unmappedFieldsFetchAutomaton != null) {
-            for (String key : source.keySet()) {
-                Object value = source.get(key);
+            for (String key : source.get().keySet()) {
+                Object value = source.get().get(key);
                 String currentPath = parentPath + key;
                 if (this.fieldContexts.containsKey(currentPath)) {
                     continue;
@@ -196,7 +202,7 @@ private void collectUnmapped(Map<String, DocumentField> documentFields, Map<Stri
                     Map<String, Object> objectMap = (Map<String, Object>) value;
                     collectUnmapped(
                         documentFields,
-                        objectMap,
+                        () -> objectMap,
                         currentPath + ".",
                         step(this.unmappedFieldsFetchAutomaton, ".", currentState)
                     );
@@ -227,7 +233,7 @@ private void collectUnmapped(Map<String, DocumentField> documentFields, Map<Stri
                 if (this.fieldContexts.containsKey(path)) {
                     continue; // this is actually a mapped field
                 }
-                List<Object> values = XContentMapValues.extractRawValues(path, source);
+                List<Object> values = XContentMapValues.extractRawValues(path, source.get());
                 if (values.isEmpty() == false) {
                     documentFields.put(path, new DocumentField(path, values));
                 }
@@ -241,7 +247,7 @@ private void collectUnmappedList(Map<String, DocumentField> documentFields, Iter
             if (value instanceof Map) {
                 @SuppressWarnings("unchecked")
                 final Map<String, Object> objectMap = (Map<String, Object>) value;
-                collectUnmapped(documentFields, objectMap, parentPath + ".", step(this.unmappedFieldsFetchAutomaton, ".", lastState));
+                collectUnmapped(documentFields, () -> objectMap, parentPath + ".", step(this.unmappedFieldsFetchAutomaton, ".", lastState));
             } else if (value instanceof List) {
                 // weird case, but can happen for objects with "enabled" : "false"
                 collectUnmappedList(documentFields, (List<?>) value, parentPath, lastState);

diff --git a/server/src/main/java/org/elasticsearch/search/lookup/SearchLookup.java b/server/src/main/java/org/elasticsearch/search/lookup/SearchLookup.java
@@ -53,11 +53,12 @@ public class SearchLookup {
      */
     public SearchLookup(
         Function<String, MappedFieldType> fieldTypeLookup,
-        TriFunction<MappedFieldType, Supplier<SearchLookup>, MappedFieldType.FielddataOperation, IndexFieldData<?>> fieldDataLookup
+        TriFunction<MappedFieldType, Supplier<SearchLookup>, MappedFieldType.FielddataOperation, IndexFieldData<?>> fieldDataLookup,
+        SourceLookup.SourceProvider sourceProvider
     ) {
         this.fieldTypeLookup = fieldTypeLookup;
         this.fieldChain = Collections.emptySet();
-        this.sourceLookup = new SourceLookup();
+        this.sourceLookup = new SourceLookup(sourceProvider);
         this.fieldDataLookup = fieldDataLookup;
     }