diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/security/PutRoleMappingResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/security/PutRoleMappingResponse.java index 04cdb14163e3e..00039f1486e1f 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/security/PutRoleMappingResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/security/PutRoleMappingResponse.java @@ -64,11 +64,10 @@ public int hashCode() { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "put_role_mapping_response", true, args -> new PutRoleMappingResponse((boolean) args[0])); static { - PARSER.declareBoolean(constructorArg(), new ParseField("created")); - // To parse the "created" field we declare "role_mapping" field object. - // Once the nested field "created" is found parser constructs the target object and - // ignores the role_mapping object. - PARSER.declareObject((a,b) -> {}, (parser, context) -> null, new ParseField("role_mapping")); + ConstructingObjectParser roleMappingParser = new ConstructingObjectParser<>( + "put_role_mapping_response.role_mapping", true, args -> (Boolean) args[0]); + roleMappingParser.declareBoolean(constructorArg(), new ParseField("created")); + PARSER.declareObject(constructorArg(), roleMappingParser::parse, new ParseField("role_mapping")); } public static PutRoleMappingResponse fromXContent(XContentParser parser) throws IOException { diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index d0cc929b56d24..219c3c5bbbae4 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -324,9 +324,31 @@ private void parseSub(XContentParser parser, FieldParser fieldParser, String cur switch (token) { case START_OBJECT: parseValue(parser, fieldParser, currentFieldName, value, context); + /* + * Well behaving parsers should consume the entire object but + * asserting that they do that is not something we can do + * efficiently here. Instead we can check that they end on an + * END_OBJECT. They could end on the *wrong* end object and + * this test won't catch them, but that is the price that we pay + * for having a cheap test. + */ + if (parser.currentToken() != XContentParser.Token.END_OBJECT) { + throw new IllegalStateException("parser for [" + currentFieldName + "] did not end on END_OBJECT"); + } break; case START_ARRAY: parseArray(parser, fieldParser, currentFieldName, value, context); + /* + * Well behaving parsers should consume the entire array but + * asserting that they do that is not something we can do + * efficiently here. Instead we can check that they end on an + * END_ARRAY. They could end on the *wrong* end array and + * this test won't catch them, but that is the price that we pay + * for having a cheap test. + */ + if (parser.currentToken() != XContentParser.Token.END_ARRAY) { + throw new IllegalStateException("parser for [" + currentFieldName + "] did not end on END_ARRAY"); + } break; case END_OBJECT: case END_ARRAY: diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index 42d53bf49859b..889f1619614aa 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -34,6 +34,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasSize; @@ -650,6 +651,49 @@ public void setArray(List testArray) { assertThat(ex.getMessage(), containsString("[foo] failed to parse field [int_array]")); } + public void testNoopDeclareObject() throws IOException { + ObjectParser, Void> parser = new ObjectParser<>("noopy", AtomicReference::new); + parser.declareString(AtomicReference::set, new ParseField("body")); + parser.declareObject((a,b) -> {}, (p, c) -> null, new ParseField("noop")); + + assertEquals("i", parser.parse(createParser(JsonXContent.jsonXContent, "{\"body\": \"i\"}"), null).get()); + Exception garbageException = expectThrows(IllegalStateException.class, () -> parser.parse( + createParser(JsonXContent.jsonXContent, "{\"noop\": {\"garbage\": \"shouldn't\"}}"), + null)); + assertEquals("parser for [noop] did not end on END_OBJECT", garbageException.getMessage()); + Exception sneakyException = expectThrows(IllegalStateException.class, () -> parser.parse( + createParser(JsonXContent.jsonXContent, "{\"noop\": {\"body\": \"shouldn't\"}}"), + null)); + assertEquals("parser for [noop] did not end on END_OBJECT", sneakyException.getMessage()); + } + + public void testNoopDeclareField() throws IOException { + ObjectParser, Void> parser = new ObjectParser<>("noopy", AtomicReference::new); + parser.declareString(AtomicReference::set, new ParseField("body")); + parser.declareField((a,b) -> {}, (p, c) -> null, new ParseField("noop"), ValueType.STRING_ARRAY); + + assertEquals("i", parser.parse(createParser(JsonXContent.jsonXContent, "{\"body\": \"i\"}"), null).get()); + Exception e = expectThrows(IllegalStateException.class, () -> parser.parse( + createParser(JsonXContent.jsonXContent, "{\"noop\": [\"ignored\"]}"), + null)); + assertEquals("parser for [noop] did not end on END_ARRAY", e.getMessage()); + } + + public void testNoopDeclareObjectArray() throws IOException { + ObjectParser, Void> parser = new ObjectParser<>("noopy", AtomicReference::new); + parser.declareString(AtomicReference::set, new ParseField("body")); + parser.declareObjectArray((a,b) -> {}, (p, c) -> null, new ParseField("noop")); + + XContentParseException garbageError = expectThrows(XContentParseException.class, () -> parser.parse( + createParser(JsonXContent.jsonXContent, "{\"noop\": [{\"garbage\": \"shouldn't\"}}]"), + null)); + assertEquals("expected value but got [FIELD_NAME]", garbageError.getCause().getMessage()); + XContentParseException sneakyError = expectThrows(XContentParseException.class, () -> parser.parse( + createParser(JsonXContent.jsonXContent, "{\"noop\": [{\"body\": \"shouldn't\"}}]"), + null)); + assertEquals("expected value but got [FIELD_NAME]", sneakyError.getCause().getMessage()); + } + static class NamedObjectHolder { public static final ObjectParser PARSER = new ObjectParser<>("named_object_holder", NamedObjectHolder::new);