Watcher: Be strict with chain input parsing (elastic/elasticsearch#3873)
When parsing chain inputs there were possibilities to write invalid JSON that resulting in losing the order of the inputs without any exception being thrown. This commit makes the parsing more strict. Closes elastic/elasticsearch#3736 Original commit: elastic/x-pack-elasticsearch@963641ee2b
This commit is contained in:
parent
95e1f2942b
commit
fe93640e43
|
@ -5,6 +5,7 @@
|
|||
*/
|
||||
package org.elasticsearch.xpack.watcher.input.chain;
|
||||
|
||||
import org.elasticsearch.ElasticsearchParseException;
|
||||
import org.elasticsearch.common.ParseField;
|
||||
import org.elasticsearch.common.collect.Tuple;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
|
@ -21,7 +22,9 @@ import java.util.List;
|
|||
public class ChainInput implements Input {
|
||||
|
||||
public static final String TYPE = "chain";
|
||||
private List<Tuple<String, Input>> inputs;
|
||||
public static final ParseField INPUTS = new ParseField("inputs");
|
||||
|
||||
private final List<Tuple<String, Input>> inputs;
|
||||
|
||||
public ChainInput(List<Tuple<String, Input>> inputs) {
|
||||
this.inputs = inputs;
|
||||
|
@ -35,7 +38,7 @@ public class ChainInput implements Input {
|
|||
@Override
|
||||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
|
||||
builder.startObject();
|
||||
builder.startArray("inputs");
|
||||
builder.startArray(INPUTS.getPreferredName());
|
||||
for (Tuple<String, Input> tuple : inputs) {
|
||||
builder.startObject().startObject(tuple.v1());
|
||||
builder.field(tuple.v2().type(), tuple.v2());
|
||||
|
@ -56,20 +59,15 @@ public class ChainInput implements Input {
|
|||
String currentFieldName;
|
||||
XContentParser.Token token;
|
||||
|
||||
ParseField inputsField = new ParseField("inputs");
|
||||
|
||||
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
||||
if (token == XContentParser.Token.FIELD_NAME) {
|
||||
currentFieldName = parser.currentName();
|
||||
token = parser.nextToken();
|
||||
if (token == XContentParser.Token.START_ARRAY && inputsField.getPreferredName().equals(currentFieldName)) {
|
||||
String currentInputFieldName = null;
|
||||
if (token == XContentParser.Token.START_ARRAY && INPUTS.getPreferredName().equals(currentFieldName)) {
|
||||
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
|
||||
if (token == XContentParser.Token.FIELD_NAME) {
|
||||
currentInputFieldName = parser.currentName();
|
||||
} else if (currentInputFieldName != null && token == XContentParser.Token.START_OBJECT) {
|
||||
inputs.add(new Tuple<>(currentInputFieldName, inputRegistry.parse(watchId, parser).input()));
|
||||
currentInputFieldName = null;
|
||||
String inputName = parser.currentName();
|
||||
inputs.add(new Tuple<>(inputName, parseSingleInput(watchId, inputName, parser, inputRegistry)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -79,6 +77,23 @@ public class ChainInput implements Input {
|
|||
return new ChainInput(inputs);
|
||||
}
|
||||
|
||||
private static Input parseSingleInput(String watchId, String name, XContentParser parser,
|
||||
InputRegistry inputRegistry) throws IOException {
|
||||
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
|
||||
throw new ElasticsearchParseException("Expected starting JSON object after [{}] in watch [{}]", name, watchId);
|
||||
}
|
||||
|
||||
Input input = inputRegistry.parse(watchId, parser).input();
|
||||
|
||||
// expecting closing of two json object to start the next element in the array
|
||||
if (parser.currentToken() != XContentParser.Token.END_OBJECT || parser.nextToken() != XContentParser.Token.END_OBJECT) {
|
||||
throw new ElasticsearchParseException("Expected closing JSON object after parsing input [{}] named [{}] in watch [{}]",
|
||||
input.type(), name, watchId);
|
||||
}
|
||||
|
||||
return input;
|
||||
}
|
||||
|
||||
public static ChainInput.Builder builder() {
|
||||
return new Builder();
|
||||
}
|
||||
|
|
|
@ -6,6 +6,7 @@
|
|||
package org.elasticsearch.xpack.watcher.input.chain;
|
||||
|
||||
import org.elasticsearch.ElasticsearchException;
|
||||
import org.elasticsearch.ElasticsearchParseException;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.unit.TimeValue;
|
||||
import org.elasticsearch.common.xcontent.ToXContent;
|
||||
|
@ -101,8 +102,8 @@ public class ChainInputTests extends ESTestCase {
|
|||
assertThat(payload.data().get("second"), instanceOf(Map.class));
|
||||
|
||||
// final payload check
|
||||
Map<String, Object> firstPayload = (Map<String,Object>) payload.data().get("first");
|
||||
Map<String, Object> secondPayload = (Map<String,Object>) payload.data().get("second");
|
||||
Map<String, Object> firstPayload = (Map<String, Object>) payload.data().get("first");
|
||||
Map<String, Object> secondPayload = (Map<String, Object>) payload.data().get("second");
|
||||
assertThat(firstPayload, hasEntry("foo", "bar"));
|
||||
assertThat(secondPayload, hasEntry("spam", "eggs"));
|
||||
}
|
||||
|
@ -167,6 +168,67 @@ public class ChainInputTests extends ESTestCase {
|
|||
assertThat(builder.bytes().utf8ToString(), containsString("\"reason\":\"ElasticsearchException[foo]\""));
|
||||
}
|
||||
|
||||
/* https://github.com/elastic/x-plugins/issues/3736
|
||||
the issue here is, that first/second in this setup do not have a guaranteed order, so we have to throw an exception
|
||||
{
|
||||
"inputs" : [
|
||||
{
|
||||
"first" : { "simple" : { "foo" : "bar" } },
|
||||
"second" : { "simple" : { "spam" : "eggs" } }
|
||||
}
|
||||
]
|
||||
}
|
||||
*/
|
||||
public void testParsingShouldBeStrictWhenClosingInputs() throws Exception {
|
||||
Map<String, InputFactory> factories = new HashMap<>();
|
||||
factories.put("simple", new SimpleInputFactory(Settings.EMPTY));
|
||||
|
||||
InputRegistry inputRegistry = new InputRegistry(Settings.EMPTY, factories);
|
||||
ChainInputFactory chainInputFactory = new ChainInputFactory(Settings.EMPTY, inputRegistry);
|
||||
factories.put("chain", chainInputFactory);
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startArray("inputs").startObject()
|
||||
.startObject("first").startObject("simple").field("foo", "bar").endObject().endObject()
|
||||
.startObject("second").startObject("simple").field("spam", "eggs").endObject().endObject()
|
||||
.endObject().endArray().endObject();
|
||||
|
||||
XContentParser parser = XContentFactory.xContent(builder.bytes()).createParser(builder.bytes());
|
||||
parser.nextToken();
|
||||
ElasticsearchParseException e =
|
||||
expectThrows(ElasticsearchParseException.class, () -> chainInputFactory.parseInput("test", parser, false));
|
||||
assertThat(e.getMessage(),
|
||||
containsString("Expected closing JSON object after parsing input [simple] named [first] in watch [test]"));
|
||||
}
|
||||
|
||||
/* https://github.com/elastic/x-plugins/issues/3736
|
||||
make sure that after the name of a chained input there is always an object
|
||||
{
|
||||
"inputs" : [
|
||||
{ "first" : [ { "simple" : { "foo" : "bar" } } ] }
|
||||
]
|
||||
}
|
||||
*/
|
||||
public void testParsingShouldBeStrictWhenStartingInputs() throws Exception {
|
||||
Map<String, InputFactory> factories = new HashMap<>();
|
||||
factories.put("simple", new SimpleInputFactory(Settings.EMPTY));
|
||||
|
||||
InputRegistry inputRegistry = new InputRegistry(Settings.EMPTY, factories);
|
||||
ChainInputFactory chainInputFactory = new ChainInputFactory(Settings.EMPTY, inputRegistry);
|
||||
factories.put("chain", chainInputFactory);
|
||||
|
||||
XContentBuilder builder = jsonBuilder().startObject().startArray("inputs")
|
||||
.startObject().startArray("first").startObject()
|
||||
.startObject("simple").field("foo", "bar").endObject()
|
||||
.endObject().endArray().endObject()
|
||||
.endArray().endObject();
|
||||
|
||||
XContentParser parser = XContentFactory.xContent(builder.bytes()).createParser(builder.bytes());
|
||||
parser.nextToken();
|
||||
ElasticsearchParseException e =
|
||||
expectThrows(ElasticsearchParseException.class, () -> chainInputFactory.parseInput("test", parser, false));
|
||||
assertThat(e.getMessage(), containsString("Expected starting JSON object after [first] in watch [test]"));
|
||||
}
|
||||
|
||||
private WatchExecutionContext createContext() {
|
||||
Watch watch = new Watch("test-watch",
|
||||
new ScheduleTrigger(new IntervalSchedule(new IntervalSchedule.Interval(1, IntervalSchedule.Interval.Unit.MINUTES))),
|
||||
|
|
Loading…
Reference in New Issue