From bb886ad9320c170b9b12a111e00f6671978c8cee Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 11 Nov 2020 17:48:08 +0100 Subject: [PATCH] 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 --- .../org/eclipse/jetty/http/HttpMethod.java | 286 ++++++++++-------- .../eclipse/jetty/http/HttpParserTest.java | 30 +- .../jetty/http/jmh/HttpMethodBenchmark.java | 127 ++++++++ .../eclipse/jetty/util/ArrayTernaryTrie.java | 6 + 4 files changed, 321 insertions(+), 128 deletions(-) create mode 100644 jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java index 83f6bfc7365..e0e035072dc 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java +++ b/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, - POST, - HEAD, - PUT, - OPTIONS, - DELETE, - TRACE, - CONNECT, - MOVE, - PROXY, - PRI; + // From https://www.iana.org/assignments/http-methods/http-methods.xhtml + ACL(Type.IDEMPOTENT), + BASELINE_CONTROL(Type.IDEMPOTENT), + BIND(Type.IDEMPOTENT), + CHECKIN(Type.IDEMPOTENT), + CHECKOUT(Type.IDEMPOTENT), + CONNECT(Type.NORMAL), + COPY(Type.IDEMPOTENT), + DELETE(Type.IDEMPOTENT), + GET(Type.SAFE), + 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), - /** - * 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. - */ - public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit) + // Other methods + PROXY(Type.NORMAL); + + // The type of the method + private enum Type { - 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: - break; - } - return null; + NORMAL, + IDEMPOTENT, + SAFE } - /** - * 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 INSENSITIVE_CACHE = new ArrayTrie<>(); - - static - { - for (HttpMethod method : HttpMethod.values()) - { - INSENSITIVE_CACHE.put(method.toString(), method); - } - } - - public static final Trie CACHE = new ArrayTernaryTrie<>(false); - - static - { - for (HttpMethod method : HttpMethod.values()) - { - CACHE.put(method.toString(), method); - } - } - - private final ByteBuffer _buffer; + private final String _method; 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 INSENSITIVE_CACHE = new ArrayTrie<>(252); + public static final Trie CACHE = new ArrayTernaryTrie<>(false, 300); + public static final Trie 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 diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index d454d5e49eb..39d72a13be6 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/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 "))); - assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET"))); - assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("MO"))); + for (HttpMethod m : HttpMethod.values()) + { + 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 "))); - assertEquals(HttpMethod.MOVE, HttpMethod.lookAheadGet(BufferUtil.toBuffer("MOVE "))); + assertNull(HttpMethod.lookAheadGet(m.asString().substring(0,2).getBytes(), 0,2)); + 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() { diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java new file mode 100644 index 00000000000..283a866dab1 --- /dev/null +++ b/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 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(); + } +} + + diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java index a1eecc1846c..1c8b976d605 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java @@ -368,6 +368,12 @@ public class ArrayTernaryTrie extends AbstractTrie 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;