Throw exception when content _id is an object #3586
Conversation
An exception is thrown if the provided id does not match the
content id, but only if the content id is a string field. If
the content id is a complex object, no exception is thrown but
the document is indexed anyway, leading to problems with search
later.
This fix adds an additional check for _id fields that are objects
and throws an exception if one is encountered
Fixes elastic#3517
| @@ -307,6 +307,8 @@ protected Field parseCreateField(ParseContext context) throws IOException { | |||
| return null; | |||
| } | |||
| return new Field(names.indexName(), context.id(), fieldType); | |||
| } else if (parser.currentName() != null && parser.currentName().equals(Defaults.NAME) && parser.currentToken().equals(XContentParser.Token.START_OBJECT)) { | |||
There was a problem hiding this comment.
you could remove the null check by changing the second check to Defaults.NAME.equals(parser.currentName())
There was a problem hiding this comment.
I think the first check should be:
if (parser.currentName() != null && parser.currentName().equals(Defaults.NAME)) {
if (parser.currentToken().isValue() == false) {
throw new MapperParsingException("Expected a value as Content id but got " + parser.currentToken());
}
/....
}that way we also fail for arrays etc.
|
is there any specific reason (except breaking current behaviour), why this is hanging around? |
|
@polyfractal ping? |
|
@spinscale No particular reason, just got lost in the shuffle I think. For the record: I have no idea if this PR is a good idea, and if this is the best way to do it. Was a quick one-line PR that I sent before I really had a good handle on the ES codebase. |
|
I left a comment, can reabase it and update ? I think it's a good change |
|
@polyfractal reassigning this to you. |
|
Closing in favour of #6730 |
An exception is thrown if the provided id does not match the content id, but only if the content id is a string field. If the content id is a complex object, no exception is thrown and the document is indexed anyway, leading to problems with search later.
This fix adds an additional check for _id fields that are objects and throws an exception if one is encountered.
Fixes #3517