Prevent deep recursion in doc values

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionFieldScriptTests.java

@@ -64,7 +64,7 @@ public void setUp() throws Exception {
         when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(mapperService, ignored -> fieldData);
+        lookup = new SearchLookup(mapperService, (ignored, l) -> fieldData);
     }
 
     private FieldScript.LeafFactory compile(String expression) {

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionNumberSortScriptTests.java

@@ -64,7 +64,7 @@ public void setUp() throws Exception {
         when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(mapperService, ignored -> fieldData);
+        lookup = new SearchLookup(mapperService, (ignored, l) -> fieldData);
     }
 
     private NumberSortScript.LeafFactory compile(String expression) {

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/ExpressionTermsSetQueryTests.java

@@ -64,7 +64,7 @@ public void setUp() throws Exception {
         when(fieldData.load(anyObject())).thenReturn(atomicFieldData);
 
         service = new ExpressionScriptEngine();
-        lookup = new SearchLookup(mapperService, ignored -> fieldData);
+        lookup = new SearchLookup(mapperService, (ignored, l) -> fieldData);
     }
 
     private TermsSetQueryScript.LeafFactory compile(String expression) {

modules/lang-painless/src/test/java/org/elasticsearch/painless/NeedsScoreTests.java

@@ -25,7 +25,6 @@
 import org.elasticsearch.painless.spi.Whitelist;
 import org.elasticsearch.script.NumberSortScript;
 import org.elasticsearch.script.ScriptContext;
-import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
 import java.util.Collections;
@@ -47,22 +46,21 @@ public void testNeedsScores() {
         PainlessScriptEngine service = new PainlessScriptEngine(Settings.EMPTY, contexts);
 
         QueryShardContext shardContext = index.newQueryShardContext(0, null, () -> 0, null);
-        SearchLookup lookup = new SearchLookup(index.mapperService(), shardContext::getForField);
 
         NumberSortScript.Factory factory = service.compile(null, "1.2", NumberSortScript.CONTEXT, Collections.emptyMap());
-        NumberSortScript.LeafFactory ss = factory.newFactory(Collections.emptyMap(), lookup);
+        NumberSortScript.LeafFactory ss = factory.newFactory(Collections.emptyMap(), shardContext.lookup());
         assertFalse(ss.needs_score());
 
         factory = service.compile(null, "doc['d'].value", NumberSortScript.CONTEXT, Collections.emptyMap());
-        ss = factory.newFactory(Collections.emptyMap(), lookup);
+        ss = factory.newFactory(Collections.emptyMap(), shardContext.lookup());
         assertFalse(ss.needs_score());
 
         factory = service.compile(null, "1/_score", NumberSortScript.CONTEXT, Collections.emptyMap());
-        ss = factory.newFactory(Collections.emptyMap(), lookup);
+        ss = factory.newFactory(Collections.emptyMap(), shardContext.lookup());
         assertTrue(ss.needs_score());
 
         factory = service.compile(null, "doc['d'].value * _score", NumberSortScript.CONTEXT, Collections.emptyMap());
-        ss = factory.newFactory(Collections.emptyMap(), lookup);
+        ss = factory.newFactory(Collections.emptyMap(), shardContext.lookup());
         assertTrue(ss.needs_score());
     }
 

server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

@@ -209,7 +209,7 @@ public boolean allowExpensiveQueries() {
 
     @SuppressWarnings("unchecked")
     public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
-        return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), this::lookup);
+        return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName(), () -> lookup().forField(fieldType.name()));
     }
 
     public void addNamedQuery(String name, Query query) {
@@ -291,7 +291,10 @@ MappedFieldType failIfFieldMappingNotFound(String name, MappedFieldType fieldMap
 
     public SearchLookup lookup() {
         if (lookup == null) {
-            lookup = new SearchLookup(getMapperService(), this::getForField);
+            lookup = new SearchLookup(
+                getMapperService(),
+                (ft, lookup) -> indexFieldDataService.apply(ft, fullyQualifiedIndex.getName(), lookup)
+            );
         }
         return lookup;
     }

server/src/main/java/org/elasticsearch/search/lookup/SearchLookup.java

@@ -24,22 +24,88 @@
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 
-import java.util.function.Function;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.BiFunction;
+import java.util.function.Supplier;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.unmodifiableList;
 
 public class SearchLookup {
+    /**
+     * The maximum "depth" of field dependencies. When a field's doc values
+     * depends on another field's doc values and depends on another field's
+     * doc values that depends on another field's doc values, etc, etc,
+     * it can make a very deep stack. At some point we're concerned about
+     * {@link StackOverflowError}. This limits that "depth".
+     */
+    static final int MAX_FIELD_CHAIN = 30;
 
-    final DocLookup docMap;
+    /**
+     * The chain of fields for which this lookup was created used for detecting
+     * loops in doc values calculations. It is empty for the "top level" lookup
+     * created for the entire search. When a lookup is created for a field we
+     * make sure its name isn't in the list then add it to the end. So the
+     * lookup for the a field named {@code foo} will be {@code ["foo"]}. If
+     * that field looks up the values of a field named {@code bar} then
+     * {@code bar}'s lookup will contain {@code ["foo", "bar"]}.
+     */
+    private final List<String> fieldChain;
+    private final MapperService mapperService;
+    private final BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> indexFieldDataService;
+    private final DocLookup docMap;
+    private final SourceLookup sourceLookup;
+    private final FieldsLookup fieldsLookup;
 
-    final SourceLookup sourceLookup;
+    /**
+     * Create the "top level" field lookup for a search request.
+     */
+    public SearchLookup(
+        MapperService mapperService,
+        BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> indexFieldDataService
+    ) {
+        this(emptyList(), mapperService, indexFieldDataService, new SourceLookup(), new FieldsLookup(mapperService));
+    }
 
-    final FieldsLookup fieldsLookup;
+    private SearchLookup(
+        List<String> fieldChain,
+        MapperService mapperService,
+        BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> indexFieldDataService,
+        SourceLookup sourceLookup,
+        FieldsLookup fieldsLookup
+    ) {
+        this.fieldChain = fieldChain;
+        this.mapperService = mapperService;;
+        this.indexFieldDataService = indexFieldDataService;
+        this.docMap = new DocLookup(mapperService, ft -> this.indexFieldDataService.apply(ft, () -> forField(ft.name())));
+        this.sourceLookup = sourceLookup;
+        this.fieldsLookup = fieldsLookup;
+    }
 
-    public SearchLookup(MapperService mapperService, Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup) {
-        docMap = new DocLookup(mapperService, fieldDataLookup);
-        sourceLookup = new SourceLookup();
-        fieldsLookup = new FieldsLookup(mapperService);
+    /**
+     * Build a new lookup for a field that needs the lookup to create doc values.
+     *
+     * @throws IllegalArgumentException if it detects a loop in the fields required to build doc values
+     */
+    public SearchLookup forField(String name) {
+        List<String> newFieldChain = new ArrayList<>(fieldChain.size() + 1);
+        newFieldChain.addAll(fieldChain);
+        newFieldChain.add(name);
+        if (fieldChain.contains(name)) {
+            throw new IllegalArgumentException("loop in field definitions: " + newFieldChain);
+        }
+        if (newFieldChain.size() > MAX_FIELD_CHAIN) {
+            throw new IllegalArgumentException("field definition too deep: " + newFieldChain);
+        }
+        return new SearchLookup(unmodifiableList(newFieldChain), mapperService, indexFieldDataService, sourceLookup, fieldsLookup);
     }
 
+    /**
+     * Builds a new {@linkplain LeafSearchLookup} for the caller to handle
+     * a {@linkplain LeafReaderContext}. This lookup shares the {@code _source}
+     * lookup but doesn't share doc values or stored fields.
+     */
     public LeafSearchLookup getLeafSearchLookup(LeafReaderContext context) {
         return new LeafSearchLookup(context,
                 docMap.getLeafDocLookup(context),

server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourcePhaseTests.java

@@ -184,7 +184,9 @@ public FetchSourceContext fetchSourceContext() {
 
         @Override
         public SearchLookup lookup() {
-            SearchLookup lookup = new SearchLookup(this.mapperService(), this::getForField);
+            SearchLookup lookup = new SearchLookup(this.mapperService(), (ft, l) -> {
+                throw new UnsupportedOperationException();
+            });
             lookup.source().setSource(source);
             return lookup;
         }