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

Handle incorrect header values #64708

Merged

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Nov 6, 2020

When Accept or Content-Type header values are incorrect, a request
should be gracefully rejected and an exception message returned to a
client.

relates #64689
relates #51816

When Accept or Content-Type header values are incorrect, a request
should be gracefully rejected and an exception message returned to a
client.

relates elastic#64689
@pgomulka pgomulka self-assigned this Nov 6, 2020
@pgomulka pgomulka added :Core/Infra/REST API REST infrastructure and utilities v8.0.0 labels Nov 6, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/REST API)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Nov 6, 2020

@Override
public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
assertThat(cause, instanceOf(RestRequest.MediaTypeHeaderException.class));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anywhere that we can assert that the header name at fault is included in the error sent back to the user ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great you noticed this. in fact the header name was not surfaced on a response.
I refactored this to return something like

{
  "error": {
    "root_cause": [
      {
        "type": "media_type_header_exception",
        "reason": "Invalid media-type value on header [Accept]"
      }
    ],
    "type": "media_type_header_exception",
    "reason": "Invalid media-type value on header [Accept]",
    "caused_by": {
      "type": "illegal_argument_exception",
      "reason": "invalid media type [texffff]"
    }
  },
  "status": 400
}

Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM

@pgomulka pgomulka merged commit 0bb64cb into elastic:master Nov 10, 2020
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 Team:Core/Infra Meta label for core/infra team v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants