Fix #5575 SEARCH method (#5576)

+ Added all IANA methods
 + Used Trie for most lookups
 + Fixed ArrayTernayTrie lookup
 + optimised GET, POST and HEAD

Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java

@@ -26,137 +26,72 @@ import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.Trie;
 
 /**
- *
+ * Known HTTP Methods
  */
 public enum HttpMethod
 {
-    GET,
+    // From https://www.iana.org/assignments/http-methods/http-methods.xhtml
-    POST,
+    ACL(Type.IDEMPOTENT),
-    HEAD,
+    BASELINE_CONTROL(Type.IDEMPOTENT),
-    PUT,
+    BIND(Type.IDEMPOTENT),
-    OPTIONS,
+    CHECKIN(Type.IDEMPOTENT),
-    DELETE,
+    CHECKOUT(Type.IDEMPOTENT),
-    TRACE,
+    CONNECT(Type.NORMAL),
-    CONNECT,
+    COPY(Type.IDEMPOTENT),
-    MOVE,
+    DELETE(Type.IDEMPOTENT),
-    PROXY,
+    GET(Type.SAFE),
-    PRI;
+    HEAD(Type.SAFE),
+    LABEL(Type.IDEMPOTENT),
+    LINK(Type.IDEMPOTENT),
+    LOCK(Type.NORMAL),
+    MERGE(Type.IDEMPOTENT),
+    MKACTIVITY(Type.IDEMPOTENT),
+    MKCALENDAR(Type.IDEMPOTENT),
+    MKCOL(Type.IDEMPOTENT),
+    MKREDIRECTREF(Type.IDEMPOTENT),
+    MKWORKSPACE(Type.IDEMPOTENT),
+    MOVE(Type.IDEMPOTENT),
+    OPTIONS(Type.SAFE),
+    ORDERPATCH(Type.IDEMPOTENT),
+    PATCH(Type.NORMAL),
+    POST(Type.NORMAL),
+    PRI(Type.SAFE),
+    PROPFIND(Type.SAFE),
+    PROPPATCH(Type.IDEMPOTENT),
+    PUT(Type.IDEMPOTENT),
+    REBIND(Type.IDEMPOTENT),
+    REPORT(Type.SAFE),
+    SEARCH(Type.SAFE),
+    TRACE(Type.SAFE),
+    UNBIND(Type.IDEMPOTENT),
+    UNCHECKOUT(Type.IDEMPOTENT),
+    UNLINK(Type.IDEMPOTENT),
+    UNLOCK(Type.IDEMPOTENT),
+    UPDATE(Type.IDEMPOTENT),
+    UPDATEREDIRECTREF(Type.IDEMPOTENT),
+    VERSION_CONTROL(Type.IDEMPOTENT),
 
-    /**
+    // Other methods
-     * Optimized lookup to find a method name and trailing space in a byte array.
+    PROXY(Type.NORMAL);
-     *
-     * @param bytes Array containing ISO-8859-1 characters
-     * @param position The first valid index
-     * @param limit The first non valid index
-     * @return An HttpMethod if a match or null if no easy match.
-     */
-    public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit)
-    {
-        int length = limit - position;
-        if (length < 4)
-            return null;
-        switch (bytes[position])
-        {
-            case 'G':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ')
-                    return GET;
-                break;
-            case 'P':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'S' && bytes[position + 3] == 'T' && length >= 5 && bytes[position + 4] == ' ')
-                    return POST;
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'O' && bytes[position + 3] == 'X' && length >= 6 && bytes[position + 4] == 'Y' && bytes[position + 5] == ' ')
-                    return PROXY;
-                if (bytes[position + 1] == 'U' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ')
-                    return PUT;
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'I' && bytes[position + 3] == ' ')
-                    return PRI;
-                break;
-            case 'H':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'A' && bytes[position + 3] == 'D' && length >= 5 && bytes[position + 4] == ' ')
-                    return HEAD;
-                break;
-            case 'O':
-                if (bytes[position + 1] == 'P' && bytes[position + 2] == 'T' && bytes[position + 3] == 'I' && length >= 8 &&
-                    bytes[position + 4] == 'O' && bytes[position + 5] == 'N' && bytes[position + 6] == 'S' && bytes[position + 7] == ' ')
-                    return OPTIONS;
-                break;
-            case 'D':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'L' && bytes[position + 3] == 'E' && length >= 7 &&
-                    bytes[position + 4] == 'T' && bytes[position + 5] == 'E' && bytes[position + 6] == ' ')
-                    return DELETE;
-                break;
-            case 'T':
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'A' && bytes[position + 3] == 'C' && length >= 6 &&
-                    bytes[position + 4] == 'E' && bytes[position + 5] == ' ')
-                    return TRACE;
-                break;
-            case 'C':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'N' && bytes[position + 3] == 'N' && length >= 8 &&
-                    bytes[position + 4] == 'E' && bytes[position + 5] == 'C' && bytes[position + 6] == 'T' && bytes[position + 7] == ' ')
-                    return CONNECT;
-                break;
-            case 'M':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'V' && bytes[position + 3] == 'E' && length >= 5 && bytes[position + 4] == ' ')
-                    return MOVE;
-                break;
 
