Clean up Watch parsing to disallow arrays.

This change cleans up the watch parsing which had allowed arrays in actions as a hold over from when actions was an array.

Fixes elastic/elasticsearch#367

Original commit: elastic/x-pack-elasticsearch@3f4fb82124
This commit is contained in:
Brian Murphy 2015-05-04 20:37:16 -04:00
parent 2ff8e2fb4e
commit b5f5862146
6 changed files with 51 additions and 18 deletions

View File

@ -22,16 +22,15 @@
"condition": {
"always": {}
},
"actions": [
{
"actions": {
"test_index": {
"index": {
"index": "test",
"doc_type": "test2"
}
}
} ]
}
}
- match: { _id: "my_watch" }
- do:

View File

@ -26,16 +26,15 @@
"condition": {
"always": {}
},
"actions": [
{
"actions": {
"test_index": {
"index": {
"index": "test",
"doc_type": "test2"
}
}
} ]
}
}
}
- match: { _id: "my_watch" }
- do:

View File

@ -26,8 +26,7 @@
"condition": {
"always": {}
},
"actions": [
{
"actions": {
"test_index": {
"index": {
"index": "test",
@ -35,8 +34,7 @@
}
}
}
]
}
}
- match: { _id: "my_watch" }
- match: { created: true }

View File

@ -26,14 +26,13 @@
"condition": {
"always": {}
},
"actions": [
{
"actions": {
"test_index": {
"index": {
"index": "test",
"doc_type": "test2"
}
}
} ]
}
}
- match: { _id: "my_watch" }

View File

@ -285,13 +285,14 @@ public class Watch implements TriggerEngine.Job, ToXContent {
TimeValue throttlePeriod = defaultThrottleTimePeriod;
String currentFieldName = null;
XContentParser.Token token = null;
XContentParser.Token token = parser.nextToken();
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (token == null ){
throw new WatcherException("could not parse watch [" + id + "]. null token");
} else if ((token.isValue() || token == XContentParser.Token.START_OBJECT || token == XContentParser.Token.START_ARRAY) && currentFieldName !=null ) {
} else if (token == null || currentFieldName == null) {
throw new WatcherException("could not parse watch [{}], unexpected token [{}]", id, token);
} else if ((token.isValue() || token == XContentParser.Token.START_OBJECT) && currentFieldName !=null ) {
assert token != XContentParser.Token.START_ARRAY;
if (TRIGGER_FIELD.match(currentFieldName)) {
trigger = triggerService.parseTrigger(id, parser);
} else if (INPUT_FIELD.match(currentFieldName)) {
@ -320,6 +321,8 @@ public class Watch implements TriggerEngine.Job, ToXContent {
} else {
throw new WatcherException("could not parse watch [{}]. unexpected field [{}]", id, currentFieldName);
}
} else if (currentFieldName != null) {
throw new WatcherException("could not parse watch [{}]. unexpected token [{}] in field [{}]", id, token, currentFieldName);
}
}
if (trigger == null) {

View File

@ -15,8 +15,10 @@ import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.watcher.WatcherException;
import org.elasticsearch.watcher.actions.ActionFactory;
import org.elasticsearch.watcher.actions.ActionRegistry;
import org.elasticsearch.watcher.actions.ActionWrapper;
@ -95,6 +97,7 @@ import java.util.Map;
import static org.elasticsearch.watcher.input.InputBuilders.searchInput;
import static org.elasticsearch.watcher.test.WatcherTestUtils.matchAllRequest;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;
public class WatchTests extends ElasticsearchTestCase {
@ -172,6 +175,38 @@ public class WatchTests extends ElasticsearchTestCase {
assertThat(parsedWatch.actions(), equalTo(actions));
}
@Test
public void testParser_BadActions() throws Exception {
ScheduleRegistry scheduleRegistry = registry(randomSchedule());
TriggerEngine triggerEngine = new ParseOnlyScheduleTriggerEngine(ImmutableSettings.EMPTY, scheduleRegistry);
TriggerService triggerService = new TriggerService(ImmutableSettings.EMPTY, ImmutableSet.of(triggerEngine));
SecretService secretService = new SecretService.PlainText();
ExecutableCondition condition = randomCondition();
ConditionRegistry conditionRegistry = registry(condition);
ExecutableInput input = randomInput();
InputRegistry inputRegistry = registry(input);
TransformRegistry transformRegistry = transformRegistry();
ExecutableActions actions = randomActions();
ActionRegistry actionRegistry = registry(actions, transformRegistry);
XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
jsonBuilder.startObject();
jsonBuilder.field("actions");
jsonBuilder.startArray();
jsonBuilder.endArray();
jsonBuilder.endObject();
Watch.Parser watchParser = new Watch.Parser(settings, mock(LicenseService.class), conditionRegistry, triggerService, transformRegistry, actionRegistry, inputRegistry, SystemClock.INSTANCE, secretService);
try {
watchParser.parse("failure", false, jsonBuilder.bytes());
fail("This watch should fail to parse as actions is an array");
} catch (WatcherException we) {
assertThat(we.getMessage().contains("could not parse watch [failure]. unexpected token"), is(true));
}
}
private static Schedule randomSchedule() {
String type = randomFrom(CronSchedule.TYPE, HourlySchedule.TYPE, DailySchedule.TYPE, WeeklySchedule.TYPE, MonthlySchedule.TYPE, YearlySchedule.TYPE, IntervalSchedule.TYPE);
switch (type) {