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

Invalid JSON request bodies result in 500 response #49428

Closed
pugnascotia opened this issue Nov 21, 2019 · 3 comments · Fixed by #49552
Closed

Invalid JSON request bodies result in 500 response #49428

pugnascotia opened this issue Nov 21, 2019 · 3 comments · Fixed by #49552
Assignees
Labels
>bug :Core/Infra/REST API REST infrastructure and utilities v7.3.1

Comments

@pugnascotia
Copy link
Contributor

pugnascotia commented Nov 21, 2019

Elasticsearch version: v7.3.1
OS version: Linux 4.4.0-1098-aws
JVM: OpenJDK 1.8.0_201 / 25.201-b09

Current behaviour:

If I execute a search with an invalid JSON request body, e.g. missing a double-quote:

GET blogs/_search
{"query":{"match":{"title":{"query":"open source software,"minimum_should_match":2}}}}

Then ES rejects the query with a 500, which indicates a server-side problem.

Expected behaviour:

The server should respond with "400 Bad Request", to indicate a client problem.

Impact:

The 500 is problematic because it indicates to clients that they can retry the request, but of course it will always fail. See also #48308, which concerns the Bulk API.

@pugnascotia pugnascotia added >bug :Core/Infra/REST API REST infrastructure and utilities v7.3.1 labels Nov 21, 2019
@elasticmachine
Copy link
Collaborator

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

@pugnascotia pugnascotia self-assigned this Nov 21, 2019
pugnascotia added a commit that referenced this issue Nov 25, 2019
Closes #49428. The code that works out an HTTP code for an exception didn't
consider the JsonParseException case, meant that an invalid JSON request could
result in a 500 Internal Server Error. Now it returns 400 Bad Request.
pugnascotia added a commit to pugnascotia/elasticsearch that referenced this issue Nov 25, 2019
Closes elastic#49428. The code that works out an HTTP code for an exception didn't
consider the JsonParseException case, meant that an invalid JSON request could
result in a 500 Internal Server Error. Now it returns 400 Bad Request.
@Mpdreamz
Copy link
Member

Clients don't retry 500

See: https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/fail-over.html

And

private static List<Integer> ERROR_NO_RETRY_STATUS_CODES = Arrays.asList(400, 401, 403, 404, 405, 500);

Great fix though thanks @pugnascotia !

@jasontedor
Copy link
Member

Some clients do, for example, Beats: https://github.com/elastic/beats/blob/ee5a4e73a98fbf63307cd5d7f6cfcf196bb15bd3/libbeat/outputs/elasticsearch/client.go#L522-L524

pugnascotia added a commit that referenced this issue Nov 26, 2019
Backport of #49552.

Closes #49428. The code that works out an HTTP code for an exception didn't
consider the JsonParseException case, meant that an invalid JSON request could
result in a 500 Internal Server Error. Now it returns 400 Bad Request.
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.3.1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants