Issue #2529 RFC2616 vs RFC7230 cleanup

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-05-17 08:00:53 +10:00
parent d7ce334f8f
commit a285deea42
6 changed files with 315 additions and 79 deletions

View File

@ -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.ArrayTernaryTrie;
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: <UL>
@ -147,6 +150,7 @@ public class HttpParser
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 @@ public class HttpParser
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 @@ public class HttpParser
* 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 @@ public class HttpParser
}
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 @@ public class HttpParser
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())
{
@ -743,9 +755,11 @@ public class HttpParser
}
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);
@ -797,22 +811,27 @@ public class HttpParser
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 @@ public class HttpParser
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 @@ public class HttpParser
}
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 @@ 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.REQUEST_ENTITY_TOO_LARGE_413);
_chunkLength=_chunkLength * 16 + TypeUtil.convertHexDigit(ch);
}
break;
}
@ -1547,6 +1575,7 @@ public class HttpParser
setState(State.START);
_endOfContent=EndOfContent.UNKNOWN_CONTENT;
_contentLength=-1;
_hasContentLength=false;
_contentPosition=0;
_responseStatus=0;
_contentChunk=null;

View File

@ -180,6 +180,32 @@ public class HttpParserTest
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
@ -453,7 +479,7 @@ public class HttpParserTest
HttpParser parser= new HttpParser(handler);
parseAll(parser,buffer);
assertThat(_bad,Matchers.notNullValue());
}
}
@Test
public void testHeaderTab() throws Exception
@ -1545,13 +1571,7 @@ public class HttpParserTest
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

View File

@ -729,17 +729,13 @@ public class HttpChannel<T> implements HttpParser.RequestHandler<T>, Runnable, H
sendResponse(new ResponseInfo(HttpVersion.HTTP_1_1,fields,0,status,reason,false),content ,true);
}
}
catch (IOException e)
{
LOG.debug(e);
}
finally
{
if (_state.unhandle()==Action.COMPLETE)
_state.completed();
else
throw new IllegalStateException();
}
catch (Throwable e)
{
LOG.debug(e);
_transport.abort();
}
}

View File

@ -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.Assert;
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 class HttpConnectionTest
{
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 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
{
// 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; 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

@ -248,6 +248,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
@ -277,8 +310,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" +
@ -301,12 +334,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,15 +18,11 @@
package org.eclipse.jetty.test.rfcs;
import static org.junit.Assert.*;
import static org.junit.matchers.JUnitMatchers.*;
import java.io.File;
import java.io.IOException;
import java.net.Socket;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Collections;
import java.util.Date;
import java.util.Enumeration;
import java.util.List;
@ -42,7 +38,6 @@ import org.eclipse.jetty.test.support.rawhttp.HttpTesting;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.StringAssert;
import org.eclipse.jetty.util.MultiPartInputStreamParser;
import org.hamcrest.Matchers;
import org.junit.AfterClass;
import org.junit.Assert;
@ -50,6 +45,11 @@ import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
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;
/**
* <a href="http://tools.ietf.org/html/rfc2616">RFC 2616</a> (HTTP/1.1) Test Case
*/
@ -380,7 +380,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");
@ -405,14 +405,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.