From 534caa89275e4bb16ce35d1ed398cb9db83041f4 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 22 Mar 2016 14:28:24 -0700 Subject: [PATCH] Handle regex parsing errors in Gsub and Grok Processors Currently, both Gsub and Grok parse regex strings during Pipeline creation. Thrown parsing exceptions were leaking out, this commit wraps those exceptions in ElasticsearchParseExceptions. --- .../ingest/processor/GsubProcessor.java | 17 ++++++++--- .../processor/GsubProcessorFactoryTests.java | 14 +++++++++ .../ingest/grok/GrokProcessor.java | 9 +++++- .../grok/GrokProcessorFactoryTests.java | 29 +++++++++++++++++++ 4 files changed, 64 insertions(+), 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 index 6e73c92070b..d986bf522e5 100644 --- 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.Map; 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 final class GsubProcessor extends AbstractProcessor { public static final class Factory extends AbstractProcessorFactory { @Override public GsubProcessor doCreate(String processorTag, Map 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 index 2440ff68408..628a81223be 100644 --- 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 class GsubProcessorFactoryTests extends ESTestCase { assertThat(e.getMessage(), equalTo("[replacement] required property is missing")); } } + + public void testCreateInvalidPattern() throws Exception { + GsubProcessor.Factory factory = new GsubProcessor.Factory(); + Map 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 index b4755d61c56..9237821baba 100644 --- 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 org.elasticsearch.ingest.core.IngestDocument; 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 final class GrokProcessor extends AbstractProcessor { 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/modules/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java b/modules/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java index db98090af39..3880d389c52 100644 --- a/modules/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java +++ b/modules/ingest-grok/src/test/java/org/elasticsearch/ingest/grok/GrokProcessorFactoryTests.java @@ -84,4 +84,33 @@ public class GrokProcessorFactoryTests extends ESTestCase { 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 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 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")); + } + + } }