diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java index 196ea17b18c74..2323c582f829f 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ParsedMediaType.java @@ -76,7 +76,7 @@ public static ParsedMediaType parseMediaType(String headerValue) { final String[] splitMediaType = elements[0].split("/"); if ((splitMediaType.length == 2 && TCHAR_PATTERN.matcher(splitMediaType[0].trim()).matches() && TCHAR_PATTERN.matcher(splitMediaType[1].trim()).matches()) == false) { - throw new IllegalArgumentException("invalid media type [" + headerValue + "]"); + throw new IllegalArgumentException("invalid media-type [" + headerValue + "]"); } if (elements.length == 1) { return new ParsedMediaType(splitMediaType[0].trim(), splitMediaType[1].trim(), Collections.emptyMap()); diff --git a/modules/transport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4BadRequestIT.java b/modules/transport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4BadRequestIT.java index 6afee02d9ebb8..b8205a58870ed 100644 --- a/modules/transport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4BadRequestIT.java +++ b/modules/transport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4BadRequestIT.java @@ -99,6 +99,6 @@ public void testInvalidHeaderValue() throws IOException { final ObjectPath objectPath = ObjectPath.createFromResponse(response); final Map map = objectPath.evaluate("error"); assertThat(map.get("type"), equalTo("media_type_header_exception")); - assertThat(map.get("reason"), equalTo("java.lang.IllegalArgumentException: Header [Content-Type] cannot be empty.")); + assertThat(map.get("reason"), equalTo("Invalid media-type value on header [Content-Type]")); } } diff --git a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java index c033ac9bfc62c..bab94109ea7f3 100644 --- a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java +++ b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java @@ -347,7 +347,7 @@ private void handleIncomingRequest(final HttpRequest httpRequest, final HttpChan innerRestRequest = RestRequest.request(xContentRegistry, httpRequest, httpChannel); } catch (final RestRequest.MediaTypeHeaderException e) { badRequestCause = ExceptionsHelper.useOrSuppress(badRequestCause, e); - innerRestRequest = requestWithoutContentTypeHeader(httpRequest, httpChannel, badRequestCause); + innerRestRequest = requestWithoutFailedHeader(httpRequest, httpChannel, badRequestCause, e.getFailedHeaderName()); } catch (final RestRequest.BadParameterException e) { badRequestCause = ExceptionsHelper.useOrSuppress(badRequestCause, e); innerRestRequest = RestRequest.requestWithoutParameters(xContentRegistry, httpRequest, httpChannel); @@ -384,8 +384,11 @@ private void handleIncomingRequest(final HttpRequest httpRequest, final HttpChan dispatchRequest(restRequest, channel, badRequestCause); } - private RestRequest requestWithoutContentTypeHeader(HttpRequest httpRequest, HttpChannel httpChannel, Exception badRequestCause) { - HttpRequest httpRequestWithoutContentType = httpRequest.removeHeader("Content-Type"); + private RestRequest requestWithoutFailedHeader(HttpRequest httpRequest, + HttpChannel httpChannel, + Exception badRequestCause, + String failedHeaderName) { + HttpRequest httpRequestWithoutContentType = httpRequest.removeHeader(failedHeaderName); try { return RestRequest.request(xContentRegistry, httpRequestWithoutContentType, httpChannel); } catch (final RestRequest.BadParameterException e) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 49a8eabef5f46..aedf3f026367a 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -90,12 +90,16 @@ private RestRequest(NamedXContentRegistry xContentRegistry, Map Map> headers, HttpRequest httpRequest, HttpChannel httpChannel, long requestId) { try { this.parsedAccept = parseHeaderWithMediaType(httpRequest.getHeaders(), "Accept"); + } catch (IllegalArgumentException e) { + throw new MediaTypeHeaderException(e, "Accept"); + } + try { this.parsedContentType = parseHeaderWithMediaType(httpRequest.getHeaders(), "Content-Type"); if (parsedContentType != null) { this.xContentType.set(parsedContentType.toMediaType(XContentType.MEDIA_TYPE_REGISTRY)); } } catch (IllegalArgumentException e) { - throw new MediaTypeHeaderException(e); + throw new MediaTypeHeaderException(e, "Content-Type"); } this.xContentRegistry = xContentRegistry; this.httpRequest = httpRequest; @@ -551,10 +555,21 @@ public static XContentType parseContentType(List header) { public static class MediaTypeHeaderException extends RuntimeException { - MediaTypeHeaderException(final IllegalArgumentException cause) { + private String failedHeaderName; + + MediaTypeHeaderException(final IllegalArgumentException cause, String failedHeaderName) { super(cause); + this.failedHeaderName = failedHeaderName; } + public String getFailedHeaderName() { + return failedHeaderName; + } + + @Override + public String getMessage() { + return "Invalid media-type value on header [" + failedHeaderName + "]"; + } } public static class BadParameterException extends RuntimeException { diff --git a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java index 348e8a83af731..c18555a5a4568 100644 --- a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java +++ b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java @@ -47,19 +47,23 @@ import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static java.net.InetAddress.getByName; import static java.util.Arrays.asList; import static org.elasticsearch.http.AbstractHttpServerTransport.resolvePublishPort; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; public class AbstractHttpServerTransportTests extends ESTestCase { @@ -173,6 +177,72 @@ public HttpStats stats() { } } + public void testIncorrectHeaderHandling() { + + final ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + try (AbstractHttpServerTransport transport = + failureAssertingtHttpServerTransport(clusterSettings, "Accept")) { + + + Map> headers = new HashMap<>(); + headers.put("Accept", Collections.singletonList("incorrectheader")); + + FakeRestRequest.FakeHttpRequest fakeHttpRequest = + new FakeRestRequest.FakeHttpRequest(RestRequest.Method.GET, "/", null, headers); + + transport.incomingRequest(fakeHttpRequest, null); + } + try (AbstractHttpServerTransport transport = + failureAssertingtHttpServerTransport(clusterSettings, "Content-Type")) { + Map> headers = new HashMap<>(); + headers.put("Accept", Collections.singletonList("application/json")); + headers.put("Content-Type", Collections.singletonList("incorrectheader")); + + FakeRestRequest.FakeHttpRequest fakeHttpRequest = + new FakeRestRequest.FakeHttpRequest(RestRequest.Method.GET, "/", null, headers); + + transport.incomingRequest(fakeHttpRequest, null); + } + } + + private AbstractHttpServerTransport failureAssertingtHttpServerTransport(ClusterSettings clusterSettings, + final String failedHeaderName) { + return new AbstractHttpServerTransport(Settings.EMPTY, networkService, bigArrays, threadPool, xContentRegistry(), + new HttpServerTransport.Dispatcher() { + @Override + public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) { + Assert.fail(); + } + + @Override + public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) { + assertThat(cause, instanceOf(RestRequest.MediaTypeHeaderException.class)); + RestRequest.MediaTypeHeaderException mediaTypeHeaderException = (RestRequest.MediaTypeHeaderException) cause; + assertThat(mediaTypeHeaderException.getFailedHeaderName(), equalTo(failedHeaderName)); + assertThat(mediaTypeHeaderException.getMessage(), + equalTo("Invalid media-type value on header [" + failedHeaderName + "]")); + } + }, clusterSettings) { + @Override + protected HttpServerChannel bind(InetSocketAddress hostAddress) { + return null; + } + + @Override + protected void doStart() { + } + + @Override + protected void stopInternal() { + } + + @Override + public HttpStats stats() { + return null; + } + }; + } + @TestLogging( value = "org.elasticsearch.http.HttpTracer:trace", reason = "to ensure we log REST requests on TRACE level") diff --git a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java index 20fa04b72573b..4c6008bd90629 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java @@ -207,7 +207,8 @@ public void testMalformedContentTypeHeader() { }); assertNotNull(e.getCause()); assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); - assertThat(e.getMessage(), equalTo("java.lang.IllegalArgumentException: invalid media type [" + type + "]")); + assertThat(e.getCause().getMessage(), equalTo("invalid media-type [" + type + "]")); + assertThat(e.getMessage(), equalTo("Invalid media-type value on header [Content-Type]")); } public void testNoContentTypeHeader() { @@ -222,8 +223,8 @@ public void testMultipleContentTypeHeaders() { () -> contentRestRequest("", Collections.emptyMap(), Collections.singletonMap("Content-Type", headers))); assertNotNull(e.getCause()); assertThat(e.getCause(), instanceOf((IllegalArgumentException.class))); - assertThat(e.getMessage(), equalTo("java.lang.IllegalArgumentException: Incorrect header [Content-Type]." + - " Only one value should be provided")); + assertThat(e.getCause().getMessage(), equalTo("Incorrect header [Content-Type]. Only one value should be provided")); + assertThat(e.getMessage(), equalTo("Invalid media-type value on header [Content-Type]")); } public void testRequiredContent() { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java index e36d4ae13b668..76e2455857a67 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java @@ -48,7 +48,7 @@ private FakeRestRequest(NamedXContentRegistry xContentRegistry, HttpRequest http super(xContentRegistry, params, httpRequest.uri(), httpRequest.getHeaders(), httpRequest, httpChannel); } - private static class FakeHttpRequest implements HttpRequest { + public static class FakeHttpRequest implements HttpRequest { private final Method method; private final String uri; @@ -56,7 +56,7 @@ private static class FakeHttpRequest implements HttpRequest { private final Map> headers; private final Exception inboundException; - private FakeHttpRequest(Method method, String uri, BytesReference content, Map> headers) { + public FakeHttpRequest(Method method, String uri, BytesReference content, Map> headers) { this(method, uri, content, headers, null); }