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

Check if from + size don't cause overflow and fail with a better error #7778

Merged
merged 1 commit into from Sep 20, 2014

Conversation

Projects
None yet
3 participants
@martijnvg
Copy link
Member

commented Sep 18, 2014

Closes #7774

@s1monw

View changes

src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java Outdated
// from and size have been set.
int numHits = from() + size();
if (numHits < 0) {
throw new QueryPhaseExecutionException(this, "From and size combined can't be negative");

This comment has been minimized.

Copy link
@s1monw

s1monw Sep 18, 2014

Contributor

I think we should return a readable / understandable msg here something like:

QueryPhaseExecutionException("Result window is too large, from+size must be less than or equal to: [" + Integer.MAX_VALUE + "] but was [" + (((long)from()) + ((long)size()))) + "]";

This comment has been minimized.

Copy link
@martijnvg

martijnvg Sep 18, 2014

Author Member

I agree that is a better message, I'll update.

On 18 September 2014 16:05, Simon Willnauer notifications@github.com
wrote:

In
src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java:

@@ -213,6 +214,14 @@ public void doClose() throws ElasticsearchException {
* Should be called before executing the main query and after all other parameters have been set.
*/
public void preProcess() {

  •    if (!(from() == -1 && size() == -1)) {
    
  •        // from and size have been set.
    
  •        int numHits = from() + size();
    
  •        if (numHits < 0) {
    
  •            throw new QueryPhaseExecutionException(this, "From and size combined can't be negative");
    

I think we should return a readable / understandable msg here something
like:

QueryPhaseExecutionException("Result window is too large, from+size must be less than or equal to: [" + Integer.MAX_VALUE + "] but was [" + (((long)from()) + ((long)size()))) + "]";


Reply to this email directly or view it on GitHub
https://github.com/elasticsearch/elasticsearch/pull/7778/files#r17728419
.

Met vriendelijke groet,

Martijn van Groningen

@s1monw s1monw removed the review label Sep 19, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2014

@martijnvg wanna get this in?

@martijnvg martijnvg added the v1.5.0 label Sep 20, 2014

@martijnvg martijnvg force-pushed the martijnvg:bugs/from_size_overflow branch Sep 20, 2014

@martijnvg martijnvg force-pushed the martijnvg:bugs/from_size_overflow branch to afcbffb Sep 20, 2014

@martijnvg martijnvg merged commit afcbffb into elastic:master Sep 20, 2014

martijnvg added a commit that referenced this pull request Sep 20, 2014

martijnvg added a commit that referenced this pull request Sep 20, 2014

martijnvg added a commit that referenced this pull request Sep 20, 2014

@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2014

@s1monw done :)

@clintongormley clintongormley changed the title Core: Check if from + size don't cause overflow and fail with a better error Internal: Check if from + size don't cause overflow and fail with a better error Sep 26, 2014

@martijnvg martijnvg deleted the martijnvg:bugs/from_size_overflow branch May 18, 2015

@clintongormley clintongormley changed the title Internal: Check if from + size don't cause overflow and fail with a better error Check if from + size don't cause overflow and fail with a better error Jun 7, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.