diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
index 3a0e2f4319c9..29d3cc9826bc 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
@@ -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;
@@ -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
@@ -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:
@@ -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;
@@ -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;
}
}
@@ -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);
@@ -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
@@ -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=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);
@@ -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:
@@ -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:
@@ -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;
@@ -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;
}
@@ -1547,6 +1575,7 @@ public void reset()
setState(State.START);
_endOfContent=EndOfContent.UNKNOWN_CONTENT;
_contentLength=-1;
+ _hasContentLength=false;
_contentPosition=0;
_responseStatus=0;
_contentChunk=null;
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 c5a42d1e5041..b1404db31cfa 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
@@ -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 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 handler = new Handler();
+ HttpParser parser= new HttpParser(handler);
+ parseAll(parser,buffer);
+ assertEquals("400", _bad);
+ }
+
@Test
public void testConnect() throws Exception
@@ -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
@@ -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
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index 0914bdede599..40e04ab76467 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -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();
}
}
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
index b4726ea54d9c..b34a3c0a2b3f 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
@@ -24,8 +24,23 @@
*/
package org.eclipse.jetty.server;
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.StringReader;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpParser;
+import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
@@ -38,20 +53,12 @@
import org.junit.Before;
import org.junit.Test;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.io.PrintWriter;
-import java.io.StringReader;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.isEmptyOrNullString;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@@ -68,13 +75,14 @@ public void init() throws Exception
{
server = new Server();
- HttpConnectionFactory http = new HttpConnectionFactory();
- http.getHttpConfiguration().setRequestHeaderSize(1024);
- http.getHttpConfiguration().setResponseHeaderSize(1024);
-
+ HttpConfiguration config = new HttpConfiguration();
+ config.setRequestHeaderSize(1024);
+ config.setResponseHeaderSize(1024);
+ config.setSendDateHeader(true);
+ HttpConnectionFactory http = new HttpConnectionFactory(config);
+
connector = new LocalConnector(server,http,null);
connector.setIdleTimeout(5000);
- connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setSendDateHeader(true);
server.addConnector(connector);
server.setHandler(new DumpHandler());
ErrorHandler eh=new ErrorHandler();
@@ -134,6 +142,164 @@ public void testFragmentedChunk() throws Exception
throw e;
}
}
+
+ /**
+ * HTTP/0.9 does not support HttpVersion (this is a bad request)
+ */
+ @Test
+ public void testHttp09_NoVersion() throws Exception
+ {
+ String request = "GET / HTTP/0.9\r\n\r\n";
+ String response = connector.getResponses(request);
+ assertThat(response, containsString("400 Bad Version"));
+ }
+
+ /**
+ * HTTP/0.9 does not support headers
+ */
+ @Test
+ public void testHttp09_NoHeaders() throws Exception
+ {
+ // header looking like another request is ignored
+ String request = "GET /one\r\nGET :/two\r\n\r\n";
+ String response = connector.getResponses(request);
+ assertThat(response, containsString("pathInfo=/"));
+ assertThat(response, not(containsString("two")));
+ }
+
+ /**
+ * Http/0.9 does not support pipelining.
+ */
+ @Test
+ public void testHttp09_MultipleRequests() throws Exception
+ {
+ // Verify that LocalConnector supports pipelining with HTTP/1.1.
+ String requests = "GET /?id=123 HTTP/1.1\r\nHost: localhost\r\n\r\nGET /?id=456 HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n";
+ String responses = connector.getResponses(requests);
+ assertThat(responses, containsString("id=123"));
+ assertThat(responses, containsString("id=456"));
+
+ // Verify that pipelining does not work with HTTP/0.9.
+ requests = "GET /?id=123\r\n\r\nGET /?id=456\r\n\r\nGET /?id=789\r\n\r\n";
+ responses = connector.getResponses(requests);
+ assertThat(responses, containsString("id=123"));
+ assertThat(responses, not(containsString("id=456")));
+ assertThat(responses, not(containsString("id=789")));
+ }
+
+ /**
+ * Ensure that excessively large hexadecimal chunk body length is parsed properly.
+ */
+ @Test
+ public void testHttp10_ChunkedBodyTruncation() throws Exception
+ {
+ String request = "POST /?id=123 HTTP/1.1\r\n" +
+ "Host: local\r\n" +
+ "Transfer-Encoding: chunked\r\n" +
+ "Content-Type: text/plain\r\n" +
+ "Connection: close\r\n" +
+ "\r\n" +
+ "1ff00000008\r\n" +
+ "abcdefgh\r\n" +
+ "\r\n" +
+ "0\r\n" +
+ "\r\n" +
+ "POST /?id=bogus HTTP/1.1\r\n" +
+ "Content-Length: 5\r\n" +
+ "Host: dummy-host.example.com\r\n" +
+ "\r\n" +
+ "12345";
+ String responses = connector.getResponses(request);
+
+ assertThat(responses,anyOf(
+ isEmptyOrNullString(),
+ containsString(" 413 "),
+ containsString(" 500 ")
+ ));
+ }
+
+ /**
+ * More then 1 Content-Length is a bad requests per HTTP rfcs.
+ */
+ @Test
+ public void testHttp11_MultipleContentLength() throws Exception
+ {
+ HttpParser.LOG.info("badMessage: 400 Bad messages EXPECTED...");
+ int contentLengths[][]= {
+ {0,8},
+ {8,0},
+ {8,8},
+ {0,8,0},
+ {1,2,3,4,5,6,7,8},
+ {8,2,1},
+ {0,0},
+ {8,0,8},
+ {-1,8},
+ {8,-1},
+ {-1,8,-1},
+ {-1,-1},
+ {8,-1,8},
+ };
+
+ for(int x = 0; x < contentLengths.length; x++)
+ {
+ StringBuilder request = new StringBuilder();
+ request.append("POST /?id=").append(Integer.toString(x)).append(" HTTP/1.1\r\n");
+ request.append("Host: local\r\n");
+ int clen[] = contentLengths[x];
+ for(int n = 0; nRFC 2616 (HTTP/1.1) Test Case
*/
@@ -380,7 +380,7 @@ public void test4_4() throws Exception
// 4.4.3 -
// Client - do not send 'Content-Length' if entity-length
// and the transfer-length are different.
- // Server - ignore 'Content-Length' if 'Transfer-Encoding' is provided.
+ // Server - bad message to avoid smuggling concerns
StringBuffer req2 = new StringBuffer();
req2.append("GET /echo/R1 HTTP/1.1\n");
@@ -405,14 +405,10 @@ public void test4_4() throws Exception
req2.append("7890AB");
responses = http.requests(req2);
- Assert.assertEquals("Response Count",2,responses.size());
+ Assert.assertEquals("Response Count",1,responses.size());
response = responses.get(0); // response 1
- assertEquals("4.4.3 Ignore Content-Length / Response Code", HttpStatus.OK_200, response.getStatus());
- assertTrue("4.4.3 Ignore Content-Length / Body", response.getContent().contains("123456\n"));
- response = responses.get(1); // response 2
- assertEquals("4.4.3 Ignore Content-Length / Response Code", HttpStatus.OK_200, response.getStatus());
- assertTrue("4.4.3 Ignore Content-Length / Body", response.getContent().contains("7890AB\n"));
+ assertEquals("4.4.3 Ignore Content-Length / Response Code", HttpStatus.BAD_REQUEST_400, response.getStatus());
// 4.4 - Server can request valid Content-Length from client if client
// fails to provide a Content-Length.