diff --git a/libs/grok/src/main/java/org/elasticsearch/grok/Grok.java b/libs/grok/src/main/java/org/elasticsearch/grok/Grok.java index ee4e15c3810c8..c0ff4ac57a1f5 100644 --- a/libs/grok/src/main/java/org/elasticsearch/grok/Grok.java +++ b/libs/grok/src/main/java/org/elasticsearch/grok/Grok.java @@ -108,7 +108,21 @@ private Grok(Map patternBank, String grokPattern, boolean namedC * check for a circular reference. */ private void forbidCircularReferences(String patternName, List path, String pattern) { - if (pattern.contains("%{" + patternName + "}") || pattern.contains("%{" + patternName + ":")) { + // first ensure that the pattern bank contains no simple circular references (i.e., any pattern + // containing an immediate reference to itself) as those can cause the remainder of this algorithm + // to recurse infinitely + for (Map.Entry entry : patternBank.entrySet()) { + if (patternReferencesItself(entry.getValue(), entry.getKey())) { + throw new IllegalArgumentException("circular reference in pattern [" + entry.getKey() + "][" + entry.getValue() + "]"); + } + } + + // next recursively check any other pattern names referenced in the pattern + innerForbidCircularReferences(patternName, path, pattern); + } + + private void innerForbidCircularReferences(String patternName, List path, String pattern) { + if (patternReferencesItself(pattern, patternName)) { String message; if (path.isEmpty()) { message = "circular reference in pattern [" + patternName + "][" + pattern + "]"; @@ -123,17 +137,18 @@ private void forbidCircularReferences(String patternName, List path, Str throw new IllegalArgumentException(message); } + // next check any other pattern names found in the pattern for (int i = pattern.indexOf("%{"); i != -1; i = pattern.indexOf("%{", i + 1)) { int begin = i + 2; - int brackedIndex = pattern.indexOf('}', begin); + int bracketIndex = pattern.indexOf('}', begin); int columnIndex = pattern.indexOf(':', begin); int end; - if (brackedIndex != -1 && columnIndex == -1) { - end = brackedIndex; - } else if (columnIndex != -1 && brackedIndex == -1) { + if (bracketIndex != -1 && columnIndex == -1) { + end = bracketIndex; + } else if (columnIndex != -1 && bracketIndex == -1) { end = columnIndex; - } else if (brackedIndex != -1 && columnIndex != -1) { - end = Math.min(brackedIndex, columnIndex); + } else if (bracketIndex != -1 && columnIndex != -1) { + end = Math.min(bracketIndex, columnIndex); } else { throw new IllegalArgumentException("pattern [" + pattern + "] has circular references to other pattern definitions"); } @@ -143,6 +158,10 @@ private void forbidCircularReferences(String patternName, List path, Str } } + private static boolean patternReferencesItself(String pattern, String patternName) { + return pattern.contains("%{" + patternName + "}") || pattern.contains("%{" + patternName + ":"); + } + private String groupMatch(String name, Region region, String pattern) { try { int number = GROK_PATTERN_REGEX.nameToBackrefNumber(name.getBytes(StandardCharsets.UTF_8), 0, diff --git a/libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java b/libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java index db2a44b83f850..570a4c2ec0420 100644 --- a/libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java +++ b/libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java @@ -320,8 +320,7 @@ public void testCircularReference() { String pattern = "%{NAME1}"; new Grok(bank, pattern, false, logger::warn); }); - assertEquals("circular reference in pattern [NAME3][!!!%{NAME1}!!!] back to pattern [NAME1] via patterns [NAME2]", - e.getMessage()); + assertEquals("circular reference in pattern [NAME3][!!!%{NAME1}!!!] back to pattern [NAME1] via patterns [NAME2]", e.getMessage()); e = expectThrows(IllegalArgumentException.class, () -> { Map bank = new TreeMap<>(); @@ -331,10 +330,23 @@ public void testCircularReference() { bank.put("NAME4", "!!!%{NAME5}!!!"); bank.put("NAME5", "!!!%{NAME1}!!!"); String pattern = "%{NAME1}"; - new Grok(bank, pattern, false, logger::warn ); + new Grok(bank, pattern, false, logger::warn); + }); + assertEquals( + "circular reference in pattern [NAME5][!!!%{NAME1}!!!] back to pattern [NAME1] via patterns [NAME2=>NAME3=>NAME4]", + e.getMessage() + ); + } + + public void testCircularSelfReference() { + Exception e = expectThrows(IllegalArgumentException.class, () -> { + Map bank = new HashMap<>(); + bank.put("ANOTHER", "%{INT}"); + bank.put("INT", "%{INT}"); + String pattern = "does_not_matter"; + new Grok(bank, pattern, false, logger::warn); }); - assertEquals("circular reference in pattern [NAME5][!!!%{NAME1}!!!] back to pattern [NAME1] " + - "via patterns [NAME2=>NAME3=>NAME4]", e.getMessage()); + assertEquals("circular reference in pattern [INT][%{INT}]", e.getMessage()); } public void testBooleanCaptures() {