Skip to content

Commit

Permalink
Issue #2529 RFC2616 vs RFC7230 cleanup
Browse files Browse the repository at this point in the history 
Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
@gregw
gregw committed May 16, 2018
1 parent d7ce334 commit a285dee
Show file tree
Hide file tree
Showing 6 changed files with 314 additions and 78 deletions.
77 changes: 53 additions & 24 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Expand Up @@ -18,8 +18,6 @@

package org.eclipse.jetty.http;

import static org.eclipse.jetty.http.HttpTokens.*;

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;

Expand All @@ -28,12 +26,16 @@
import org.eclipse.jetty.util.ArrayTrie;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.Trie;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;

import static org.eclipse.jetty.http.HttpTokens.CARRIAGE_RETURN;
import static org.eclipse.jetty.http.HttpTokens.LINE_FEED;
import static org.eclipse.jetty.http.HttpTokens.SPACE;
import static org.eclipse.jetty.http.HttpTokens.TAB;


/* ------------------------------------------------------------ */
/** A Parser for HTTP 0.9, 1.0 and 1.1
Expand Down Expand Up @@ -80,6 +82,7 @@ public class HttpParser
public static final Logger LOG = Log.getLogger(HttpParser.class);
public final static boolean __STRICT=Boolean.getBoolean("org.eclipse.jetty.http.HttpParser.STRICT");
public final static int INITIAL_URI_LENGTH=256;
private final static int MAX_CHUNK_LENGTH=Integer.MAX_VALUE/16-16;

/**
* Cache of common {@link HttpField}s including: <UL>
Expand Down Expand Up @@ -147,6 +150,7 @@ public enum State
private HttpVersion _version;
private ByteBuffer _uri=ByteBuffer.allocate(INITIAL_URI_LENGTH); // Tune?
private EndOfContent _endOfContent;
private boolean _hasContentLength;
private long _contentLength;
private long _contentPosition;
private int _chunkLength;
Expand Down Expand Up @@ -355,7 +359,12 @@ private BadMessageException(String message)

private BadMessageException(int code,String message)
{
super(message);
this(code,message,null);
}

private BadMessageException(int code,String message,Throwable cause)
{
super(message, cause);
_code=code;
}
}
Expand Down Expand Up @@ -406,7 +415,7 @@ else if (!(ch==LINE_FEED || ch==TAB))
* otherwise skip white space until something else to parse.
*/
private boolean quickStart(ByteBuffer buffer)
{
{
if (_requestHandler!=null)
{
_method = HttpMethod.lookAheadGet(buffer);
Expand Down Expand Up @@ -444,7 +453,7 @@ else if (_responseHandler!=null)
}
else if (ch==0)
break;
else if (ch<0)
else if (ch!='\n')
throw new BadMessageException();

// count this white space as a header byte to avoid DOS
Expand Down Expand Up @@ -687,8 +696,11 @@ else if (ch < HttpTokens.SPACE && ch>=0)
return false;
}
}
else
else
{
if (version!=HttpVersion.HTTP_1_0 && version!=HttpVersion.HTTP_1_1)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Version");

int pos = buffer.position()+version.asString().length()-1;
if (pos<buffer.limit())
{
Expand Down Expand Up @@ -743,9 +755,11 @@ else if (ch<0)
}
if (_version==null)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Unknown Version");
if (_version!=HttpVersion.HTTP_1_0 && _version!=HttpVersion.HTTP_1_1)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Version");

// Should we try to cache header fields?
if (_connectionFields==null && _version.getVersion()>=HttpVersion.HTTP_1_1.getVersion())
if (_connectionFields==null && _version.getVersion()==HttpVersion.HTTP_1_1.getVersion())
{
int header_cache = _handler.getHeaderCacheSize();
_connectionFields=new ArrayTernaryTrie<>(header_cache);
Expand Down Expand Up @@ -797,22 +811,27 @@ private boolean handleKnownHeaders(ByteBuffer buffer)
switch (_header)
{
case CONTENT_LENGTH:
if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
if (_hasContentLength)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Content-Lengths");
_hasContentLength = true;

if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Content-Length");

try
{
try
{
_contentLength=Long.parseLong(_valueString);
}
catch(NumberFormatException e)
{
LOG.ignore(e);
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Content-Length");
}
if (_contentLength <= 0)
_endOfContent=EndOfContent.NO_CONTENT;
else
_endOfContent=EndOfContent.CONTENT_LENGTH;
_contentLength=Long.parseLong(_valueString);
}
catch(NumberFormatException e)
{
LOG.ignore(e);
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Content-Length");
}
if (_contentLength <= 0)
_endOfContent=EndOfContent.NO_CONTENT;
else
_endOfContent=EndOfContent.CONTENT_LENGTH;

break;

case TRANSFER_ENCODING:
Expand All @@ -827,6 +846,10 @@ else if (_valueString.contains(HttpHeaderValue.CHUNKED.toString()))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad chunking");
}
}

if (_hasContentLength && _endOfContent==EndOfContent.CHUNKED_CONTENT)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad chunking");

break;

case HOST:
Expand All @@ -846,8 +869,7 @@ else if (_valueString.contains(HttpHeaderValue.CHUNKED.toString()))
}
catch (final Exception e)
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Host header")
{{initCause(e);}};
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Host header",e);
}

