Issue #448 - RFC2616 Compliance Mode should track and report RFC7230 violations

Improved compliance level reporting
Improved method names to make code read better
This commit is contained in:
Greg Wilkins 2016-03-23 14:42:05 +11:00
parent 7f96db72c4
commit 4ae077f2b6
4 changed files with 20 additions and 64 deletions

View File

@ -36,6 +36,7 @@ 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.HttpCompliance.RFC7230;
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;
@ -298,22 +299,27 @@ public class HttpParser
}
/* ------------------------------------------------------------------------------- */
protected boolean checkCompliance(HttpCompliance compliance,String reason)
/** Check RFC compliance violation
* @param compliance The compliance level violated
* @param reason The reason for the violation
* @return True if the current compliance level is set so as to Not allow this violation
*/
protected boolean complianceViolation(HttpCompliance compliance,String reason)
{
if (_complianceHandler==null)
return _compliance.ordinal()<=compliance.ordinal();
if (_compliance.ordinal()<=compliance.ordinal())
return _compliance.ordinal()>=compliance.ordinal();
if (_compliance.ordinal()<compliance.ordinal())
{
_complianceHandler.onComplianceViolation(_compliance,compliance,reason);
return true;
return false;
}
return false;
return true;
}
/* ------------------------------------------------------------------------------- */
protected String legacyString(String orig, String cached)
{
return (orig.equals(cached) || !checkCompliance(LEGACY,"case sensitive"))?cached:orig;
return (orig.equals(cached) || complianceViolation(RFC2616,"case sensitive"))?cached:orig;
}
/* ------------------------------------------------------------------------------- */
@ -711,7 +717,7 @@ public class HttpParser
else if (ch < HttpTokens.SPACE && ch>=0)
{
// HTTP/0.9
if (!checkCompliance(RFC2616,"HTTP/0.9"))
if (complianceViolation(RFC7230,"HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
setState(State.END);
@ -780,7 +786,7 @@ public class HttpParser
else
{
// HTTP/0.9
if (!checkCompliance(RFC2616,"HTTP/0.9"))
if (complianceViolation(RFC7230,"HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
@ -983,7 +989,7 @@ public class HttpParser
case HttpTokens.SPACE:
case HttpTokens.TAB:
{
if (!checkCompliance(RFC2616,"header folding"))
if (complianceViolation(RFC7230,"header folding"))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding");
// header value without name - continuation?
@ -1188,7 +1194,7 @@ public class HttpParser
break;
}
if (ch==HttpTokens.LINE_FEED && checkCompliance(RFC2616,"name only header"))
if (ch==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"name only header"))
{
if (_headerString==null)
{

View File

@ -350,56 +350,6 @@ public class HttpParserTest
Assert.assertEquals("", _val[2]);
Assert.assertEquals(2, _headers);
}
@Test
public void testEmptyQuotedValue2616() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name0: \"\"\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("Name0", _hdr[1]);
Assert.assertEquals("", _val[1]);
Assert.assertEquals(2, _headers);
}
@Test
public void testEmptyQuotedValue7230() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Name0: \"\"\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, HttpCompliance.RFC7230);
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("Name0", _hdr[1]);
Assert.assertEquals("", _val[1]);
Assert.assertEquals(2, _headers);
}
@Test
public void testNoColon2616() throws Exception

View File

@ -468,7 +468,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
{
_complianceViolations = new ArrayList<>();
}
String violation = String.format("%s<=%s: %s for %s", compliance, required, reason, getHttpTransport());
String violation = String.format("%s<%s: %s for %s", compliance, required, reason, getHttpTransport());
_complianceViolations.add(violation);
if (LOG.isDebugEnabled())
LOG.debug(violation);

View File

@ -147,7 +147,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponses(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200 OK"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<=RFC2616: name only header"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
assertThat("Response body", response, containsString("[Name] = []"));
}
@ -165,7 +165,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponses(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<=RFC2616: name only header"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
assertThat("Response body", response, containsString("[Name] = []"));
}
@ -184,7 +184,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponses(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<=RFC2616: header folding"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: header folding"));
assertThat("Response body", response, containsString("[Name] = [Some Value]"));
}