Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw exception if an additional field was placed inside the "query" body #4913

Closed
wants to merge 2 commits into from

Conversation

brwe
Copy link
Contributor

@brwe brwe commented Jan 27, 2014

Currently the parser accepts queries like

"query" : {
     "any_query": {
         ...
     },
     "any_field_name":...
}

The "any_field_name" is silently ignored. However, this also causes the parser
not to move to the next closing bracket which in turn can lead to additional query
paremters being ignored such as "fields", "highlight",...
This was the case in issue #4895

closes issue #4895

@@ -574,6 +575,8 @@ private void parseSource(SearchContext context, BytesReference source) throws Se
element.parse(parser, context);
} else if (token == null) {
break;
} else if ((token != XContentParser.Token.START_OBJECT)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so in theory there can only be a single fieldname and from thereone we pass on to the actual query parser, right? so instead of else if we can just put an else in there and make sure we move the parser to the first fieldName as we expect it before we enter the loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I added a commit for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated again - even more strict!

…body

Currently the parser accepts queries like

```
"query" : {
     "any_query": {
         ...
     },
     "any_field_name":...
}
```

The "any_field_name" is silently ignored. However, this also causes the parser
not to move to the next closing bracket which in turn can lead to additional query
paremters being ignored such as "fields", "highlight",...
This was the case in issue elastic#4895

closes issue elastic#4895
@brwe brwe added the review label Apr 22, 2014
@s1monw
Copy link
Contributor

s1monw commented Apr 23, 2014

I think we should push this - it's a good fix

@brwe brwe added v1.1.2 and removed review labels Apr 25, 2014
@brwe
Copy link
Contributor Author

brwe commented Apr 25, 2014

Pushed to master and 1.x.

@brwe brwe closed this Apr 25, 2014
@clintongormley clintongormley changed the title Throw exception if an additional field was placed inside the "query" bod... Throw exception if an additional field was placed inside the "query" body Jun 7, 2015
@clintongormley clintongormley added the :Core/Infra/REST API REST infrastructure and utilities label Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants