Improved REST error handling

core/src/main/java/org/elasticsearch/common/path/PathTrie.java

@@ -26,6 +26,28 @@
 import static java.util.Collections.unmodifiableMap;
 
 public class PathTrie<T> {
+
+    public static enum TrieMatchingMode {
+        /*
+         * Retrieve only explicitly mapped nodes, no wildcards are 
+         * matched.
+         */
+        EXPLICIT_NODES_ONLY,
+        /*
+         * Retrieve only explicitly mapped nodes, with wildcards 
+         * allowed as root nodes.
+         */
+        WILDCARD_ROOT_NODES_ALLOWED,
+        /*
+         * Retrieve only explicitly mapped nodes, with wildcards 
+         * allowed as leaf nodes.
+         */
+        WILDCARD_LEAF_NODES_ALLOWED,
+        /*
+         * Retrieve both explicitly mapped and wildcard nodes.
+         */
+        WILDCARD_NODES_ALLOWED
+    }
 
     public interface Decoder {
         String decode(String value);
@@ -107,9 +129,10 @@ public synchronized void insert(String[] path, int index, T value) {
                 if (isNamedWildcard(token)) {
                     node.updateKeyWithNamedWildcard(token);
                 }
-
-                // in case the target(last) node already exist but without a value
-                // than the value should be updated.
+                /*
+                 * If the target node already exists, but is without a value,
+                 *  then the value should be updated.
+                 */
                 if (index == (path.length - 1)) {
                     assert (node.value == null || node.value == value);
                     if (node.value == null) {
@@ -133,23 +156,58 @@ private boolean isNamedWildcard() {
             return namedWildcard != null;
         }
 
-        public T retrieve(String[] path, int index, Map<String, String> params) {
+        public T retrieve(String[] path, int index, Map<String, String> params, TrieMatchingMode trieMatchingMode) {
             if (index >= path.length)
                 return null;
 
             String token = path[index];
             TrieNode node = children.get(token);
             boolean usedWildcard;
+
             if (node == null) {
-                node = children.get(wildcard);
-                if (node == null) {
+                if (trieMatchingMode == TrieMatchingMode.WILDCARD_NODES_ALLOWED) {
+                    node = children.get(wildcard);
+                    if (node == null) {
+                        return null;
+                    }
+                    usedWildcard = true;
+                } else if (trieMatchingMode == TrieMatchingMode.WILDCARD_ROOT_NODES_ALLOWED && index == 1) {
+                    /*
+                     * Allow root node wildcard matches.
+                     */
+                    node = children.get(wildcard);
+                    if (node == null) {
+                        return null;
+                    }
+                    usedWildcard = true;
+                } else if (trieMatchingMode == TrieMatchingMode.WILDCARD_LEAF_NODES_ALLOWED && index + 1 == path.length) {
+                    /*
+                     * Allow leaf node wildcard matches.
+                     */
+                    node = children.get(wildcard);
+                    if (node == null) {
+                        return null;
+                    }
+                    usedWildcard = true;
+                } else {
                     return null;
                 }
-                usedWildcard = true;
             } else {
-                // If we are at the end of the path, the current node does not have a value but there
-                // is a child wildcard node, use the child wildcard node
-                if (index + 1 == path.length && node.value == null && children.get(wildcard) != null) {
+                if (index + 1 == path.length && node.value == null && children.get(wildcard) != null
+                        && trieMatchingMode != TrieMatchingMode.WILDCARD_ROOT_NODES_ALLOWED
+                        && trieMatchingMode != TrieMatchingMode.EXPLICIT_NODES_ONLY) {
+                    /*
+                     * If we are at the end of the path, the current node does not have a value but
+                     * there is a child wildcard node, use the child wildcard node.
+                     */
+                    node = children.get(wildcard);
+                    usedWildcard = true;
+                } else if (index == 1 && node.value == null && children.get(wildcard) != null 
+                        && trieMatchingMode == TrieMatchingMode.WILDCARD_ROOT_NODES_ALLOWED) {
+                    /*
+                     * If we are at the root, and root wildcards are allowed, use the child wildcard
+                     * node.
+                     */
                     node = children.get(wildcard);
                     usedWildcard = true;
                 } else {
@@ -163,16 +221,16 @@ public T retrieve(String[] path, int index, Map<String, String> params) {
                 return node.value;
             }
 
-            T res = node.retrieve(path, index + 1, params);
-            if (res == null && !usedWildcard) {
+            T nodeValue = node.retrieve(path, index + 1, params, trieMatchingMode);
+            if (nodeValue == null && !usedWildcard && trieMatchingMode != TrieMatchingMode.EXPLICIT_NODES_ONLY) {
                 node = children.get(wildcard);
                 if (node != null) {
                     put(params, node, token);
-                    res = node.retrieve(path, index + 1, params);
+                    nodeValue = node.retrieve(path, index + 1, params, trieMatchingMode);
                 }
             }
 
-            return res;
+            return nodeValue;
         }
 
         private void put(Map<String, String> params, TrieNode node, String value) {
@@ -194,18 +252,22 @@ public void insert(String path, T value) {
             return;
         }
         int index = 0;
-        // supports initial delimiter.
+        // Supports initial delimiter.
         if (strings.length > 0 && strings[0].isEmpty()) {
             index = 1;
         }
         root.insert(strings, index, value);
     }
 
     public T retrieve(String path) {
-        return retrieve(path, null);
+        return retrieve(path, null, TrieMatchingMode.WILDCARD_NODES_ALLOWED);
     }
 
     public T retrieve(String path, Map<String, String> params) {
+        return retrieve(path, params, TrieMatchingMode.WILDCARD_NODES_ALLOWED);
+    }
+
+    public T retrieve(String path, Map<String, String> params, TrieMatchingMode trieMatchingMode) {
         if (path.length() == 0) {
             return rootValue;
         }
@@ -214,10 +276,12 @@ public T retrieve(String path, Map<String, String> params) {
             return rootValue;
         }
         int index = 0;
-        // supports initial delimiter.
+
+        // Supports initial delimiter.
         if (strings.length > 0 && strings[0].isEmpty()) {
             index = 1;
         }
-        return root.retrieve(strings, index, params);
+
+        return root.retrieve(strings, index, params, trieMatchingMode);
     }
 }

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

@@ -23,21 +23,26 @@
 import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.path.PathTrie;
+import org.elasticsearch.common.path.PathTrie.TrieMatchingMode;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.elasticsearch.rest.RestStatus.BAD_REQUEST;
+import static org.elasticsearch.rest.RestStatus.METHOD_NOT_ALLOWED;
 import static org.elasticsearch.rest.RestStatus.OK;
+import static org.elasticsearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE;
 
 public class RestController extends AbstractLifecycleComponent {
     private final PathTrie<RestHandler> getHandlers = new PathTrie<>(RestUtils.REST_DECODER);
@@ -235,20 +240,124 @@ boolean checkRequestParameters(final RestRequest request, final RestChannel chan
     }
 
     void executeHandler(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
-        final RestHandler handler = getHandler(request);
+        // Request execution flag
+        boolean requestHandled = false;
+
+        /*
+         * First try handlers mapped to explicit paths only.
+         */
+        requestHandled = executeHandler(request, channel, client, TrieMatchingMode.EXPLICIT_NODES_ONLY);
+
+        if (requestHandled == false) {
+            /*
+             * Fallback to handlers mapped to explicit paths, with a wildcard allowed as a leaf node only.
+             */
+            requestHandled = executeHandler(request, channel, client, TrieMatchingMode.WILDCARD_LEAF_NODES_ALLOWED);
+        }
+
+        if (requestHandled == false) {
+            /*
+             * Fallback to handlers mapped to explicit paths, with a wildcard allowed as a root node only.
+             */
+            requestHandled = executeHandler(request, channel, client, TrieMatchingMode.WILDCARD_ROOT_NODES_ALLOWED);
+        }
+
+        if (requestHandled == false) {
+            /*
+             * Fallback to handlers mapped to explicit paths, with a wildcard allowed as any node.
+             */
+            requestHandled = executeHandler(request, channel, client, TrieMatchingMode.WILDCARD_NODES_ALLOWED);
+        }
+
+        /*
+         * If request has not been handled, fallback to a bad request error.
+         */
+        if (requestHandled == false) {
+            handleBadRequest(request, channel);
+        }
+    }
+
+    private boolean executeHandler(RestRequest request, RestChannel channel, NodeClient client, PathTrie.TrieMatchingMode trieMatchingMode) throws Exception {
+        /*
+         * Get the matching handler for this request, including both explicit
+         * and wildcard path matches, if one exists.
+         */
+        final RestHandler handler = getHandler(request, trieMatchingMode);
         if (handler != null) {
+            /*
+             * Handle valid REST request (the request matches a handler).
+             */
             handler.handleRequest(request, channel, client);
+            return true;
         } 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, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY));
-            } else {
-                final String msg = "No handler found for uri [" + request.uri() + "] and method [" + request.method() + "]";
-                channel.sendResponse(new BytesRestResponse(BAD_REQUEST, msg));
+            /*
+             * Get the map of matching handlers for a request, for the full set of HTTP methods.
+             */
+            final HashSet<RestRequest.Method> validMethodSet = getValidHandlerMethodSet(request, trieMatchingMode);
+            if (validMethodSet.size() > 0 
+                    && !validMethodSet.contains(request.method())
+                    && request.method() != RestRequest.Method.OPTIONS) {
+                /*
+                 * If an alternative handler for an explicit path is registered to a
+                 * different HTTP method than the one supplied - return a 405 Method
+                 * Not Allowed error.
+                 */
+                handleUnsupportedHttpMethod(request, channel, validMethodSet);
+                return true;
+            } else if (!validMethodSet.contains(request.method()) 
+                    && request.method() == RestRequest.Method.OPTIONS) {
+                handleOptionsRequest(request, channel, validMethodSet);
+                return true;
             }
         }
+
+        return false;
+    }
+
+    /**
+     * Handle requests to a valid REST endpoint using an unsupported HTTP
+     * method. A 405 HTTP response code is returned, and 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>).
+     */
+    private void handleUnsupportedHttpMethod(RestRequest request, RestChannel channel, HashSet<RestRequest.Method> validMethodSet) {
+        BytesRestResponse bytesRestResponse = new BytesRestResponse(METHOD_NOT_ALLOWED, TEXT_CONTENT_TYPE, BytesArray.EMPTY);
+        bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ","));
+        channel.sendResponse(bytesRestResponse);
     }
 
+    /**
+     * Handle HTTP OPTIONS requests to a valid REST endpoint. A 200 HTTP
+     * response code is returned, and 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>).
+     */
+    private void handleOptionsRequest(RestRequest request, RestChannel channel, HashSet<RestRequest.Method> validMethodSet) {
+        if (request.method() == RestRequest.Method.OPTIONS && validMethodSet.size() > 0) {
+            BytesRestResponse bytesRestResponse = new BytesRestResponse(OK, TEXT_CONTENT_TYPE, BytesArray.EMPTY);
+            bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ","));
+            channel.sendResponse(bytesRestResponse);
+        } else if (request.method() == RestRequest.Method.OPTIONS && validMethodSet.size() == 0) {
+            /*
+             * When we have an OPTIONS HTTP 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, TEXT_CONTENT_TYPE, BytesArray.EMPTY));
+        }
+    }
+
+    /**
+     * Handle a requests with no candidate handlers (return a 400 Bad Request
+     * error).
+     */
+    private void handleBadRequest(RestRequest request, RestChannel channel) {
+        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);
         PathTrie<RestHandler> handlers = getHandlersForMethod(request.method());
@@ -259,6 +368,16 @@ private RestHandler getHandler(RestRequest request) {
         }
     }
 
+    private RestHandler getHandler(RestRequest request, PathTrie.TrieMatchingMode trieMatchingMode) {
+        String path = getPath(request);
+        PathTrie<RestHandler> handlers = getHandlersForMethod(request.method());
+        if (handlers != null) {
+            return handlers.retrieve(path, request.params(), trieMatchingMode);
+        } else {
+            return null;
+        }
+    }
+
     private PathTrie<RestHandler> getHandlersForMethod(RestRequest.Method method) {
         if (method == RestRequest.Method.GET) {
             return getHandlers;
@@ -276,6 +395,33 @@ private PathTrie<RestHandler> getHandlersForMethod(RestRequest.Method method) {
             return null;
         }
     }
+
+    /**
+     * Get the valid set of HTTP methods for a REST request.
+     */
+    private HashSet<RestRequest.Method> getValidHandlerMethodSet(RestRequest request, PathTrie.TrieMatchingMode trieMatchingMode) {
+        String path = getPath(request);
+        HashSet<RestRequest.Method> validMethodSet = new HashSet<RestRequest.Method>();
+        if (getHandlers.retrieve(path, request.params(), trieMatchingMode) != null) {
+            validMethodSet.add(RestRequest.Method.GET);
+        }
+        if (postHandlers.retrieve(path, request.params(), trieMatchingMode) != null) {
+            validMethodSet.add(RestRequest.Method.POST);
+        }
+        if (putHandlers.retrieve(path, request.params(), trieMatchingMode) != null) {
+            validMethodSet.add(RestRequest.Method.PUT);
+        }
+        if (deleteHandlers.retrieve(path, request.params(), trieMatchingMode) != null) {
+            validMethodSet.add(RestRequest.Method.DELETE);
+        }
+        if (headHandlers.retrieve(path, request.params(), trieMatchingMode) != null) {
+            validMethodSet.add(RestRequest.Method.HEAD);
+        }
+        if (optionsHandlers.retrieve(path, request.params(), trieMatchingMode) != 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