fix grok's pattern parsing to validate pattern names in expression (#…

…25063) Unknown patterns used to silently be ignored. This was a problem because users did not know they were providing an invalid pattern name, and maybe thought the rest of their regexes were invalid. Fixes #22831.
elastic · Jun 6, 2017 · 19d8116 · 19d8116
1 parent 88f4594
commit 19d8116
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 1 deletion.
diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Grok.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Grok.java
@@ -29,7 +29,6 @@
 import org.joni.exception.ValueException;
 
 import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Locale;
@@ -107,8 +106,13 @@ public String toRegex(String grokPattern) {
             // TODO(tal): Support definitions
             String definition = groupMatch(DEFINITION_GROUP, region, grokPattern);
             String patternName = groupMatch(PATTERN_GROUP, region, grokPattern);
+
             String pattern = patternBank.get(patternName);
 
+            if (pattern == null) {
+                throw new IllegalArgumentException("Unable to find pattern [" + patternName + "] in Grok's pattern dictionary");
+            }
+
             String grokPart;
             if (namedCaptures && subName != null) {
                 grokPart = String.format(Locale.US, "(?<%s>%s)", namedPatternRef, pattern);

diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java
@@ -54,6 +54,16 @@ public void testNoMatch() {
         assertThat(e.getMessage(), equalTo("Provided Grok expressions do not match field value: [23]"));
     }
 
+    public void testNoMatchingPatternName() {
+        String fieldName = RandomDocumentPicks.randomFieldName(random());
+        IngestDocument doc = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
+        doc.setFieldValue(fieldName, "23");
+        Exception e = expectThrows(IllegalArgumentException.class, () -> new GrokProcessor(randomAlphaOfLength(10),
+            Collections.singletonMap("ONE", "1"), Collections.singletonList("%{NOTONE:not_one}"), fieldName,
+            false, false));
+        assertThat(e.getMessage(), equalTo("Unable to find pattern [NOTONE] in Grok's pattern dictionary"));
+    }
+
     public void testMatchWithoutCaptures() throws Exception {
         String fieldName = "value";
         IngestDocument originalDoc = new IngestDocument(new HashMap<>(), new HashMap<>());

diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokTests.java
@@ -24,6 +24,7 @@
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -48,6 +49,11 @@ public void testMatchWithoutCaptures() {
         assertEquals(0, matches.size());
     }
 
+    public void testNoMatchingPatternInDictionary() {
+        Exception e = expectThrows(IllegalArgumentException.class, () -> new Grok(Collections.emptyMap(), "%{NOTFOUND}"));
+        assertThat(e.getMessage(), equalTo("Unable to find pattern [NOTFOUND] in Grok's pattern dictionary"));
+    }
+
     public void testSimpleSyslogLine() {
         String line = "Mar 16 00:01:25 evita postfix/smtpd[1713]: connect from camomile.cloud9.net[168.100.1.3]";
         Grok grok = new Grok(basePatterns, "%{SYSLOGLINE}");