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 4597028330a..914a4068256 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
@@ -93,6 +93,7 @@ public class HttpParser
@Deprecated
public final static String __STRICT="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:
@@ -166,6 +167,7 @@ public class HttpParser
private HttpVersion _version;
private Utf8StringBuilder _uri=new Utf8StringBuilder(INITIAL_URI_LENGTH); // Tune?
private EndOfContent _endOfContent;
+ private boolean _hasContentLength;
private long _contentLength;
private long _contentPosition;
private int _chunkLength;
@@ -552,8 +554,8 @@ public class HttpParser
}
else if (ch==0)
break;
- else if (ch<0)
- throw new BadMessageException();
+ else if (ch!='\n')
+ throw new BadMessageException("Bad preamble");
// count this white space as a header byte to avoid DOS
if (_maxHeaderBytes>0 && ++_headerBytes>_maxHeaderBytes)
@@ -662,8 +664,7 @@ public class HttpParser
_length=_string.length();
String version=takeString();
_version=HttpVersion.CACHE.get(version);
- if (_version==null)
- throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Unknown Version");
+ checkVersion();
setState(State.SPACE1);
}
else if (ch < HttpTokens.SPACE)
@@ -778,7 +779,7 @@ public class HttpParser
version=HttpVersion.CACHE.getBest(buffer,0,buffer.remaining());
if (version!=null)
- {
+ {
int pos = buffer.position()+version.asString().length()-1;
if (pos=HttpVersion.HTTP_1_1.getVersion() && _handler.getHeaderCacheSize()>0)
@@ -880,6 +882,15 @@ public class HttpParser
return handle;
}
+ private void checkVersion()
+ {
+ if (_version==null)
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Unknown Version");
+
+ if (_version.getVersion()<10 || _version.getVersion()>20)
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Version");
+ }
+
private void parsedHeader()
{
// handler last header if any. Delayed to here just in case there was a continuation line (above)
@@ -892,11 +903,14 @@ public class HttpParser
switch (_header)
{
case CONTENT_LENGTH:
- if (_endOfContent == EndOfContent.CONTENT_LENGTH)
- {
- throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Duplicate Content-Length");
- }
- else if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
+ if (_hasContentLength && complianceViolation(RFC7230,"Duplicate Content-Lengths"))
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Duplicate Content-Lengths");
+ _hasContentLength = true;
+
+ if (_endOfContent == EndOfContent.CHUNKED_CONTENT && complianceViolation(RFC7230,"Chunked and Content-Length"))
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Content-Length");
+
+ if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
{
_contentLength=convertContentLength(_valueString);
if (_contentLength <= 0)
@@ -919,6 +933,11 @@ public class HttpParser
else if (_valueString.contains(HttpHeaderValue.CHUNKED.toString()))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad chunking");
}
+
+ if (_hasContentLength && _endOfContent==EndOfContent.CHUNKED_CONTENT && complianceViolation(RFC7230,"Chunked and Content-Length"))
+ throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Chunked and Content-Length");
+
+
break;
case HOST:
@@ -1559,9 +1578,16 @@ public class HttpParser
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.PAYLOAD_TOO_LARGE_413);
+
_chunkLength=_chunkLength * 16 + TypeUtil.convertHexDigit(ch);
+ }
break;
}
@@ -1687,6 +1713,7 @@ public class HttpParser
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 b9673cee6d7..341fe52c037 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
@@ -210,6 +210,32 @@ public class HttpParserTest
Assert.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);
+ Assert.assertEquals("GET", _methodOrVersion);
+ Assert.assertEquals("/", _uriOrStatus);
+ Assert.assertEquals("HTTP/1.0", _versionOrReason);
+ Assert.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);
+ Assert.assertEquals("Bad preamble", _bad);
+ }
+
+
@Test
public void testConnect() throws Exception
{
@@ -1546,7 +1572,7 @@ public class HttpParserTest
parser.parseNext(buffer);
Assert.assertEquals("POST", _methodOrVersion);
- Assert.assertEquals("Duplicate Content-Length", _bad);
+ Assert.assertEquals("Duplicate Content-Lengths", _bad);
Assert.assertFalse(buffer.hasRemaining());
Assert.assertEquals(HttpParser.State.CLOSE, parser.getState());
parser.atEOF();
@@ -1570,7 +1596,7 @@ public class HttpParserTest
parser.parseNext(buffer);
Assert.assertEquals("POST", _methodOrVersion);
- Assert.assertEquals("Duplicate Content-Length", _bad);
+ Assert.assertEquals("Duplicate Content-Lengths", _bad);
Assert.assertFalse(buffer.hasRemaining());
Assert.assertEquals(HttpParser.State.CLOSE, parser.getState());
parser.atEOF();
@@ -1593,7 +1619,7 @@ public class HttpParserTest
+ "\r\n");
HttpParser.RequestHandler handler = new Handler();
- HttpParser parser = new HttpParser(handler);
+ HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616);
parseAll(parser, buffer);
Assert.assertEquals("POST", _methodOrVersion);
@@ -1620,7 +1646,7 @@ public class HttpParserTest
+ "\r\n");
HttpParser.RequestHandler handler = new Handler();
- HttpParser parser = new HttpParser(handler);
+ HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616);
parseAll(parser, buffer);
Assert.assertEquals("POST", _methodOrVersion);
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 e1857ecc721..2ee5a576971 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
@@ -681,7 +681,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
{
action=_state.handling();
}
- catch(IllegalStateException e)
+ catch(Throwable e)
{
// The bad message cannot be handled in the current state, so throw
// to hopefull somebody that can handle
@@ -709,12 +709,15 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
finally
{
- // TODO: review whether it's the right state to check.
- if (_state.unhandle()==Action.COMPLETE)
- _state.onComplete();
- else
- throw new IllegalStateException(); // TODO: don't throw from finally blocks !
- onCompleted();
+ try
+ {
+ onCompleted();
+ }
+ catch(Throwable e)
+ {
+ LOG.debug(e);
+ abort(e);
+ }
}
}
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 33a5e49c260..629dec650f8 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,7 +24,12 @@
*/
package org.eclipse.jetty.server;
+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;
@@ -45,6 +50,7 @@ import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpCompliance;
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;
@@ -135,6 +141,166 @@ public class HttpConnectionTest
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
+ {
+ connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616);
+ // 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
+ {
+ connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616);
+ // 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 testHttp11_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; n