Improved REST error handling

core/src/main/java/org/elasticsearch/rest/RestController.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.rest;
 
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.path.PathTrie;
@@ -38,6 +39,7 @@
 import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableSet;
 import static org.elasticsearch.rest.RestStatus.BAD_REQUEST;
+import static org.elasticsearch.rest.RestStatus.METHOD_NOT_ALLOWED;
 import static org.elasticsearch.rest.RestStatus.OK;
 
 /**
@@ -208,19 +210,43 @@ boolean checkRequestParameters(final RestRequest request, final RestChannel chan
         return true;
     }
 
-    void executeHandler(RestRequest request, RestChannel channel) throws Exception {
-        final RestHandler handler = getHandler(request);
-        if (handler != null) {
-            handler.handleRequest(request, channel);
-        } else {
-            if (request.method() == RestRequest.Method.OPTIONS) {
-                // when we have OPTIONS request, simply send OK by default (with the Access Control Origin header which gets automatically added)
-                channel.sendResponse(new BytesRestResponse(OK));
-            } else {
-                channel.sendResponse(new BytesRestResponse(BAD_REQUEST, "No handler found for uri [" + request.uri() + "] and method [" + request.method() + "]"));
-            }
-        }
-    }
+	void executeHandler(RestRequest request, RestChannel channel) throws Exception {
+		final RestHandler handler = getHandler(request);
+		if (handler != null) {
+			handler.handleRequest(request, channel);
+		} else {
+			HashSet<RestRequest.Method> validMethodSet = getValidHandlerMethodSet(request);
+			if (request.method() == RestRequest.Method.OPTIONS && validMethodSet.size() == 0) {
+				/*
+				 * when we have an OPTIONS request and no valid handlers, simply
+				 * send OK by default (with the Access Control Origin header
+				 * which gets automatically added)
+				 */
+				channel.sendResponse(new BytesRestResponse(OK));
+			} else if (request.method() == RestRequest.Method.OPTIONS && validMethodSet.size() > 0) {
+				/*
+				 * when we have an OPTIONS request against a valid handler,
+				 * return a list of valid methods for the handler per the HTTP
+				 * spec
+				 */
+				BytesRestResponse bytesRestResponse = new BytesRestResponse(OK);
+				bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ","));
+				channel.sendResponse(bytesRestResponse);
+			} else if (validMethodSet.size() > 0) {
+				/*
+				 * when we have a request to a valid handler with an unsupported
+				 * method, return a list of valid methods for the handler per
+				 * the HTTP spec
+				 */
+				BytesRestResponse bytesRestResponse = new BytesRestResponse(METHOD_NOT_ALLOWED);
+				bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ","));
+				channel.sendResponse(bytesRestResponse);
+			} else {
+				channel.sendResponse(new BytesRestResponse(BAD_REQUEST,
+						"No handler found for uri [" + request.uri() + "] and method [" + request.method() + "]"));
+			}
+		}
+	}
 
     private RestHandler getHandler(RestRequest request) {
         String path = getPath(request);
@@ -242,6 +268,30 @@ private RestHandler getHandler(RestRequest request) {
         }
     }
 
+    private HashSet<RestRequest.Method> getValidHandlerMethodSet(RestRequest request) {
+        String path = getPath(request);
+        HashSet<RestRequest.Method> validMethodSet = new HashSet<RestRequest.Method>();
+        if (getHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.GET);
+        }
+        if (postHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.POST);
+        }
+        if (putHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.PUT);
+        }
+        if (deleteHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.DELETE);
+        }
+        if (headHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.HEAD);
+        }
+        if (optionsHandlers.retrieve(path, request.params()) != null) {
+            validMethodSet.add(RestRequest.Method.OPTIONS);
+        }
+        return validMethodSet;
+    }
+
     private String getPath(RestRequest request) {
         // we use rawPath since we don't want to decode it while processing the path resolution
         // so we can handle things like:

core/src/test/java/org/elasticsearch/rest/RestHTTPResponseHeadersIT.java

@@ -0,0 +1,84 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.rest;
+
+import org.elasticsearch.common.network.NetworkModule;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.rest.client.http.HttpResponse;
+import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Refer to
+ * <a href="https://github.com/elastic/elasticsearch/issues/15335">Unsupported
+ * methods on REST endpoints should respond with status code 405</a> for more
+ * information.
+ */
+@ClusterScope(scope = ESIntegTestCase.Scope.SUITE, numDataNodes = 1)
+public class RestHTTPResponseHeadersIT extends ESIntegTestCase {
+
+	@Override
+	protected Settings nodeSettings(int nodeOrdinal) {
+		return Settings.builder().put(NetworkModule.HTTP_ENABLED.getKey(), true).put(super.nodeSettings(nodeOrdinal)).build();
+	}
+
+	/**
+	 * For an OPTIONS request to a valid REST endpoint, verify that a 200 HTTP
+	 * response code is returned, and that the response 'Allow' header includes
+	 * a list of valid HTTP methods for the endpoint (see
+	 * <a href="https://tools.ietf.org/html/rfc2616#section-9.2">HTTP/1.1 - 9.2
+	 * - Options</a>).
+	 */
+	public void testValidEndpointOptionsResponseHTTPHeader() throws Exception {
+		createIndex("test");
+		HttpResponse httpResponse = httpClient().method("OPTIONS").path("/test").execute();
+		assertThat(httpResponse.getStatusCode(), is(200));
+		assertThat(httpResponse.getHeaders().get("Allow"), notNullValue());
+		List<String> allowHeader = Arrays.asList(httpResponse.getHeaders().get("Allow").split(","));
+		assertThat(allowHeader, containsInAnyOrder("HEAD", "GET", "PUT", "POST", "DELETE"));
+	}
+
+	/**
+	 * For requests to a valid REST endpoint using an unsupported HTTP method,
+	 * verify that a 405 HTTP response code is returned, and that the response
+	 * 'Allow' header includes a list of valid HTTP methods for the endpoint
+	 * (see
+	 * <a href="https://tools.ietf.org/html/rfc2616#section-10.4.6">HTTP/1.1 -
+	 * 10.4.6 - 405 Method Not Allowed</a>).
+	 */
+	public void testUnsupportedMethodResponseHTTPHeader() throws Exception {
+		createIndex("test");
+		HttpResponse httpResponse = httpClient().method("DELETE").path("/test/_analyze").execute();
+		assertThat(httpResponse.getStatusCode(), is(405));
+		assertThat(httpResponse.getHeaders().get("Allow"), notNullValue());
+		List<String> allowHeader = Arrays.asList(httpResponse.getHeaders().get("Allow").split(","));
+		assertThat(allowHeader, containsInAnyOrder("HEAD", "GET", "POST"));
+	}
+
+}