From ba3037f5fe4228d22cbae9161857439e9188c91f Mon Sep 17 00:00:00 2001 From: Brian Murphy Date: Thu, 30 Apr 2015 12:23:03 -0400 Subject: [PATCH] When parsing a http request port is a required field. The parser for http request had a bug and was accepting requests that did not specify a port. This changes this. Fixes elastic/elasticsearch#299 Original commit: elastic/x-pack-elasticsearch@e0eafe37878282f446f37e5e59f660b3e80da256 --- .../watcher/support/http/HttpRequestTemplate.java | 2 +- .../watcher/actions/webhook/WebhookActionTests.java | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/elasticsearch/watcher/support/http/HttpRequestTemplate.java b/src/main/java/org/elasticsearch/watcher/support/http/HttpRequestTemplate.java index e476694ee64..069edf1e4c2 100644 --- a/src/main/java/org/elasticsearch/watcher/support/http/HttpRequestTemplate.java +++ b/src/main/java/org/elasticsearch/watcher/support/http/HttpRequestTemplate.java @@ -255,7 +255,7 @@ public class HttpRequestTemplate implements ToXContent { if (builder.host == null) { throw new ParseException("could not parse http request template. missing required [host] string field"); } - if (builder.port < 0) { + if (builder.port <= 0) { throw new ParseException("could not parse http request template. missing required [port] numeric field"); } diff --git a/src/test/java/org/elasticsearch/watcher/actions/webhook/WebhookActionTests.java b/src/test/java/org/elasticsearch/watcher/actions/webhook/WebhookActionTests.java index e7c8ae4a57e..43ef68d40d7 100644 --- a/src/test/java/org/elasticsearch/watcher/actions/webhook/WebhookActionTests.java +++ b/src/test/java/org/elasticsearch/watcher/actions/webhook/WebhookActionTests.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.watcher.actions.ActionException; import org.elasticsearch.watcher.actions.email.service.*; import org.elasticsearch.watcher.execution.TriggeredExecutionContext; import org.elasticsearch.watcher.execution.WatchExecutionContext; @@ -206,16 +205,24 @@ public class WebhookActionTests extends ElasticsearchTestCase { assertThat(parsedAction.action(), is(action)); } - @Test(expected = ActionException.class) + @Test(expected = WebhookActionException.class) + @Repeat(iterations = 5) public void testParser_Failure() throws Exception { + XContentBuilder builder = jsonBuilder().startObject(); + if (randomBoolean()) { + builder.field(HttpRequestTemplate.Parser.HOST_FIELD.getPreferredName(), TEST_HOST); + } else { + builder.field(HttpRequestTemplate.Parser.PORT_FIELD.getPreferredName(), TEST_PORT); + } + builder.endObject(); - XContentBuilder builder = jsonBuilder().startObject().endObject(); XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); parser.nextToken(); WebhookActionFactory actionParser = getParser(ExecuteScenario.Success.client()); //This should fail since we are not supplying a url actionParser.parseExecutable("_watch", randomAsciiOfLength(5), parser); + fail("expected a WebhookActionException since we only provided either a host or a port but not both"); } @Test @Repeat(iterations = 30)