Restrict Document list access in ParseContext (#29463)

Today we expose a mutable list of documents in ParseContext via
ParseContext#docs(). This, on the one hand places knowledge how
to access nested documnts in multiple places and on the other
allows for potential illegal access to nested only docs after
the docs are reversed. This change restricts the access and
streamlines nested / non-root doc access.

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

@@ -76,7 +76,7 @@ final class DocumentParser {
             throw new IllegalStateException("found leftover path elements: " + remainingPath);
         }
 
-        reverseOrder(context);
+        context.postParse();
 
         return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
     }
@@ -141,12 +141,6 @@ final class DocumentParser {
         return false;
     }
 
-    private static void reverseOrder(ParseContext.InternalParseContext context) {
-        // reverse the order of docs for nested docs support, parent should be last
-        if (context.docs().size() > 1) {
-            Collections.reverse(context.docs());
-        }
-    }
 
     private static ParsedDocument parsedDocument(SourceToParse source, ParseContext.InternalParseContext context, Mapping update) {
         return new ParsedDocument(

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

@@ -263,7 +263,7 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
         if (fieldType().isEnabled() == false) {
             return;
         }
-        for (ParseContext.Document document : context.docs()) {
+        for (ParseContext.Document document : context) {
             final List<String> paths = new ArrayList<>(document.getFields().size());
             String previousPath = ""; // used as a sentinel - field names can't be empty
             for (IndexableField field : document.getFields()) {

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

@@ -267,9 +267,6 @@ public class IdFieldMapper extends MetadataFieldMapper {
         super.parse(context);
     }
 
-    @Override
-    public void postParse(ParseContext context) throws IOException {}
-
     @Override
     protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {
         if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) {

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

@@ -172,9 +172,6 @@ public class IndexFieldMapper extends MetadataFieldMapper {
     @Override
     public void preParse(ParseContext context) throws IOException {}
 
-    @Override
-    public void postParse(ParseContext context) throws IOException {}
-
     @Override
     protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {}
 

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

@@ -64,7 +64,9 @@ public abstract class MetadataFieldMapper extends FieldMapper {
     /**
      * Called after {@link FieldMapper#parse(ParseContext)} on the {@link RootObjectMapper}.
      */
-    public abstract void postParse(ParseContext context) throws IOException;
+    public void postParse(ParseContext context) throws IOException {
+        // do nothing
+    }
 
     @Override
     public MetadataFieldMapper merge(Mapper mergeWith) {

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

@@ -29,10 +29,11 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
-public abstract class ParseContext {
+public abstract class ParseContext implements Iterable<ParseContext.Document>{
 
     /** Fork of {@link org.apache.lucene.document.Document} with additional functionality. */
     public static class Document implements Iterable<IndexableField> {
@@ -171,6 +172,11 @@ public abstract class ParseContext {
             this.in = in;
         }
 
+        @Override
+        public Iterable<Document> nonRootDocuments() {
+            return in.nonRootDocuments();
+        }
+
         @Override
         public DocumentMapperParser docMapperParser() {
             return in.docMapperParser();
@@ -211,11 +217,6 @@ public abstract class ParseContext {
             return in.rootDoc();
         }
 
-        @Override
-        public List<Document> docs() {
-            return in.docs();
-        }
-
         @Override
         public Document doc() {
             return in.doc();
@@ -280,6 +281,11 @@ public abstract class ParseContext {
         public List<Mapper> getDynamicMappers() {
             return in.getDynamicMappers();
         }
+
+        @Override
+        public Iterator<Document> iterator() {
+            return in.iterator();
+        }
     }
 
     public static class InternalParseContext extends ParseContext {
@@ -309,9 +315,10 @@ public abstract class ParseContext {
 
         private long numNestedDocs;
 
-
         private final List<Mapper> dynamicMappers;
 
+        private boolean docsReversed = false;
+
         public InternalParseContext(@Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper,
                 SourceToParse source, XContentParser parser) {
             this.indexSettings = indexSettings;
@@ -360,8 +367,7 @@ public abstract class ParseContext {
             return documents.get(0);
         }
 
-        @Override
-        public List<Document> docs() {
+        List<Document> docs() {
             return this.documents;
         }
 
@@ -426,8 +432,35 @@ public abstract class ParseContext {
         public List<Mapper> getDynamicMappers() {
             return dynamicMappers;
         }
+
+        @Override
+        public Iterable<Document> nonRootDocuments() {
+            if (docsReversed) {
+                throw new IllegalStateException("documents are already reversed");
+            }
+            return documents.subList(1, documents.size());
+        }
+
+        void postParse() {
+            // reverse the order of docs for nested docs support, parent should be last
+            if (documents.size() > 1) {
+                docsReversed = true;
+                Collections.reverse(documents);
+            }
+        }
+
+        @Override
+        public Iterator<Document> iterator() {
+            return documents.iterator();
+        }
     }
 
+    /**
+     * Returns an Iterable over all non-root documents. If there are no non-root documents
+     * the iterable will return an empty iterator.
+     */
+    public abstract Iterable<Document> nonRootDocuments();
+
     public abstract DocumentMapperParser docMapperParser();
 
     /**
@@ -506,8 +539,6 @@ public abstract class ParseContext {
 
     public abstract Document rootDoc();
 
-    public abstract List<Document> docs();
-
     public abstract Document doc();
 
     protected abstract void addDoc(Document doc);

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

@@ -157,10 +157,6 @@ public class RoutingFieldMapper extends MetadataFieldMapper {
         super.parse(context);
     }
 
-    @Override
-    public void postParse(ParseContext context) throws IOException {
-    }
-
     @Override
     public Mapper parse(ParseContext context) throws IOException {
         // no need ot parse here, we either get the routing in the sourceToParse

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

@@ -253,11 +253,9 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
         // we share the parent docs fields to ensure good compression
         SequenceIDFields seqID = context.seqID();
         assert seqID != null;
-        int numDocs = context.docs().size();
         final Version versionCreated = context.mapperService().getIndexSettings().getIndexVersionCreated();
         final boolean includePrimaryTerm = versionCreated.before(Version.V_6_1_0);
-        for (int i = 1; i < numDocs; i++) {
-            final Document doc = context.docs().get(i);
+        for (Document doc : context.nonRootDocuments()) {
             doc.add(seqID.seqNo);
             doc.add(seqID.seqNoDocValue);
             if (includePrimaryTerm) {

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

@@ -216,10 +216,6 @@ public class SourceFieldMapper extends MetadataFieldMapper {
         super.parse(context);
     }
 
-    @Override
-    public void postParse(ParseContext context) throws IOException {
-    }
-
     @Override
     public Mapper parse(ParseContext context) throws IOException {
         // nothing to do here, we will call it in pre parse
@@ -228,32 +224,23 @@ public class SourceFieldMapper extends MetadataFieldMapper {
 
     @Override
     protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {
-        if (!enabled) {
-            return;
-        }
-        if (!fieldType().stored()) {
-            return;
-        }
         BytesReference source = context.sourceToParse().source();
-        // Percolate and tv APIs may not set the source and that is ok, because these APIs will not index any data
-        if (source == null) {
-            return;
-        }
-
-        if (filter != null) {
-            // we don't update the context source if we filter, we want to keep it as is...
-            Tuple<XContentType, Map<String, Object>> mapTuple =
-                XContentHelper.convertToMap(source, true, context.sourceToParse().getXContentType());
-            Map<String, Object> filteredSource = filter.apply(mapTuple.v2());
-            BytesStreamOutput bStream = new BytesStreamOutput();
-            XContentType contentType = mapTuple.v1();
-            XContentBuilder builder = XContentFactory.contentBuilder(contentType, bStream).map(filteredSource);
-            builder.close();
-
-            source = bStream.bytes();
+        if (enabled && fieldType().stored() && source != null) {
+            // Percolate and tv APIs may not set the source and that is ok, because these APIs will not index any data
+            if (filter != null) {
+                // we don't update the context source if we filter, we want to keep it as is...
+                Tuple<XContentType, Map<String, Object>> mapTuple =
+                    XContentHelper.convertToMap(source, true, context.sourceToParse().getXContentType());
+                Map<String, Object> filteredSource = filter.apply(mapTuple.v2());
+                BytesStreamOutput bStream = new BytesStreamOutput();
+                XContentType contentType = mapTuple.v1();
+                XContentBuilder builder = XContentFactory.contentBuilder(contentType, bStream).map(filteredSource);
+                builder.close();
+                source = bStream.bytes();
+            }
+            BytesRef ref = source.toBytesRef();
+            fields.add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
         }
-        BytesRef ref = source.toBytesRef();
-        fields.add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
     }
 
     @Override

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

@@ -269,10 +269,6 @@ public class TypeFieldMapper extends MetadataFieldMapper {
         super.parse(context);
     }
 
-    @Override
-    public void postParse(ParseContext context) throws IOException {
-    }
-
     @Override
     public Mapper parse(ParseContext context) throws IOException {
         // we parse in pre parse

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

@@ -128,8 +128,7 @@ public class VersionFieldMapper extends MetadataFieldMapper {
         // that don't have the field. This is consistent with the default value for efficiency.
         Field version = context.version();
         assert version != null;
-        for (int i = 1; i < context.docs().size(); i++) {
-            final Document doc = context.docs().get(i);
+        for (Document doc : context.nonRootDocuments()) {
             doc.add(version);
         }
     }