Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #448 Log Compliance violations 

Added a ComplianceHandler to HttpParsere to log compliance violations for #448
Am cautious that this may have a performance impact.
This commit is contained in:
Greg Wilkins 2016-03-22 13:58:50 +11:00
parent 36f559b483
commit 9352d91d48
4 changed files with 162 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneHandler.java
View File

@ -18,6 +18,8 @@
package org.eclipse.jetty.embedded;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.Server;
public class OneHandler
@ -25,6 +27,7 @@ public class OneHandler
public static void main( String[] args ) throws Exception
{
Server server = new Server(8080);
server.getConnectors()[0].getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.LEGACY);
server.setHandler(new HelloHandler());
server.start();

73
jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View File

@ -34,6 +34,8 @@ import org.eclipse.jetty.util.Utf8StringBuilder;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import static org.eclipse.jetty.http.HttpCompliance.LEGACY;
import static org.eclipse.jetty.http.HttpCompliance.RFC2616;
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;
@ -141,6 +143,7 @@ public class HttpParser
private final HttpHandler _handler;
private final RequestHandler _requestHandler;
private final ResponseHandler _responseHandler;
private final ComplianceHandler _complianceHandler;
private final int _maxHeaderBytes;
private final HttpCompliance _compliance;
private HttpField _field;
@ -280,6 +283,7 @@ public class HttpParser
_responseHandler=null;
_maxHeaderBytes=maxHeaderBytes;
_compliance=compliance==null?compliance():compliance;
_complianceHandler=(ComplianceHandler)(handler instanceof ComplianceHandler?handler:null);
}
/* ------------------------------------------------------------------------------- */
@ -290,8 +294,30 @@ public class HttpParser
_responseHandler=handler;
_maxHeaderBytes=maxHeaderBytes;
_compliance=compliance==null?compliance():compliance;
_complianceHandler=(ComplianceHandler)(handler instanceof ComplianceHandler?handler:null);
}
/* ------------------------------------------------------------------------------- */
protected boolean checkCompliance(HttpCompliance compliance,String reason)
{
if (_complianceHandler==null)
return _compliance.ordinal()<=compliance.ordinal();
if (_compliance.ordinal()<=compliance.ordinal())
{
_complianceHandler.onComplianceViolation(_compliance,compliance,reason);
return true;
}
return false;
}
/* ------------------------------------------------------------------------------- */
protected String legacyString(String orig, String cached)
{
System.err.printf("o=%s%n",orig);
System.err.printf("c=%s%n",cached);
return (orig.equals(cached) || !checkCompliance(LEGACY,"case sensitive"))?cached:orig;
}
/* ------------------------------------------------------------------------------- */
public long getContentLength()
{
@ -585,8 +611,8 @@ public class HttpParser
_length=_string.length();
_methodString=takeString();
HttpMethod method=HttpMethod.CACHE.get(_methodString);
if (method!=null && _compliance!=HttpCompliance.LEGACY)
_methodString=method.asString();
if (method!=null)
_methodString=legacyString(_methodString,method.asString());
setState(State.SPACE1);
}
else if (ch < SPACE)
@ -687,9 +713,8 @@ public class HttpParser
else if (ch < HttpTokens.SPACE && ch>=0)
{
// HTTP/0.9
if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
if (!checkCompliance(RFC2616,"HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
setState(State.END);
BufferUtil.clear(buffer);
@ -757,7 +782,7 @@ public class HttpParser
else
{
// HTTP/0.9
if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
if (!checkCompliance(RFC2616,"HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
@ -874,7 +899,7 @@ public class HttpParser
_host=true;
if (!(_field instanceof HostPortHttpField))
{
_field=new HostPortHttpField(_header,_compliance==HttpCompliance.LEGACY?_headerString:_header.asString(),_valueString);
_field=new HostPortHttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
add_to_connection_trie=_connectionFields!=null;
}
break;
@ -903,7 +928,7 @@ public class HttpParser
if (add_to_connection_trie && !_connectionFields.isFull() && _header!=null && _valueString!=null)
{
if (_field==null)
_field=new HttpField(_header,_compliance==HttpCompliance.LEGACY?_headerString:_header.asString(),_valueString);
_field=new HttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
_connectionFields.put(_field);
}
}
@ -960,8 +985,8 @@ public class HttpParser
case HttpTokens.SPACE:
case HttpTokens.TAB:
{
if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Continuation");
if (!checkCompliance(RFC2616,"header folding"))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding");
// header value without name - continuation?
if (_valueString==null)
@ -1062,17 +1087,17 @@ public class HttpParser
final String n;
final String v;
if (_compliance==HttpCompliance.LEGACY)
if (_compliance==LEGACY)
{
// Have to get the fields exactly from the buffer to match case
String fn=field.getName();
n=legacyString(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn);
String fv=field.getValue();
n=BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII);
if (fv==null)
v=null;
else
{
v=BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1);
v=legacyString(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv);
field=new HttpField(field.getHeader(),n,v);
}
}
@ -1164,6 +1189,22 @@ public class HttpParser
_length=_string.length();
break;
}
if (ch==HttpTokens.LINE_FEED && checkCompliance(RFC2616,"name only header"))
{
if (_headerString==null)
{
_headerString=takeString();
_header=HttpHeader.CACHE.get(_headerString);
}
_value=null;
_string.setLength(0);
_valueString="";
_length=-1;
setState(State.HEADER);
break;
}
throw new IllegalCharacterException(_state,ch,buffer);
@ -1729,6 +1770,14 @@ public class HttpParser
public boolean startResponse(HttpVersion version, int status, String reason);
}
/* ------------------------------------------------------------------------------- */
/* ------------------------------------------------------------------------------- */
/* ------------------------------------------------------------------------------- */
public interface ComplianceHandler extends HttpHandler
{
public void onComplianceViolation(HttpCompliance compliance,HttpCompliance required,String reason);
}
/* ------------------------------------------------------------------------------- */
@SuppressWarnings("serial")
private static class IllegalCharacterException extends BadMessageException

163
jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View File

@ -18,6 +18,9 @@
package org.eclipse.jetty.http;
import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertThat;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
@ -119,6 +122,7 @@ public class HttpParserTest
Assert.assertEquals("/999", _uriOrStatus);
Assert.assertEquals("HTTP/0.9", _versionOrReason);
Assert.assertEquals(-1, _headers);
assertThat(_complianceViolation,containsString("0.9"));
}
@Test
@ -130,6 +134,7 @@ public class HttpParserTest
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);
Assert.assertEquals("HTTP/0.9 not supported", _bad);
Assert.assertNull(_complianceViolation);
}
@Test
@ -146,6 +151,7 @@ public class HttpParserTest
Assert.assertEquals("/222", _uriOrStatus);
Assert.assertEquals("HTTP/0.9", _versionOrReason);
Assert.assertEquals(-1, _headers);
assertThat(_complianceViolation,containsString("0.9"));
}
@Test
@ -158,6 +164,7 @@ public class HttpParserTest
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);
Assert.assertEquals("HTTP/0.9 not supported", _bad);
Assert.assertNull(_complianceViolation);
}
@Test
@ -241,7 +248,7 @@ public class HttpParserTest
}
@Test
public void test2616Continuations() throws Exception
public void testFoldedField2616() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
@ -260,10 +267,11 @@ public class HttpParserTest
Assert.assertEquals("Name", _hdr[1]);
Assert.assertEquals("value extra", _val[1]);
Assert.assertEquals(1, _headers);
assertThat(_complianceViolation,containsString("folding"));
}
@Test
public void test7230NoContinuations() throws Exception
public void testFoldedField7230() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
@ -277,52 +285,44 @@ public class HttpParserTest
parseAll(parser, buffer);
Assert.assertThat(_bad, Matchers.notNullValue());
Assert.assertThat(_bad, Matchers.containsString("Bad Continuation"));
Assert.assertThat(_bad, Matchers.containsString("Header Folding"));
Assert.assertNull(_complianceViolation);
}
@Test
public void test7230NoWhiteSpaceInName() throws Exception
public void testWhiteSpaceInName() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
" Name: value\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, HttpCompliance.RFC7230);
parseAll(parser, buffer);
Assert.assertThat(_bad, Matchers.notNullValue());
Assert.assertThat(_bad, Matchers.containsString("Bad"));
init();
buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"N ame: value\r\n" +
"\r\n");
handler = new Handler();
parser = new HttpParser(handler);
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, 4096, HttpCompliance.RFC7230);
parseAll(parser, buffer);
Assert.assertThat(_bad, Matchers.notNullValue());
Assert.assertThat(_bad, Matchers.containsString("Illegal character"));
init();
buffer = BufferUtil.toBuffer(
}
@Test
public void testWhiteSpaceAfterName() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name : value\r\n" +
"\r\n");
handler = new Handler();
parser = new HttpParser(handler);
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, 4096, HttpCompliance.RFC7230);
parseAll(parser, buffer);
Assert.assertThat(_bad, Matchers.notNullValue());
Assert.assertThat(_bad, Matchers.containsString("Illegal character"));
}
@Test
public void testNoValue() throws Exception
{
@ -330,12 +330,11 @@ public class HttpParserTest
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name0: \r\n" +
"Name1: \r\n" +
"Connection: close\r\n" +
"Name1:\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, HttpCompliance.RFC2616);
HttpParser parser = new HttpParser(handler);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
@ -349,10 +348,54 @@ public class HttpParserTest
Assert.assertEquals("", _val[1]);
Assert.assertEquals("Name1", _hdr[2]);
Assert.assertEquals("", _val[2]);
Assert.assertEquals("Connection", _hdr[3]);
Assert.assertEquals("close", _val[3]);
Assert.assertEquals(3, _headers);
Assert.assertEquals(2, _headers);
}
@Test
public void testNoColon2616() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name\r\n" +
"Other: value\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
Assert.assertTrue(_messageCompleted);
Assert.assertEquals("GET", _methodOrVersion);
Assert.assertEquals("/", _uriOrStatus);
Assert.assertEquals("HTTP/1.0", _versionOrReason);
Assert.assertEquals("Host", _hdr[0]);
Assert.assertEquals("localhost", _val[0]);
Assert.assertEquals("Name", _hdr[1]);
Assert.assertEquals("", _val[1]);
Assert.assertEquals("Other", _hdr[2]);
Assert.assertEquals("value", _val[2]);
Assert.assertEquals(2, _headers);
assertThat(_complianceViolation,containsString("name only"));
}
@Test
public void testNoColon7230() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler,HttpCompliance.RFC7230);
parseAll(parser, buffer);
Assert.assertThat(_bad, Matchers.containsString("Illegal character"));
Assert.assertNull(_complianceViolation);
}
@Test
public void testHeaderParseDirect() throws Exception
@ -607,7 +650,7 @@ public class HttpParserTest
}
@Test
public void testNonStrict() throws Exception
public void testCaseInsensitive() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"get / http/1.0\r\n" +
@ -626,10 +669,11 @@ public class HttpParserTest
Assert.assertEquals("Connection", _hdr[1]);
Assert.assertEquals("close", _val[1]);
Assert.assertEquals(1, _headers);
Assert.assertNull(_complianceViolation);
}
@Test
public void testStrict() throws Exception
public void testCaseSensitiveLegacy() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"gEt / http/1.0\r\n" +
@ -648,6 +692,7 @@ public class HttpParserTest
Assert.assertEquals("cOnNeCtIoN", _hdr[1]);
Assert.assertEquals("ClOsE", _val[1]);
Assert.assertEquals(1, _headers);
assertThat(_complianceViolation,containsString("case sensitive"));
}
@Test
@ -1732,44 +1777,6 @@ public class HttpParserTest
Assert.assertEquals(null, _bad);
}
@Test(expected = BadMessageException.class)
public void test7230HeaderValueWithNewLine() throws Exception
{
testHeaderValueWithNewLine(HttpCompliance.RFC7230);
}
@Test
public void test2616HeaderValueWithNewLine() throws Exception
{
testHeaderValueWithNewLine(HttpCompliance.RFC2616);
}
private void testHeaderValueWithNewLine(HttpCompliance compliance) throws Exception
{
ByteBuffer buffer= BufferUtil.toBuffer(
"GET / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Header: va\r\n\tlue\r\n"+
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser= new HttpParser(handler,compliance);
parseAll(parser,buffer);
if (_bad != null)
throw new BadMessageException(_bad);
Assert.assertTrue(_headerCompleted);
Assert.assertTrue(_messageCompleted);
Assert.assertEquals("GET", _methodOrVersion);
Assert.assertEquals("/", _uriOrStatus);
Assert.assertEquals("HTTP/1.1", _versionOrReason);
Assert.assertEquals("Host",_hdr[0]);
Assert.assertEquals("localhost",_val[0]);
Assert.assertEquals("Header",_hdr[1]);
Assert.assertEquals("va lue",_val[1]);
}
@Before
public void init()
{
@ -1783,6 +1790,7 @@ public class HttpParserTest
_headers = 0;
_headerCompleted = false;
_messageCompleted = false;
_complianceViolation = null;
}
private String _host;
@ -1799,8 +1807,9 @@ public class HttpParserTest
private boolean _early;
private boolean _headerCompleted;
private boolean _messageCompleted;
private String _complianceViolation;
private class Handler implements HttpParser.RequestHandler, HttpParser.ResponseHandler
private class Handler implements HttpParser.RequestHandler, HttpParser.ResponseHandler, HttpParser.ComplianceHandler
{
private HttpFields fields;
@ -1904,5 +1913,11 @@ public class HttpParserTest
{
return 512;
}
@Override
public void onComplianceViolation(HttpCompliance compliance, HttpCompliance required, String reason)
{
_complianceViolation=reason;
}
}
}

10
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
View File

@ -24,6 +24,7 @@ import java.nio.ByteBuffer;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HostPortHttpField;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpGenerator;
@ -43,7 +44,7 @@ import org.eclipse.jetty.util.log.Logger;
/**
* A HttpChannel customized to be transported over the HTTP/1 protocol
*/
class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandler
public class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandler, HttpParser.ComplianceHandler
{
private static final Logger LOG = Log.getLogger(HttpChannelOverHttp.class);
private final static HttpField PREAMBLE_UPGRADE_H2C = new HttpField(HttpHeader.UPGRADE,"h2c");
@ -451,4 +452,11 @@ class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandl
{
return getHttpConfiguration().getHeaderCacheSize();
}
@Override
public void onComplianceViolation(HttpCompliance compliance,HttpCompliance required, String reason)
{
if (LOG.isDebugEnabled())
LOG.debug(compliance+"<="+required+": "+reason+" for "+getHttpTransport());
}
}