Send REST response for incorrectly encoded url params

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequestHandler.java

@@ -77,7 +77,11 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Except
                 new Netty4HttpChannel(serverTransport, httpRequest, pipelinedRequest, detailedErrorsEnabled, threadContext);
 
         if (request.decoderResult().isSuccess()) {
-            serverTransport.dispatchRequest(httpRequest, channel);
+            if (httpRequest.getException() != null) {
+                serverTransport.dispatchBadRequest(httpRequest, channel, httpRequest.getException());
+            } else {
+                serverTransport.dispatchRequest(httpRequest, channel);
+            }
         } else {
             assert request.decoderResult().isFailure();
             serverTransport.dispatchBadRequest(httpRequest, channel, request.decoderResult().cause());

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

@@ -62,6 +62,7 @@ public abstract class RestRequest implements ToXContent.Params {
     private final String rawPath;
     private final Set<String> consumedParams = new HashSet<>();
     private final SetOnce<XContentType> xContentType = new SetOnce<>();
+    private final SetOnce<Exception> exception = new SetOnce<>();
 
     /**
      * Creates a new RestRequest
@@ -74,12 +75,18 @@ public RestRequest(NamedXContentRegistry xContentRegistry, String uri, Map<Strin
         this.xContentRegistry = xContentRegistry;
         final Map<String, String> params = new HashMap<>();
         int pathEndPos = uri.indexOf('?');
+        Exception parsingQueryStringException= null;
         if (pathEndPos < 0) {
             this.rawPath = uri;
         } else {
             this.rawPath = uri.substring(0, pathEndPos);
-            RestUtils.decodeQueryString(uri, pathEndPos + 1, params);
+            try {
+                RestUtils.decodeQueryString(uri, pathEndPos + 1, params);
+            } catch (Exception e) {
+                parsingQueryStringException = e;
+            }
         }
+        this.exception.set(parsingQueryStringException);
         this.params = params;
         this.headers = Collections.unmodifiableMap(headers);
         final List<String> contentType = getAllHeaderValues("Content-Type");
@@ -206,6 +213,10 @@ public SocketAddress getLocalAddress() {
         return null;
     }
 
+    public Exception getException() {
+        return exception.get();
+    }
+
     public final boolean hasParam(String key) {
         return params.containsKey(key);
     }

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

@@ -42,11 +42,13 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
 import static org.elasticsearch.ElasticsearchExceptionTests.assertDeepEquals;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 
@@ -197,6 +199,58 @@ public BytesReference content() {
         assertThat(content, containsString("\"status\":" + 400));
     }
 
+    public void testResponseWhenParametersEncodingError() throws IOException {
+        parametersEncodingError("path?param=value%", "unterminated escape sequence at end of string: value%");
+        parametersEncodingError("path?param%=value%", "unterminated escape sequence at end of string: param%");
+        parametersEncodingError("path?param=value%a", "partial escape sequence at end of string: value%a");
+        parametersEncodingError("path?param%a=value%a", "partial escape sequence at end of string: param%a");
+        parametersEncodingError("path?param%aZ=value%a", "invalid escape sequence `%aZ' at index 5 of: param%aZ");
+    }
+
+    private void parametersEncodingError(String uri, String errorString) throws IOException {
+        final RestRequest request = new TestParamsRequest(NamedXContentRegistry.EMPTY, uri, Collections.emptyMap());
+        assertThat(request.getException().getClass(), equalTo(IllegalArgumentException.class));
+        assertThat(request.getException().getMessage(), equalTo(errorString));
+
+        final RestChannel channel = new DetailedExceptionRestChannel(request);
+        final BytesRestResponse response = new BytesRestResponse(channel, request.getException());
+        assertNotNull(response.content());
+        final String content = response.content().utf8ToString();
+        assertThat(content, containsString("\"type\":\"illegal_argument_exception\""));
+        assertThat(content, containsString("\"reason\":\"" + errorString + "\""));
+        assertThat(content, containsString("\"status\":" + 400));
+    }
+
+    private class TestParamsRequest extends RestRequest {
+
+        private final String uri;
+
+        TestParamsRequest(NamedXContentRegistry xContentRegistry, String uri, Map<String, List<String>> headers) {
+            super(xContentRegistry, uri, headers);
+            this.uri = uri;
+        }
+
+        @Override
+        public Method method() {
+            return null;
+        }
+
+        @Override
+        public String uri() {
+            return uri;
+        }
+
+        @Override
+        public boolean hasContent() {
+            return false;
+        }
+
+        @Override
+        public BytesReference content() {
+            return null;
+        }
+    }
+
     public void testResponseWhenInternalServerError() throws IOException {
         final RestRequest request = new FakeRestRequest();
         final RestChannel channel = new DetailedExceptionRestChannel(request);