-            default:
+    // The type of the method
-                break;
+    private enum Type
-        }
+    {
-        return null;
+        NORMAL,
+        IDEMPOTENT,
+        SAFE
     }
 
-    /**
+    private final String _method;
-     * Optimized lookup to find a method name and trailing space in a byte array.
-     *
-     * @param buffer buffer containing ISO-8859-1 characters, it is not modified.
-     * @return An HttpMethod if a match or null if no easy match.
-     */
-    public static HttpMethod lookAheadGet(ByteBuffer buffer)
-    {
-        if (buffer.hasArray())
-            return lookAheadGet(buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.arrayOffset() + buffer.limit());
-
-        int l = buffer.remaining();
-        if (l >= 4)
-        {
-            HttpMethod m = CACHE.getBest(buffer, 0, l);
-            if (m != null)
-            {
-                int ml = m.asString().length();
-                if (l > ml && buffer.get(buffer.position() + ml) == ' ')
-                    return m;
-            }
-        }
-        return null;
-    }
-
-    public static final Trie<HttpMethod> INSENSITIVE_CACHE = new ArrayTrie<>();
-
-    static
-    {
-        for (HttpMethod method : HttpMethod.values())
-        {
-            INSENSITIVE_CACHE.put(method.toString(), method);
-        }
-    }
-
-    public static final Trie<HttpMethod> CACHE = new ArrayTernaryTrie<>(false);
-
-    static
-    {
-        for (HttpMethod method : HttpMethod.values())
-        {
-            CACHE.put(method.toString(), method);
-        }
-    }
-
-    private final ByteBuffer _buffer;
     private final byte[] _bytes;
+    private final ByteBuffer _buffer;
+    private final Type _type;
 
-    HttpMethod()
+    HttpMethod(Type type)
     {
-        _bytes = StringUtil.getBytes(toString());
+        _method = name().replace('_', '-');
+        _type = type;
+        _bytes = StringUtil.getBytes(_method);
         _buffer = ByteBuffer.wrap(_bytes);
     }
 
@@ -170,6 +105,28 @@ public enum HttpMethod
         return toString().equalsIgnoreCase(s);
     }
 
