Skip to content

Commit

Permalink
RestRequest parameter parsing should not cause 500 error
Browse files Browse the repository at this point in the history
Currently, RestRequest throws IllegalStateExceptions in a few places which leads
to 500 errors on the client side and e.g. turn up as warnings in the
"rest.suppressed" error log. By changing them to IllegalArgumentExceptions we
can change these to 400 (Bad Request) error codes.

Closes elastic#112604
  • Loading branch information
cbuescher committed Sep 6, 2024
1 parent 000ebaf commit d6800d9
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
6 changes: 3 additions & 3 deletions server/src/main/java/org/elasticsearch/rest/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public final BytesReference requiredContent() {
if (hasContent() == false) {
throw new ElasticsearchParseException("request body is required");
} else if (xContentType.get() == null) {
throw new IllegalStateException("unknown content type");
throw new IllegalArgumentException("unknown content type");
}
return content();
}
Expand Down Expand Up @@ -561,12 +561,12 @@ public final Tuple<XContentType, BytesReference> contentOrSourceParam() {
String source = param("source");
String typeParam = param("source_content_type");
if (source == null || typeParam == null) {
throw new IllegalStateException("source and source_content_type parameters are required");
throw new IllegalArgumentException("source and source_content_type parameters are required");
}
BytesArray bytes = new BytesArray(source);
final XContentType xContentType = parseContentType(Collections.singletonList(typeParam));
if (xContentType == null) {
throw new IllegalStateException("Unknown value for source_content_type [" + typeParam + "]");
throw new IllegalArgumentException("Unknown value for source_content_type [" + typeParam + "]");
}
return new Tuple<>(xContentType, bytes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void testContentOrSourceParam() throws IOException {
.contentOrSourceParam()
.v2()
);
e = expectThrows(IllegalStateException.class, () -> contentRestRequest("", Map.of("source", "stuff2")).contentOrSourceParam());
e = expectThrows(IllegalArgumentException.class, () -> contentRestRequest("", Map.of("source", "stuff2")).contentOrSourceParam());
assertEquals("source and source_content_type parameters are required", e.getMessage());
}

Expand Down Expand Up @@ -245,7 +245,7 @@ public void testRequiredContent() {
.requiredContent()
);
assertEquals("request body is required", e.getMessage());
e = expectThrows(IllegalStateException.class, () -> contentRestRequest("test", null, Collections.emptyMap()).requiredContent());
e = expectThrows(IllegalArgumentException.class, () -> contentRestRequest("test", null, Collections.emptyMap()).requiredContent());
assertEquals("unknown content type", e.getMessage());
}

Expand Down

0 comments on commit d6800d9

Please sign in to comment.