Skip to content

Commit

Permalink
[7.14] Improve circular reference detection in grok processor (elasti…
Browse files Browse the repository at this point in the history 
…c#74581) (elastic#74829)
  • Loading branch information
@danhermann @droberts195
danhermann authored and droberts195 committed Jul 5, 2021
1 parent c1e7781 commit 3bb551d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 19 deletions.
48 changes: 34 additions & 14 deletions libs/grok/src/main/java/org/elasticsearch/grok/Grok.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ private Grok(Map<String, String> patternBank, String grokPattern, boolean namedC
this.namedCaptures = namedCaptures;
this.matcherWatchdog = matcherWatchdog;

for (Map.Entry<String, String> entry : patternBank.entrySet()) {
String name = entry.getKey();
String pattern = entry.getValue();
forbidCircularReferences(name, new ArrayList<>(), pattern);
}
forbidCircularReferences();

String expression = toRegex(grokPattern);
byte[] expressionBytes = expression.getBytes(StandardCharsets.UTF_8);
Expand All @@ -107,8 +103,27 @@ private Grok(Map<String, String> patternBank, String grokPattern, boolean namedC
* a reference to another named pattern. This method will navigate to all these named patterns and
* check for a circular reference.
*/
private void forbidCircularReferences(String patternName, List<String> path, String pattern) {
if (pattern.contains("%{" + patternName + "}") || pattern.contains("%{" + patternName + ":")) {
private void forbidCircularReferences() {

// 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<String, String> 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 each pattern
for (Map.Entry<String, String> entry : patternBank.entrySet()) {
String name = entry.getKey();
String pattern = entry.getValue();
innerForbidCircularReferences(name, new ArrayList<>(), pattern);
}
}

private void innerForbidCircularReferences(String patternName, List<String> path, String pattern) {
if (patternReferencesItself(pattern, patternName)) {
String message;
if (path.isEmpty()) {
message = "circular reference in pattern [" + patternName + "][" + pattern + "]";
Expand All @@ -123,26 +138,31 @@ private void forbidCircularReferences(String patternName, List<String> 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");
}
String otherPatternName = pattern.substring(begin, end);
path.add(otherPatternName);
forbidCircularReferences(patternName, path, patternBank.get(otherPatternName));
innerForbidCircularReferences(patternName, path, patternBank.get(otherPatternName));
}
}

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,
Expand Down
22 changes: 17 additions & 5 deletions libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> bank = new TreeMap<>();
Expand All @@ -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<String, String> 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() {
Expand Down

0 comments on commit 3bb551d

Please sign in to comment.