+    /**
+     * An HTTP method is safe if it doesn't alter the state of the server.
+     * In other words, a method is safe if it leads to a read-only operation.
+     * Several common HTTP methods are safe: GET , HEAD , or OPTIONS .
+     * All safe methods are also idempotent, but not all idempotent methods are safe
+     * @return if the method is safe.
+     */
+    public boolean isSafe()
+    {
+        return _type == Type.SAFE;
+    }
+
+    /**
+     * An idempotent HTTP method is an HTTP method that can be called many times without different outcomes.
+     * It would not matter if the method is called only once, or ten times over. The result should be the same.
+     * @return true if the method is idempotent.
+     */
+    public boolean isIdempotent()
+    {
+        return _type.ordinal() >= Type.IDEMPOTENT.ordinal();
+    }
+
     public ByteBuffer asBuffer()
     {
         return _buffer.asReadOnlyBuffer();
@@ -177,11 +134,94 @@ public enum HttpMethod
 
     public String asString()
     {
-        return toString();
+        return _method;
+    }
+
+    public String toString()
+    {
+        return _method;
+    }
+
+    public static final Trie<HttpMethod> INSENSITIVE_CACHE = new ArrayTrie<>(252);
+    public static final Trie<HttpMethod> CACHE = new ArrayTernaryTrie<>(false, 300);
+    public static final Trie<HttpMethod> LOOK_AHEAD = new ArrayTernaryTrie<>(false, 330);
+    public static final int ACL_AS_INT = ('A' & 0xff) << 24 | ('C' & 0xFF) << 16 | ('L' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int GET_AS_INT = ('G' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int PRI_AS_INT = ('P' & 0xff) << 24 | ('R' & 0xFF) << 16 | ('I' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int PUT_AS_INT = ('P' & 0xff) << 24 | ('U' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int POST_AS_INT = ('P' & 0xff) << 24 | ('O' & 0xFF) << 16 | ('S' & 0xFF) << 8 | ('T' & 0xFF);
+    public static final int HEAD_AS_INT = ('H' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('A' & 0xFF) << 8 | ('D' & 0xFF);
+    static
+    {
+        for (HttpMethod method : HttpMethod.values())
+        {
+            if (!INSENSITIVE_CACHE.put(method.asString(), method))
+                throw new IllegalStateException("INSENSITIVE_CACHE too small: " + method);
+
+            if (!CACHE.put(method.asString(), method))
+                throw new IllegalStateException("CACHE too small: " + method);
+
+            if (!LOOK_AHEAD.put(method.asString() + ' ', method))
+                throw new IllegalStateException("LOOK_AHEAD too small: " + method);
+        }
     }
 
     /**
-     * Converts the given String parameter to an HttpMethod
+     * Optimized lookup to find a method name and trailing space in a byte array.
+     *
+     * @param bytes Array containing ISO-8859-1 characters
+     * @param position The first valid index
+     * @param limit The first non valid index
+     * @return An HttpMethod if a match or null if no easy match.
+     * @deprecated Not used
+     */
+    @Deprecated
+    public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit)
+    {
+        return LOOK_AHEAD.getBest(bytes, position, limit - position);
+    }
+
+    /**
+     * Optimized lookup to find a method name and trailing space in a byte array.
+     *
+     * @param buffer buffer containing ISO-8859-1 characters, it is not modified.
+     * @return An HttpMethod if a match or null if no easy match.
+     */
+    public static HttpMethod lookAheadGet(ByteBuffer buffer)
+    {
+        int len = buffer.remaining();
+        // Short cut for 3 char methods, mostly for GET optimisation
+        if (len > 3)
+        {
+            switch (buffer.getInt(buffer.position()))
+            {
+                case ACL_AS_INT:
+                    return ACL;
+                case GET_AS_INT:
+                    return GET;
+                case PRI_AS_INT:
+                    return PRI;
+                case PUT_AS_INT:
+                    return PUT;
+                case POST_AS_INT:
+                    if (len > 4 && buffer.get(buffer.position() + 4) == ' ')
+                        return POST;
+                    break;
+                case HEAD_AS_INT:
+                    if (len > 4 && buffer.get(buffer.position() + 4) == ' ')
+                        return HEAD;
+                    break;
+                default:
+                    break;
+            }
+        }
+        return LOOK_AHEAD.getBest(buffer, 0, len);
+    }
+
+    /**
+     * Converts the given String parameter to an HttpMethod.
+     * The string may differ from the Enum name as a '-'  in the method
+     * name is represented as a '_' in the Enum name.
      *
      * @param method the String to get the equivalent HttpMethod from
      * @return the HttpMethod or null if the parameter method is unknown

jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java

@@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 
 import org.eclipse.jetty.http.HttpParser.State;
 import org.eclipse.jetty.toolchain.test.Net;
@@ -32,6 +33,8 @@ import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.eclipse.jetty.http.HttpComplianceSection.NO_FIELD_FOLDING;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -85,12 +88,20 @@ public class HttpParserTest
     @Test
     public void httpMethodTest()
     {
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("Wibble ")));
+        for (HttpMethod m : HttpMethod.values())
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET")));
+        {
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("MO")));
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString().substring(0,2))));
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString())));
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + "FOO")));
+            assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " ")));
+            assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " /foo/bar")));
 
-        assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET ")));
+            assertNull(HttpMethod.lookAheadGet(m.asString().substring(0,2).getBytes(), 0,2));
-        assertEquals(HttpMethod.MOVE, HttpMethod.lookAheadGet(BufferUtil.toBuffer("MOVE ")));
+            assertNull(HttpMethod.lookAheadGet(m.asString().getBytes(), 0, m.asString().length()));
+            assertNull(HttpMethod.lookAheadGet((m.asString() + "FOO").getBytes(), 0, m.asString().length() + 3));
+            assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " ").getBytes(), 1, m.asString().length() + 2));
+            assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " /foo").getBytes(), 1, m.asString().length() + 6));
+        }
 
         ByteBuffer b = BufferUtil.allocateDirect(128);
         BufferUtil.append(b, BufferUtil.toBuffer("GET"));
@@ -100,6 +111,15 @@ public class HttpParserTest
         assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(b));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"GET", "POST", "VERSION-CONTROL"})
+    public void httpMethodNameTest(String methodName)
+    {
+        HttpMethod method = HttpMethod.fromString(methodName);
+        assertNotNull(method, "Method should have been found: " + methodName);
+        assertEquals(methodName.toUpperCase(Locale.US), method.toString());
+    }
+
     @Test
     public void testLineParseMockIP()
     {

jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java

@@ -0,0 +1,127 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.http.jmh;
+
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.util.BufferUtil;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.profile.GCProfiler;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+@State(Scope.Benchmark)
+@Threads(4)
+@Warmup(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
+@Measurement(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
+public class HttpMethodBenchmark
+{
+    private static final ByteBuffer GET = BufferUtil.toBuffer("GET / HTTP/1.1\r\n\r\n");
+    private static final ByteBuffer POST = BufferUtil.toBuffer("POST / HTTP/1.1\r\n\r\n");
+    private static final ByteBuffer MOVE = BufferUtil.toBuffer("MOVE / HTTP/1.1\r\n\r\n");
+    private static final Map<String, HttpMethod> MAP = new HashMap<>();
+
+    static
+    {
+        for (HttpMethod m : HttpMethod.values())
+            MAP.put(m.asString(), m);
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testTrieGetBest() throws Exception
+    {
+        return HttpMethod.LOOK_AHEAD.getBest(GET, 0, GET.remaining());
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testIntSwitch() throws Exception
+    {
+        switch (GET.getInt(0))
+        {
+            case HttpMethod.ACL_AS_INT:
+                return HttpMethod.ACL;
+            case HttpMethod.GET_AS_INT:
+                return HttpMethod.GET;
+            case HttpMethod.PRI_AS_INT:
+                return HttpMethod.PRI;
+            case HttpMethod.PUT_AS_INT:
+                return HttpMethod.PUT;
+            default:
+                return null;
+        }
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testMapGet() throws Exception
+    {
+        for (int i = 0; i < GET.remaining(); i++)
+        {
+            if (GET.get(i) == (byte)' ')
+                return MAP.get(BufferUtil.toString(GET, 0, i, StandardCharsets.US_ASCII));
+        }
+        return null;
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testHttpMethodPost() throws Exception
+    {
+        return HttpMethod.lookAheadGet(POST);
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testHttpMethodMove() throws Exception
+    {
+        return HttpMethod.lookAheadGet(MOVE);
+    }
+
+    public static void main(String[] args) throws RunnerException
+    {
+        Options opt = new OptionsBuilder()
+            .include(HttpMethodBenchmark.class.getSimpleName())
+            .warmupIterations(10)
+            .measurementIterations(10)
+            .addProfiler(GCProfiler.class)
+            .forks(1)
+            .threads(1)
+            .build();
+
+        new Runner(opt).run();
+    }
+}
+
+

jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java

@@ -368,6 +368,12 @@ public class ArrayTernaryTrie<V> extends AbstractTrie<V>
         return getBest(0, b, offset, len);
     }
 
+    @Override
+    public V getBest(byte[] b, int offset, int len)
+    {
+        return getBest(0, b, offset, len);
+    }
+
     private V getBest(int t, byte[] b, int offset, int len)
     {
         int node = t;