Adding depth check in doc parser for deep nested document

Fixing the issue (#5195)
opensearch-project · Nov 10, 2022 · fe1f692 · fe1f692
1 parent e10c925
commit fe1f692
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 0 deletions.
diff --git a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java
@@ -427,6 +427,8 @@ private static void innerParseObject(
     ) throws IOException {
         assert token == XContentParser.Token.FIELD_NAME || token == XContentParser.Token.END_OBJECT;
         String[] paths = null;
+        context.incrementFieldCurrentDepth();
+        context.checkFieldDepthLimit();
         while (token != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
@@ -454,6 +456,7 @@ private static void innerParseObject(
             }
             token = parser.nextToken();
         }
+        context.decrementFieldCurrentDepth();
     }
 
     private static void nested(ParseContext context, ObjectMapper.Nested nested) {

diff --git a/server/src/main/java/org/opensearch/index/mapper/ParseContext.java b/server/src/main/java/org/opensearch/index/mapper/ParseContext.java
@@ -37,6 +37,7 @@
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.util.BytesRef;
+import org.opensearch.OpenSearchParseException;
 import org.opensearch.common.xcontent.XContentParser;
 import org.opensearch.index.IndexSettings;
 
@@ -312,6 +313,21 @@ public void addIgnoredField(String field) {
         public Collection<String> getIgnoredFields() {
             return in.getIgnoredFields();
         }
+
+        @Override
+        public void incrementFieldCurrentDepth() {
+            in.incrementFieldCurrentDepth();
+        }
+
+        @Override
+        public void decrementFieldCurrentDepth() {
+            in.decrementFieldCurrentDepth();
+        }
+
+        @Override
+        public void checkFieldDepthLimit() {
+            in.checkFieldDepthLimit();
+        }
     }
 
     /**
@@ -345,6 +361,10 @@ public static class InternalParseContext extends ParseContext {
 
         private long numNestedDocs;
 
+        private long currentFieldDepth;
+
+        private final long maxAllowedFieldDepth;
+
         private final List<Mapper> dynamicMappers;
 
         private boolean docsReversed = false;
@@ -371,6 +391,8 @@ public InternalParseContext(
             this.dynamicMappers = new ArrayList<>();
             this.maxAllowedNumNestedDocs = indexSettings.getMappingNestedDocsLimit();
             this.numNestedDocs = 0L;
+            this.currentFieldDepth = 0L;
+            this.maxAllowedFieldDepth = indexSettings.getMappingDepthLimit();
         }
 
         @Override
@@ -522,6 +544,33 @@ public void addIgnoredField(String field) {
         public Collection<String> getIgnoredFields() {
             return Collections.unmodifiableCollection(ignoredFields);
         }
+
+        @Override
+        public void incrementFieldCurrentDepth() {
+            this.currentFieldDepth++;
+        }
+
+        @Override
+        public void decrementFieldCurrentDepth(){
+            if(this.currentFieldDepth > 0) {
+                this.currentFieldDepth--;
+            }
+        }
+
+        @Override
+        public void checkFieldDepthLimit(){
+            if(this.currentFieldDepth > maxAllowedFieldDepth){
+                this.currentFieldDepth = 0;
+                throw new OpenSearchParseException("The depth of the field has exceeded the allowed limit of ["
+                    + maxAllowedFieldDepth
+                    + "]."
+                    + " This limit can be set by changing the ["
+                    + MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey()
+                    + "] index level setting."
+                );
+            }
+        }
+
     }
 
     /**
@@ -687,4 +736,11 @@ public final <T> T parseExternalValue(Class<T> clazz) {
      * Get dynamic mappers created while parsing.
      */
     public abstract List<Mapper> getDynamicMappers();
+
+    public abstract void incrementFieldCurrentDepth();
+
+    public abstract void decrementFieldCurrentDepth();
+
+    public abstract void checkFieldDepthLimit();
+
 }
diff --git a/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java
@@ -55,7 +55,9 @@
 import java.util.Collections;
 import java.util.List;
 
+import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
+import static java.util.stream.Collectors.toList;
 import static org.opensearch.test.StreamsUtils.copyToBytesFromClasspath;
 import static org.opensearch.test.StreamsUtils.copyToStringFromClasspath;
 import static org.hamcrest.Matchers.containsString;
@@ -1486,4 +1488,82 @@ public void testTypeless() throws IOException {
         ParsedDocument doc = mapper.parse(source(b -> b.field("foo", "1234")));
         assertNull(doc.dynamicMappingsUpdate()); // no update since we reused the existing type
     }
+
+    public void testDocumentContainsDeepNestedFieldParsing() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(mapping(b -> {}));
+        ParsedDocument doc = mapper.parse(source(b -> {
+            b.startObject("inner1");
+            {
+                b.field("inner1_field1", "inner1_value1");
+                b.startObject("inner2");
+                {
+                    b.startObject("inner3");
+                    {
+                        b.field("inner3_field1", "inner3_value1");
+                        b.field("inner3_field2", "inner3_value2");
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+            }
+            b.endObject();
+        }));
+
+
+        Mapping update = doc.dynamicMappingsUpdate();
+        assertNotNull(update); //dynamic mapping update
+
+        Mapper objMapper = update.root().getMapper("inner1");
+        Mapper inner1_field1_mapper = ((ObjectMapper) objMapper).getMapper("inner1_field1");
+        assertNotNull(inner1_field1_mapper);
+        Mapper inner2_mapper = ((ObjectMapper) objMapper).getMapper("inner2");
+        assertNotNull(inner2_mapper);
+        Mapper inner3_mapper = ((ObjectMapper) inner2_mapper).getMapper("inner3");
+        assertNotNull(inner3_mapper);
+        assertThat(doc.rootDoc().get("inner1.inner2.inner3.inner3_field1"), equalTo("inner3_value1"));
+    }
+
+    public void testDocumentContainsDeepNestedFieldParsingFail() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(mapping(b -> {
+        }));
+        long depth_limit = MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getDefault(Settings.EMPTY);
+        MapperParsingException e = expectThrows(MapperParsingException.class,
+            () -> mapper.parse(source(b -> {
+                for(int i = 1; i <= depth_limit; i++){
+                    b.startObject("inner" + i);
+                }
+                b.field("inner_field", "inner_value");
+                for(int i = 1; i <= depth_limit; i++){
+                    b.endObject();
+                }
+            })));
+
+        //check that parsing succeeds with valid doc
+        //after throwing exception
+        ParsedDocument doc = mapper.parse(source(b -> {
+            b.startObject("inner1");
+            {
+                b.startObject("inner2");
+                {
+                    b.startObject("inner3");
+                    {
+                        b.field("inner3_field1", "inner3_value1");
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+            }
+            b.endObject();
+        }));
+
+
+        Mapping update = doc.dynamicMappingsUpdate();
+        assertNotNull(update); //dynamic mapping update
+        Mapper objMapper = update.root().getMapper("inner1");
+        Mapper inner2_mapper = ((ObjectMapper) objMapper).getMapper("inner2");
+        assertNotNull(inner2_mapper);
+        Mapper inner3_mapper = ((ObjectMapper) inner2_mapper).getMapper("inner3");
+        assertNotNull(inner3_mapper);
+        assertThat(doc.rootDoc().get("inner1.inner2.inner3.inner3_field1"), equalTo("inner3_value1"));
+    }
 }