Merge remote-tracking branch 'origin/jetty-9.2.x' into jetty-9.3.x

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-05-17 08:16:39 +10:00
commit be8ff431a4
6 changed files with 286 additions and 39 deletions

View File

@ -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: <UL>
@ -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<buffer.limit())
{
@ -787,12 +788,14 @@ public class HttpParser
{
_cr=true;
_version=version;
checkVersion();
_string.setLength(0);
buffer.position(pos+1);
}
else if (n==HttpTokens.LINE_FEED)
{
_version=version;
checkVersion();
_string.setLength(0);
buffer.position(pos);
}
@ -831,8 +834,7 @@ public class HttpParser
_length=_string.length();
_version=HttpVersion.CACHE.get(takeString());
}
if (_version==null)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Unknown Version");
checkVersion();
// Should we try to cache header fields?
if (_connectionFields==null && _version.getVersion()>=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;

View File

@ -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);

View File

@ -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);
}
}
}

View File

@ -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<clen.length; n++)
{
request.append("Content-Length: ").append(Integer.toString(clen[n])).append("\r\n");
}
request.append("Content-Type: text/plain\r\n");
request.append("Connection: close\r\n");
request.append("\r\n");
request.append("abcdefgh"); // actual content of 8 bytes
String rawResponses = connector.getResponses(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponses);
assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_BAD_REQUEST));
}
}
/**
* More then 1 Content-Length is a bad requests per HTTP rfcs.
*/
@Test
public void testHttp11_ContentLengthAndChunk() throws Exception
{
HttpParser.LOG.info("badMessage: 400 Bad messages EXPECTED...");
int contentLengths[][]= {
{-1,8},
{8,-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<clen.length; n++)
{
if (clen[n]==-1)
request.append("Transfer-Encoding: chunked\r\n");
else
request.append("Content-Length: ").append(Integer.toString(clen[n])).append("\r\n");
}
request.append("Content-Type: text/plain\r\n");
request.append("Connection: close\r\n");
request.append("\r\n");
request.append("8;\r\n"); // chunk header
request.append("abcdefgh"); // actual content of 8 bytes
request.append("\r\n0;\r\n"); // last chunk
String rawResponses = connector.getResponses(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponses);
assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_BAD_REQUEST));
}
}
@Test
public void testNoPath() throws Exception

View File

@ -244,6 +244,39 @@ public class PartialRFC2616Test
assertEquals("Quality parameters","ccc",HttpFields.valueParameters(list.get(4),null));
assertEquals("Quality parameters","ddd",HttpFields.valueParameters(list.get(5),null));
}
@Test
public void test4_1() throws Exception
{
int offset=0;
// If _content length not used, second request will not be read.
String response = connector.getResponses(
"\r\n" +
"GET /R1 HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"\r\n" +
"\r\n" +
"\r\n" +
"\r\n" +
"GET /R2 HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"\r\n" +
" \r\n" +
"GET /R3 HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Connection: close\r\n" +
"\r\n"
);
offset=checkContains(response,offset,"HTTP/1.1 200 OK","2. identity")+10;
offset=checkContains(response,offset,"/R1","2. identity")+3;
offset=checkContains(response,offset,"HTTP/1.1 200 OK","2. identity")+10;
offset=checkContains(response,offset,"/R2","2. identity")+3;
checkNotContained(response,offset,"HTTP/1.1 200 OK","2. identity");
checkNotContained(response,offset,"/R3","2. identity");
}
@Test
public void test4_4_2() throws Exception
@ -273,8 +306,8 @@ public class PartialRFC2616Test
@Test
public void test4_4_3() throws Exception
{
// _content length is ignored, as chunking is used. If it is
// not ignored, the second request wont be seen.
// Due to smuggling concerns, handling has been changed to
// treat content length and chunking as a bad request.
int offset=0;
String response = connector.getResponses(
"GET /R1 HTTP/1.1\n" +
@ -297,12 +330,8 @@ public class PartialRFC2616Test
"Content-Length: 6\n" +
"\n" +
"abcdef");
offset=checkContains(response,offset,"HTTP/1.1 200 OK","3. ignore c-l")+1;
offset=checkContains(response,offset,"/R1","3. ignore c-l")+1;
offset=checkContains(response,offset,"123456","3. ignore c-l")+1;
offset=checkContains(response,offset,"HTTP/1.1 200 OK","3. ignore c-l")+1;
offset=checkContains(response,offset,"/R2","3. _content-length")+1;
offset=checkContains(response,offset,"abcdef","3. _content-length")+1;
offset=checkContains(response,offset,"HTTP/1.1 400 Bad","3. ignore c-l")+1;
checkNotContained(response,offset,"/R2","3. _content-length");
}
@Test

View File

@ -18,12 +18,12 @@
package org.eclipse.jetty.test.rfcs;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.matchers.JUnitMatchers.containsString;
import java.io.File;
import java.io.IOException;
@ -384,7 +384,7 @@ public abstract class RFC2616BaseTest
// 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");
@ -409,14 +409,10 @@ public abstract class RFC2616BaseTest
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.