break;
Expand Down Expand Up @@ -1433,9 +1455,15 @@ protected boolean parseContent(ByteBuffer buffer)
setState(State.CHUNK);
}
else if (ch <= HttpTokens.SPACE || ch == HttpTokens.SEMI_COLON)
{
setState(State.CHUNK_PARAMS);
}
else
{
if (_chunkLength>MAX_CHUNK_LENGTH)
throw new BadMessageException(HttpStatus.REQUEST_ENTITY_TOO_LARGE_413);
_chunkLength=_chunkLength * 16 + TypeUtil.convertHexDigit(ch);
}
break;
}

Expand Down Expand Up @@ -1547,6 +1575,7 @@ public void reset()
setState(State.START);
_endOfContent=EndOfContent.UNKNOWN_CONTENT;
_contentLength=-1;
_hasContentLength=false;
_contentPosition=0;
_responseStatus=0;
_contentChunk=null;
Expand Down
36 changes: 28 additions & 8 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View file
Expand Up @@ -180,6 +180,32 @@ public void testLongURLParse() throws Exception
assertEquals("HTTP/1.0", _versionOrReason);
assertEquals(-1, _headers);
}

@Test
public void testAllowedLinePreamble() throws Exception
{
ByteBuffer buffer= BufferUtil.toBuffer("\r\n\r\nGET / HTTP/1.0\r\n");

HttpParser.RequestHandler<ByteBuffer> handler = new Handler();
HttpParser parser= new HttpParser(handler);
parseAll(parser,buffer);
assertEquals("GET", _methodOrVersion);
assertEquals("/", _uriOrStatus);
assertEquals("HTTP/1.0", _versionOrReason);
assertEquals(-1, _headers);
}

@Test
public void testDisallowedLinePreamble() throws Exception
{
ByteBuffer buffer= BufferUtil.toBuffer("\r\n \r\nGET / HTTP/1.0\r\n");

HttpParser.RequestHandler<ByteBuffer> handler = new Handler();
HttpParser parser= new HttpParser(handler);
parseAll(parser,buffer);
assertEquals("400", _bad);
}


@Test
public void testConnect() throws Exception
Expand Down Expand Up @@ -453,7 +479,7 @@ public void testBadHeaderEncoding() throws Exception
HttpParser parser= new HttpParser(handler);
parseAll(parser,buffer);
assertThat(_bad,Matchers.notNullValue());
}
}

@Test
public void testHeaderTab() throws Exception
Expand Down Expand Up @@ -1545,13 +1571,7 @@ public void testHTTP2Preface() throws Exception
HttpParser parser= new HttpParser(handler);
parseAll(parser,buffer);

assertTrue(_headerCompleted);
assertTrue(_messageCompleted);
assertEquals("PRI", _methodOrVersion);
assertEquals("*", _uriOrStatus);
assertEquals("HTTP/2.0", _versionOrReason);
assertEquals(-1, _headers);
assertEquals(null, _bad);
assertEquals("Bad Version", _bad);
}

@Before
Expand Down
12 changes: 4 additions & 8 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View file
Expand Up @@ -729,17 +729,13 @@ public void badMessage(int status, String reason)

sendResponse(new ResponseInfo(HttpVersion.HTTP_1_1,fields,0,status,reason,false),content ,true);
}
if (_state.unhandle()==Action.COMPLETE)
_state.completed();
}
catch (IOException e)
catch (Throwable e)
{
LOG.debug(e);
}
finally
{
if (_state.unhandle()==Action.COMPLETE)
_state.completed();
else
throw new IllegalStateException();
_transport.abort();
}
}

Expand Down

0 comments on commit a285dee

Please sign in to comment.