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 bad HTTP requests #23153

Merged
merged 11 commits into from
Feb 13, 2017
Merged

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Feb 13, 2017

When Netty decodes a bad HTTP request, it marks the decoder result on the HTTP request as a failure, and reroutes the request to GET /bad-request. This either leads to puzzling responses when a bad request is sent to Elasticsearch (if an index named "bad-request" does not exist then it produces an index not found exception and otherwise responds with the index settings for the index named "bad-request"). This commit addresses this by inspecting the decoder result on the HTTP request and dispatching the request to a bad request handler preserving the initial cause of the bad request and providing an error message to the client.

Closes #23034

When Netty decodes a bad HTTP request, it marks the decoder result on
the HTTP request as a failure, and reroutes the request to GET
/bad-request. This either leads to puzzling responses when a bad request
is sent to Elasticsearch (if an index named "bad-request" does not exist
then it produces an index not found exception and otherwise responds
with the index settings for the index named "bad-request"). This commit
addresses this by inspecting the decoder result on the HTTP request and
dispatching the request to a bad request handler preserving the initial
cause of the bad request and providing an error message to the client.
When dispatching a bad request, if sending the response to the client
fails, we should log the bad request cause and the underlying cause of
the failure to send.
When a bad request is dispatched, in case the cause is unknown we should
indicate such.
Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext);

/**
* Dispatches the {@link RestRequest} to the bad request handler.
Copy link
Member

Choose a reason for hiding this comment

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

nit: can you make "bad request" more concrete/descriptive or give a example

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed c617b7e.

* master:
  Update to forbiddenapis 2.3 (improves Gradle configuration time) (elastic#23154)
  Make the version of the remote node accessible on a transport channel (elastic#23019)
  Fix total disk bytes returning negative value (elastic#23093)
  Fix communication with 5.3.0 nodes
  Update redirects.asciidoc (elastic#23148)
@jasontedor jasontedor merged commit 5343b87 into elastic:master Feb 13, 2017
jasontedor added a commit that referenced this pull request Feb 14, 2017
When Netty decodes a bad HTTP request, it marks the decoder result on
the HTTP request as a failure, and reroutes the request to GET
/bad-request. This either leads to puzzling responses when a bad request
is sent to Elasticsearch (if an index named "bad-request" does not exist
then it produces an index not found exception and otherwise responds
with the index settings for the index named "bad-request"). This commit
addresses this by inspecting the decoder result on the HTTP request and
dispatching the request to a bad request handler preserving the initial
cause of the bad request and providing an error message to the client.

Relates #23153
jasontedor added a commit that referenced this pull request Feb 14, 2017
When Netty decodes a bad HTTP request, it marks the decoder result on
the HTTP request as a failure, and reroutes the request to GET
/bad-request. This either leads to puzzling responses when a bad request
is sent to Elasticsearch (if an index named "bad-request" does not exist
then it produces an index not found exception and otherwise responds
with the index settings for the index named "bad-request"). This commit
addresses this by inspecting the decoder result on the HTTP request and
dispatching the request to a bad request handler preserving the initial
cause of the bad request and providing an error message to the client.

Relates #23153
@jasontedor jasontedor deleted the handle-bad-requests branch February 14, 2017 00:21
@jasontedor
Copy link
Member Author

Thanks @jaymode.

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.

None yet

2 participants