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

Reject all requests that have an unconsumed body #37504

Merged

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Jan 15, 2019

This commit removes some leniency from REST handling where we move to reject all requests that have a body where the body is not used during the course of handling the request. For example,

DELETE /index
{
  "query" : {
    "term" :  {
      "field" : "value"
    }
  }
}

is now rejected.

Closes #8217
Supersedes #30792
Supersedes #37501

This commit removes some leniency from REST handling where we move to
reject all requests that have a body where the body is not used during
the course of handling the request. For example,

DELETE /index
{ "query" : {
    "term" :  {
      "field" : "value"
    }
  }
}

is now rejected.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, nice way to protect APIs in the future from ignoring bodies 👍

@jasontedor jasontedor merged commit 687978b into elastic:master Jan 16, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 16, 2019
This commit removes some leniency from REST handling where we move to
reject all requests that have a body where the body is not used during
the course of handling the request. For example,

DELETE /index
{
  "query" : {
    "term" :  {
      "field" : "value"
    }
  }
}

is now rejected.
@jasontedor jasontedor deleted the reject-requests-with-an-unconsumed-body branch January 16, 2019 13:19
@jasontedor jasontedor removed the v6.7.0 label Jan 16, 2019
cshtdd added a commit to cshtdd/elasticsearch that referenced this pull request Jan 16, 2019
* master:
  Deprecate _type from LeafDocLookup (elastic#37491)
  Allow system privilege to execute proxied actions (elastic#37508)
  Update Put Watch to allow unknown fields (elastic#37494)
  AwaitsFix testAddNewReplicas
  SQL: Add protocol tests and remove jdbc_type from drivers response (elastic#37516)
  SQL: [Docs] Add an ES-SQL column for data types (elastic#37529)
  IndexMetaData#mappingOrDefault doesn't need to take a type argument. (elastic#37480)
  Simplify + Cleanup Dead Code in Settings (elastic#37341)
  Reject all requests that have an unconsumed body (elastic#37504)
  [Ml] Prevent config snapshot failure blocking migration (elastic#37493)
  Fix line length for aliases and remove suppression (elastic#37455)
  Add SSL Configuration Library (elastic#37287)
  SQL: Remove slightly used meta commands (elastic#37506)
  Simplify Snapshot Create Request Handling (elastic#37464)
  Remove the use of AbstracLifecycleComponent constructor elastic#37488 (elastic#37488)
  [ML] log minimum diskspace setting if forecast fails due to insufficient d… (elastic#37486)
pull bot pushed a commit to sadlil/elasticsearch that referenced this pull request Jul 28, 2019
The change elastic#37504 modifies the BaseRestHandler to make it reject all requests 
that have an unconsumed body. The notion of consumed or unconsumed body
 is carried by the RestRequest object and its contentConsumed attribute, which
 is set to true when the content() or content(true) methods are used.

In our REST layer, we usually expect the RestHandlers to consume the request 
content when needed, but it appears that the RestController always consumes
 the content upfront.

This commit changes the content() method used by the RestController so that it 
does not mark the content as consumed.
jkakavas pushed a commit that referenced this pull request Jul 31, 2019
The change #37504 modifies the BaseRestHandler to make it reject all requests 
that have an unconsumed body. The notion of consumed or unconsumed body
 is carried by the RestRequest object and its contentConsumed attribute, which
 is set to true when the content() or content(true) methods are used.

In our REST layer, we usually expect the RestHandlers to consume the request 
content when needed, but it appears that the RestController always consumes
 the content upfront.

This commit changes the content() method used by the RestController so that it 
does not mark the content as consumed.
jaymode added a commit that referenced this pull request Dec 14, 2020
The change #37504 modifies the BaseRestHandler to make it reject all requests
that have an unconsumed body. The notion of consumed or unconsumed body
 is carried by the RestRequest object and its contentConsumed attribute, which
 is set to true when the content() or content(true) methods are used.

In our REST layer, we usually expect the RestHandlers to consume the request
content when needed, but it appears that the RestController always consumes
 the content upfront.

This commit changes the content() method used by the RestController so that it
does not mark the content as consumed.

Backport of #44902
Closes #65242

Co-authored-by: Tanguy Leroux <tlrx.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/REST API REST infrastructure and utilities v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants