Skip to content

Commit

Permalink
Fix #5575 SEARCH method (#5576)
Browse files Browse the repository at this point in the history 
 + 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>
  • Loading branch information
@gregw @joakime
gregw and joakime committed Nov 11, 2020
1 parent 6b82637 commit bb886ad
Show file tree
Hide file tree
Showing 4 changed files with 321 additions and 128 deletions.
284 changes: 162 additions & 122 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java
View file
Expand Up @@ -26,162 +26,202 @@
import org.eclipse.jetty.util.Trie;

/**
*
* Known HTTP Methods
*/
public enum HttpMethod
{
GET,
POST,
HEAD,
PUT,
OPTIONS,
DELETE,
TRACE,
CONNECT,
MOVE,
PROXY,
PRI;

/**
* 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)
// 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),

// 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());
private final String _method;
private final byte[] _bytes;
private final ByteBuffer _buffer;
private final Type _type;

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;
HttpMethod(Type type)
{
_method = name().replace('_', '-');
_type = type;
_bytes = StringUtil.getBytes(_method);
_buffer = ByteBuffer.wrap(_bytes);
}

public static final Trie<HttpMethod> INSENSITIVE_CACHE = new ArrayTrie<>();
public byte[] getBytes()
{
return _bytes;
}

static
public boolean is(String s)
{
for (HttpMethod method : HttpMethod.values())
{
INSENSITIVE_CACHE.put(method.toString(), method);
}
return toString().equalsIgnoreCase(s);
}

public static final Trie<HttpMethod> CACHE = new ArrayTernaryTrie<>(false);
/**
* 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;
}

static
/**
* 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()
{
for (HttpMethod method : HttpMethod.values())
{
CACHE.put(method.toString(), method);
}
return _type.ordinal() >= Type.IDEMPOTENT.ordinal();
}

private final ByteBuffer _buffer;
private final byte[] _bytes;
public ByteBuffer asBuffer()
{
return _buffer.asReadOnlyBuffer();
}

HttpMethod()
public String asString()
{
_bytes = StringUtil.getBytes(toString());
_buffer = ByteBuffer.wrap(_bytes);
return _method;
}

public byte[] getBytes()
public String toString()
{
return _bytes;
return _method;
}

public boolean is(String s)
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
{
return toString().equalsIgnoreCase(s);
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);
}
}

public ByteBuffer asBuffer()
/**
* 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 _buffer.asReadOnlyBuffer();
return LOOK_AHEAD.getBest(bytes, position, limit - position);
}

public String asString()
/**
* 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)
{
return toString();
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
* 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
Expand Down
32 changes: 26 additions & 6 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View file
Expand Up @@ -23,6 +23,7 @@
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;
Expand All @@ -32,6 +33,8 @@
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;
Expand Down Expand Up @@ -85,12 +88,20 @@ public static void parseAll(HttpParser parser, ByteBuffer buffer)
@Test
public void httpMethodTest()
{
assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("Wibble ")));
assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET")));
assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("MO")));

assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET ")));
assertEquals(HttpMethod.MOVE, HttpMethod.lookAheadGet(BufferUtil.toBuffer("MOVE ")));
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")));

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"));
Expand All @@ -100,6 +111,15 @@ public void httpMethodTest()
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()
{
Expand Down

0 comments on commit bb886ad

Please sign in to comment.