grok validate patterns recusrion to iterative

Signed-off-by: Sandesh Kumar <[email protected]>
sandeshkr419 · Jun 12, 2024 · b476c05 · b476c05
1 parent 2f8cb07
commit b476c05
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -75,6 +75,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix NPE on restore searchable snapshot ([#13911](https://github.com/opensearch-project/OpenSearch/pull/13911))
 - Fix double invocation of postCollection when MultiBucketCollector is present ([#14015](https://github.com/opensearch-project/OpenSearch/pull/14015))
 - Java high-level REST client bulk() is not respecting the bulkRequest.requireAlias(true) method call ([#14146](https://github.com/opensearch-project/OpenSearch/pull/14146))
+- Refactoring Grok.validatePatternBank by using an iterative approach ([#14206](https://github.com/opensearch-project/OpenSearch/pull/14206))
 
 ### Security
 

diff --git a/libs/grok/src/main/java/org/opensearch/grok/Grok.java b/libs/grok/src/main/java/org/opensearch/grok/Grok.java
@@ -39,11 +39,14 @@
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.Stack;
 import java.util.function.Consumer;
 
@@ -144,7 +147,7 @@ private Grok(
      */
     private void validatePatternBank() {
         for (String patternName : patternBank.keySet()) {
-            validatePatternBank(patternName, new Stack<>());
+            validatePatternBank(patternName);
         }
     }
 
@@ -156,33 +159,82 @@ private void validatePatternBank() {
      * a reference to another named pattern. This method will navigate to all these named patterns and
      * check for a circular reference.
      */
-    private void validatePatternBank(String patternName, Stack<String> path) {
-        String pattern = patternBank.get(patternName);
-        boolean isSelfReference = pattern.contains("%{" + patternName + "}") || pattern.contains("%{" + patternName + ":");
-        if (isSelfReference) {
-            throwExceptionForCircularReference(patternName, pattern);
-        } else if (path.contains(patternName)) {
-            // current pattern name is already in the path, fetch its predecessor
-            String prevPatternName = path.pop();
-            String prevPattern = patternBank.get(prevPatternName);
-            throwExceptionForCircularReference(prevPatternName, prevPattern, patternName, path);
-        }
-        path.push(patternName);
-        for (int i = pattern.indexOf("%{"); i != -1; i = pattern.indexOf("%{", i + 1)) {
-            int begin = i + 2;
-            int syntaxEndIndex = pattern.indexOf('}', begin);
-            if (syntaxEndIndex == -1) {
-                throw new IllegalArgumentException("Malformed pattern [" + patternName + "][" + pattern + "]");
+    private void validatePatternBank(String initialPatternName) {
+        Stack<Frame> stack = new Stack<>();
+        Set<String> visitedPatterns = new HashSet<>();
+        Map<String, Stack<String>> pathMap = new HashMap<>();
+
+        Stack<String> initialPath = new Stack<>();
+        initialPath.push(initialPatternName);
+        pathMap.put(initialPatternName, initialPath);
+        stack.push(new Frame(initialPatternName, initialPath, 0));
+
+        while (!stack.isEmpty()) {
+            Frame frame = stack.peek();
+            String patternName = frame.patternName;
+            Stack<String> path = frame.path;
+            int startIndex = frame.startIndex;
+            String pattern = patternBank.get(patternName);
+
+            if (visitedPatterns.contains(patternName)) {
+                stack.pop();
+                continue;
             }
-            int semanticNameIndex = pattern.indexOf(':', begin);
-            int end = syntaxEndIndex;
-            if (semanticNameIndex != -1) {
-                end = Math.min(syntaxEndIndex, semanticNameIndex);
+
+            visitedPatterns.add(patternName);
+            boolean foundDependency = false;
+
+            for (int i = startIndex; i < pattern.length(); i++) {
+                if (pattern.startsWith("%{", i)) {
+                    int begin = i + 2;
+                    int syntaxEndIndex = pattern.indexOf('}', begin);
+                    if (syntaxEndIndex == -1) {
+                        throw new IllegalArgumentException("Malformed pattern [" + patternName + "][" + pattern + "]");
+                    }
+
+                    int semanticNameIndex = pattern.indexOf(':', begin);
+                    int end = semanticNameIndex == -1 ? syntaxEndIndex : Math.min(syntaxEndIndex, semanticNameIndex);
+
+                    String dependsOnPattern = pattern.substring(begin, end);
+
+                    if (dependsOnPattern.equals(patternName)) {
+                        throwExceptionForCircularReference(patternName, pattern);
+                    }
+
+                    if (pathMap.containsKey(dependsOnPattern)) {
+                        path.remove(path.size() - 1);
+                        throwExceptionForCircularReference(patternName, pattern, dependsOnPattern, path);
+                    }
+
+                    Stack<String> newPath = new Stack<>();
+                    newPath.addAll(path);
+                    newPath.push(dependsOnPattern);
+                    pathMap.put(dependsOnPattern, newPath);
+
+                    stack.push(new Frame(dependsOnPattern, newPath, 0));
+                    frame.startIndex = i + 1;
+                    foundDependency = true;
+                    break;
+                }
+            }
+
+            if (!foundDependency) {
+                pathMap.remove(patternName);
+                stack.pop();
             }
-            String dependsOnPattern = pattern.substring(begin, end);
-            validatePatternBank(dependsOnPattern, path);
         }
-        path.pop();
+    }
+
+    private static class Frame {
+        String patternName;
+        Stack<String> path;
+        int startIndex;
+
+        Frame(String patternName, Stack<String> path, int startIndex) {
+            this.patternName = patternName;
+            this.path = path;
+            this.startIndex = startIndex;
+        }
     }
 
     private static void throwExceptionForCircularReference(String patternName, String pattern) {