Skip to content

Commit a587c7d

Browse files
authored
RestRequest parameter parsing should not cause 500 error (#112605)
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 #112604
1 parent fc2760c commit a587c7d

File tree

3 files changed

+17
-8
lines changed

3 files changed

+17
-8
lines changed

server/src/main/java/org/elasticsearch/rest/RestRequest.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.ElasticsearchParseException;
1313
import org.elasticsearch.ElasticsearchStatusException;
1414
import org.elasticsearch.common.Strings;
15+
import org.elasticsearch.common.ValidationException;
1516
import org.elasticsearch.common.bytes.BytesArray;
1617
import org.elasticsearch.common.bytes.BytesReference;
1718
import org.elasticsearch.common.unit.ByteSizeValue;
@@ -321,11 +322,17 @@ public final BytesReference requiredContent() {
321322
if (hasContent() == false) {
322323
throw new ElasticsearchParseException("request body is required");
323324
} else if (xContentType.get() == null) {
324-
throw new IllegalStateException("unknown content type");
325+
throwValidationException("unknown content type");
325326
}
326327
return content();
327328
}
328329

330+
private static void throwValidationException(String msg) {
331+
ValidationException unknownContentType = new ValidationException();
332+
unknownContentType.addValidationError(msg);
333+
throw unknownContentType;
334+
}
335+
329336
/**
330337
* Get the value of the header or {@code null} if not found. This method only retrieves the first header value if multiple values are
331338
* sent. Use of {@link #getAllHeaderValues(String)} should be preferred
@@ -585,12 +592,12 @@ public final Tuple<XContentType, BytesReference> contentOrSourceParam() {
585592
String source = param("source");
586593
String typeParam = param("source_content_type");
587594
if (source == null || typeParam == null) {
588-
throw new IllegalStateException("source and source_content_type parameters are required");
595+
throwValidationException("source and source_content_type parameters are required");
589596
}
590597
BytesArray bytes = new BytesArray(source);
591598
final XContentType xContentType = parseContentType(Collections.singletonList(typeParam));
592599
if (xContentType == null) {
593-
throw new IllegalStateException("Unknown value for source_content_type [" + typeParam + "]");
600+
throwValidationException("Unknown value for source_content_type [" + typeParam + "]");
594601
}
595602
return new Tuple<>(xContentType, bytes);
596603
}

server/src/test/java/org/elasticsearch/rest/RestRequestTests.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.elasticsearch.rest;
1010

1111
import org.elasticsearch.ElasticsearchParseException;
12+
import org.elasticsearch.common.ValidationException;
1213
import org.elasticsearch.common.bytes.BytesArray;
1314
import org.elasticsearch.common.bytes.BytesReference;
1415
import org.elasticsearch.core.CheckedConsumer;
@@ -33,6 +34,7 @@
3334
import static java.util.Collections.singletonMap;
3435
import static org.elasticsearch.rest.RestRequest.OPERATOR_REQUEST;
3536
import static org.elasticsearch.rest.RestRequest.SERVERLESS_REQUEST;
37+
import static org.hamcrest.Matchers.containsString;
3638
import static org.hamcrest.Matchers.equalTo;
3739
import static org.hamcrest.Matchers.instanceOf;
3840
import static org.hamcrest.Matchers.is;
@@ -130,8 +132,8 @@ public void testContentOrSourceParam() throws IOException {
130132
.contentOrSourceParam()
131133
.v2()
132134
);
133-
e = expectThrows(IllegalStateException.class, () -> contentRestRequest("", Map.of("source", "stuff2")).contentOrSourceParam());
134-
assertEquals("source and source_content_type parameters are required", e.getMessage());
135+
e = expectThrows(ValidationException.class, () -> contentRestRequest("", Map.of("source", "stuff2")).contentOrSourceParam());
136+
assertThat(e.getMessage(), containsString("source and source_content_type parameters are required"));
135137
}
136138

137139
public void testHasContentOrSourceParam() throws IOException {
@@ -246,8 +248,8 @@ public void testRequiredContent() {
246248
.requiredContent()
247249
);
248250
assertEquals("request body is required", e.getMessage());
249-
e = expectThrows(IllegalStateException.class, () -> contentRestRequest("test", null, Collections.emptyMap()).requiredContent());
250-
assertEquals("unknown content type", e.getMessage());
251+
e = expectThrows(ValidationException.class, () -> contentRestRequest("test", null, Collections.emptyMap()).requiredContent());
252+
assertThat(e.getMessage(), containsString("unknown content type"));
251253
}
252254

253255
public void testIsServerlessRequest() {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/RestRequestFilterTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public void testFilterUnknownContentTypeThrows() throws IOException {
115115
}
116116
RestRequestFilter filter = () -> Collections.singleton("root.second.third");
117117
RestRequest filtered = filter.getFilteredRequest(restRequest);
118-
IllegalStateException e = expectThrows(IllegalStateException.class, () -> filtered.content());
118+
Exception e = expectThrows(IllegalArgumentException.class, () -> filtered.content());
119119
assertThat(e.getMessage(), containsString("unknown content type"));
120120
}
121121

0 commit comments

Comments
 (0)