Adding depth check in doc parser for deep nested document

Fixing the issue (#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com>
opensearch-project · Dec 1, 2022 · ae7253f · ae7253f
1 parent 0210b76
commit ae7253f
Show file tree

Hide file tree

Showing 4 changed files with 135 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -77,6 +77,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
 - Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
 - Reject bulk requests with invalid actions ([#5299](https://github.com/opensearch-project/OpenSearch/issues/5299))
+- Added depth check in doc parser for deep nested document ([#5199](https://github.com/opensearch-project/OpenSearch/pull/5199))
 
 ### Security
 

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,34 @@ 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 +737,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
@@ -1486,4 +1486,78 @@ 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"));
+    }
 }