Merge pull request elastic#17260 from talevy/fix-regex-exceptions

Handle regex parsing errors in Gsub and Grok Processors
bleskes · Mar 29, 2016 · 16e888f · 16e888f
2 parents 9ac3887 + 534caa8
commit 16e888f
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 5 deletions.
diff --git a/core/src/main/java/org/elasticsearch/ingest/processor/GsubProcessor.java b/core/src/main/java/org/elasticsearch/ingest/processor/GsubProcessor.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.ingest.processor;
 
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.ingest.core.AbstractProcessor;
 import org.elasticsearch.ingest.core.AbstractProcessorFactory;
 import org.elasticsearch.ingest.core.IngestDocument;
@@ -28,6 +29,9 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static org.elasticsearch.ingest.core.ConfigurationUtils.newConfigurationException;
+import static org.elasticsearch.ingest.core.ConfigurationUtils.readStringProperty;
+
 /**
  * Processor that allows to search for patterns in field content and replace them with corresponding string replacement.
  * Support fields of string type only, throws exception if a field is of a different type.
@@ -79,10 +83,15 @@ public String getType() {
     public static final class Factory extends AbstractProcessorFactory<GsubProcessor> {
         @Override
         public GsubProcessor doCreate(String processorTag, Map<String, Object> config) throws Exception {
-            String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field");
-            String pattern = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "pattern");
-            String replacement = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "replacement");
-            Pattern searchPattern = Pattern.compile(pattern);
+            String field = readStringProperty(TYPE, processorTag, config, "field");
+            String pattern = readStringProperty(TYPE, processorTag, config, "pattern");
+            String replacement = readStringProperty(TYPE, processorTag, config, "replacement");
+            Pattern searchPattern;
+            try {
+                searchPattern = Pattern.compile(pattern);
+            } catch (Exception e) {
+                throw newConfigurationException(TYPE, processorTag, "pattern", "Invalid regex pattern. " + e.getMessage());
+            }
             return new GsubProcessor(processorTag, field, searchPattern, replacement);
         }
     }

diff --git a/core/src/test/java/org/elasticsearch/ingest/processor/GsubProcessorFactoryTests.java b/core/src/test/java/org/elasticsearch/ingest/processor/GsubProcessorFactoryTests.java
@@ -84,4 +84,18 @@ public void testCreateNoReplacementPresent() throws Exception {
             assertThat(e.getMessage(), equalTo("[replacement] required property is missing"));
         }
     }
+
+    public void testCreateInvalidPattern() throws Exception {
+        GsubProcessor.Factory factory = new GsubProcessor.Factory();
+        Map<String, Object> config = new HashMap<>();
+        config.put("field", "field1");
+        config.put("pattern", "[");
+        config.put("replacement", "-");
+        try {
+            factory.create(config);
+            fail("factory create should have failed");
+        } catch(ElasticsearchParseException e) {
+            assertThat(e.getMessage(), equalTo("[pattern] Invalid regex pattern. Unclosed character class near index 0\n[\n^"));
+        }
+    }
 }
diff --git a/modules/ingest-grok/src/main/java/org/elasticsearch/ingest/grok/GrokProcessor.java b/modules/ingest-grok/src/main/java/org/elasticsearch/ingest/grok/GrokProcessor.java
@@ -27,6 +27,8 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.elasticsearch.ingest.core.ConfigurationUtils.newConfigurationException;
+
 public final class GrokProcessor extends AbstractProcessor {
 
     public static final String TYPE = "grok";
@@ -82,7 +84,12 @@ public GrokProcessor doCreate(String processorTag, Map<String, Object> config) t
                 patternBank.putAll(customPatternBank);
             }
 
-            Grok grok = new Grok(patternBank, matchPattern);
+            Grok grok;
+            try {
+                grok = new Grok(patternBank, matchPattern);
+            } catch (Exception e) {
+                throw newConfigurationException(TYPE, processorTag, "pattern", "Invalid regex pattern. " + e.getMessage());
+            }
             return new GrokProcessor(processorTag, grok, matchField);
         }
 

diff --git a/...es/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java b/...es/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java
@@ -84,4 +84,33 @@ public void testCreateWithCustomPatterns() throws Exception {
         assertThat(processor.getGrok(), notNullValue());
         assertThat(processor.getGrok().match("foo!"), equalTo(true));
     }
+
+    public void testCreateWithInvalidPattern() throws Exception {
+        GrokProcessor.Factory factory = new GrokProcessor.Factory(Collections.emptyMap());
+        Map<String, Object> config = new HashMap<>();
+        config.put("field", "_field");
+        config.put("pattern", "[");
+        try {
+            factory.create(config);
+            fail("should fail");
+        } catch (ElasticsearchParseException e) {
+            assertThat(e.getMessage(), equalTo("[pattern] Invalid regex pattern. premature end of char-class"));
+        }
+
+    }
+
+    public void testCreateWithInvalidPatternDefinition() throws Exception {
+        GrokProcessor.Factory factory = new GrokProcessor.Factory(Collections.emptyMap());
+        Map<String, Object> config = new HashMap<>();
+        config.put("field", "_field");
+        config.put("pattern", "%{MY_PATTERN:name}!");
+        config.put("pattern_definitions", Collections.singletonMap("MY_PATTERN", "["));
+        try {
+            factory.create(config);
+            fail("should fail");
+        } catch (ElasticsearchParseException e) {
+            assertThat(e.getMessage(), equalTo("[pattern] Invalid regex pattern. premature end of char-class"));
+        }
+
+    }
 }