Adding depth check nested array fields

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

Hide file tree

Showing 3 changed files with 93 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
@@ -567,6 +567,8 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
     private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName, String[] paths)
         throws IOException {
         String arrayFieldName = lastFieldName;
+        context.incrementFieldArrayDepth();
+        context.checkFieldArrayDepthLimit();
 
         Mapper mapper = getMapper(context, parentMapper, lastFieldName, paths);
         if (mapper != null) {
@@ -611,6 +613,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
                 context.path().remove();
             }
         }
+        context.decrementFieldArrayDepth();
     }
 
     private static boolean parsesArrayValue(Mapper mapper) {

diff --git a/server/src/main/java/org/opensearch/index/mapper/ParseContext.java b/server/src/main/java/org/opensearch/index/mapper/ParseContext.java
@@ -328,6 +328,21 @@ public void decrementFieldCurrentDepth() {
         public void checkFieldDepthLimit() {
             in.checkFieldDepthLimit();
         }
+
+        @Override
+        public void incrementFieldArrayDepth() {
+            in.incrementFieldArrayDepth();
+        }
+
+        @Override
+        public void decrementFieldArrayDepth() {
+            in.decrementFieldArrayDepth();
+        }
+
+        @Override
+        public void checkFieldArrayDepthLimit() {
+            in.checkFieldArrayDepthLimit();
+        }
     }
 
     /**
@@ -365,6 +380,10 @@ public static class InternalParseContext extends ParseContext {
 
         private final long maxAllowedFieldDepth;
 
+        private long currentArrayDepth;
+
+        private final long maxAllowedArrayDepth;
+
         private final List<Mapper> dynamicMappers;
 
         private boolean docsReversed = false;
@@ -392,7 +411,9 @@ public InternalParseContext(
             this.maxAllowedNumNestedDocs = indexSettings.getMappingNestedDocsLimit();
             this.numNestedDocs = 0L;
             this.currentFieldDepth = 0L;
+            this.currentArrayDepth = 0L;
             this.maxAllowedFieldDepth = indexSettings.getMappingDepthLimit();
+            this.maxAllowedArrayDepth = indexSettings.getMappingDepthLimit();
         }
 
         @Override
@@ -572,6 +593,32 @@ public void checkFieldDepthLimit() {
             }
         }
 
+        @Override
+        public void incrementFieldArrayDepth() {
+            this.currentArrayDepth++;
+        }
+
+        @Override
+        public void decrementFieldArrayDepth() {
+            if (this.currentArrayDepth > 0) {
+                this.currentArrayDepth--;
+            }
+        }
+
+        @Override
+        public void checkFieldArrayDepthLimit() {
+            if (this.currentArrayDepth > maxAllowedArrayDepth) {
+                this.currentArrayDepth = 0;
+                throw new OpenSearchParseException(
+                    "The depth of the nested array field has exceeded the allowed limit of ["
+                        + maxAllowedArrayDepth
+                        + "]."
+                        + " This limit can be set by changing the ["
+                        + MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey()
+                        + "] index level setting."
+                );
+            }
+        }
     }
 
     /**
@@ -744,4 +791,10 @@ public final <T> T parseExternalValue(Class<T> clazz) {
 
     public abstract void checkFieldDepthLimit();
 
+    public abstract void incrementFieldArrayDepth();
+
+    public abstract void decrementFieldArrayDepth();
+
+    public abstract void checkFieldArrayDepthLimit();
+
 }
diff --git a/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java
@@ -1560,4 +1560,41 @@ public void testDocumentContainsDeepNestedFieldParsingFail() throws Exception {
         assertNotNull(inner3_mapper);
         assertThat(doc.rootDoc().get("inner1.inner2.inner3.inner3_field1"), equalTo("inner3_value1"));
     }
+
+    public void testDocumentContainsDeepNestedFieldParsingShouldFail() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { b.field("type", "nested"); }));
+        long depth_limit = MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getDefault(Settings.EMPTY);
+        MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {
+            b.startObject("field");
+            b.startArray("inner");
+            for (int i = 1; i <= depth_limit; i++) {
+                b.startArray();
+            }
+            b.startArray().value(0).value(0).endArray();
+            for (int i = 1; i <= depth_limit; i++) {
+                b.endArray();
+            }
+            b.endArray();
+            b.endObject();
+        })));
+        // check parsing success for nested array within allowed depth limit
+        ParsedDocument doc = mapper.parse(source(b -> {
+            b.startObject("field");
+            b.startArray("inner");
+            for (int i = 1; i < depth_limit - 1; i++) {
+                b.startArray();
+            }
+            b.startArray().value(0).value(0).endArray();
+            for (int i = 1; i < depth_limit - 1; i++) {
+                b.endArray();
+            }
+            b.endArray();
+            b.endObject();
+        }
+
+        ));
+        Mapping update = doc.dynamicMappingsUpdate();
+        assertNotNull(update); // dynamic mapping update
+
+    }
 }