Issue #1966 Case Sensitive method (PR #1967)

Issue #1966 Case Sensitive method (PR #1967)

* Modified the compliance violations to warn if case insensitivety is applied to a header
* removed duplicated if
* Fixed string comparison
* Improved compliance messages and comments
* updated expected violation messages
* Require a header colon only when in 7230 compliance mode
This commit is contained in:
Greg Wilkins 2017-11-29 18:56:54 +01:00 committed by GitHub
parent 70fe268bde
commit d4061fcfeb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 25 deletions

View File

@ -331,9 +331,10 @@ public class HttpParser
}
/* ------------------------------------------------------------------------------- */
protected String legacyString(String orig, String cached)
protected String caseInsensitiveHeader(String orig, String normative)
{
return (_compliance!=LEGACY || orig.equals(cached) || complianceViolation(RFC2616,"case sensitive"))?cached:orig;
return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig))
?normative:orig;
}
/* ------------------------------------------------------------------------------- */
@ -646,9 +647,27 @@ public class HttpParser
{
_length=_string.length();
_methodString=takeString();
// TODO #1966 This cache lookup is case insensitive when it should be case sensitive by RFC2616, RFC7230
HttpMethod method=HttpMethod.CACHE.get(_methodString);
if (method!=null)
_methodString=legacyString(_methodString,method.asString());
{
switch(_compliance)
{
case LEGACY:
// Legacy correctly allows case sensitive header;
break;
case RFC2616:
case RFC7230:
if (!method.asString().equals(_methodString) && _complianceHandler!=null)
_complianceHandler.onComplianceViolation(_compliance,HttpCompliance.LEGACY,
"https://tools.ietf.org/html/rfc7230#section-3.1.1 case insensitive method "+_methodString);
// TODO Good to used cached version for faster equals checking, but breaks case sensitivity because cache is insensitive
_methodString = method.asString();
break;
}
}
setState(State.SPACE1);
}
else if (b < SPACE)
@ -749,7 +768,7 @@ public class HttpParser
else if (b < HttpTokens.SPACE && b>=0)
{
// HTTP/0.9
if (complianceViolation(RFC7230,"HTTP/0.9"))
if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
setState(State.END);
@ -816,7 +835,7 @@ public class HttpParser
else
{
// HTTP/0.9
if (complianceViolation(RFC7230,"HTTP/0.9"))
if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9"))
throw new BadMessageException("HTTP/0.9 not supported");
handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
@ -934,7 +953,7 @@ public class HttpParser
_host=true;
if (!(_field instanceof HostPortHttpField) && _valueString!=null && !_valueString.isEmpty())
{
_field=new HostPortHttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
_field=new HostPortHttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString);
add_to_connection_trie=_fieldCache!=null;
}
break;
@ -963,7 +982,7 @@ public class HttpParser
if (add_to_connection_trie && !_fieldCache.isFull() && _header!=null && _valueString!=null)
{
if (_field==null)
_field=new HttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
_field=new HttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString);
_fieldCache.put(_field);
}
}
@ -1031,7 +1050,7 @@ public class HttpParser
case HttpTokens.SPACE:
case HttpTokens.TAB:
{
if (complianceViolation(RFC7230,"header folding"))
if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2.4 folding"))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding");
// header value without name - continuation?
@ -1154,13 +1173,13 @@ public class HttpParser
{
// 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);
n=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn);
String fv=field.getValue();
if (fv==null)
v=null;
else
{
v=legacyString(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv);
v=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv);
field=new HttpField(field.getHeader(),n,v);
}
}
@ -1253,7 +1272,7 @@ public class HttpParser
break;
}
if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"name only header"))
if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 No colon"))
{
if (_headerString==null)
{

View File

@ -350,7 +350,7 @@ public class HttpParserTest
}
@Test
public void testNoColon2616() throws Exception
public void testNoColonLegacy() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
@ -360,7 +360,7 @@ public class HttpParserTest
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616);
HttpParser parser = new HttpParser(handler,HttpCompliance.LEGACY);
parseAll(parser, buffer);
Assert.assertTrue(_headerCompleted);
@ -375,7 +375,7 @@ public class HttpParserTest
Assert.assertEquals("Other", _hdr[2]);
Assert.assertEquals("value", _val[2]);
Assert.assertEquals(2, _headers);
Assert.assertThat(_complianceViolation, Matchers.containsString("name only"));
Assert.assertThat(_complianceViolation, Matchers.containsString("No colon"));
}
@Test
@ -674,10 +674,42 @@ public class HttpParserTest
}
@Test
public void testCaseInsensitive() throws Exception
public void testCaseSensitiveMethod() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"get / http/1.0\r\n" +
"gEt / http/1.0\r\n" +
"Host: localhost\r\n" +
"Connection: close\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, -1, HttpCompliance.RFC7230);
parseAll(parser, buffer);
Assert.assertNull(_bad);
Assert.assertEquals("GET", _methodOrVersion);
Assert.assertThat(_complianceViolation, Matchers.containsString("case insensitive method gEt"));
}
@Test
public void testCaseSensitiveMethodLegacy() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"gEt / http/1.0\r\n" +
"Host: localhost\r\n" +
"Connection: close\r\n" +
"\r\n");
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY);
parseAll(parser, buffer);
Assert.assertNull(_bad);
Assert.assertEquals("gEt", _methodOrVersion);
Assert.assertNull(_complianceViolation);
}
@Test
public void testCaseInsensitiveHeader() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / http/1.0\r\n" +
"HOST: localhost\r\n" +
"cOnNeCtIoN: ClOsE\r\n" +
"\r\n");
@ -697,10 +729,10 @@ public class HttpParserTest
}
@Test
public void testCaseSensitiveLegacy() throws Exception
public void testCaseInSensitiveHeaderLegacy() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"gEt / http/1.0\r\n" +
"GET / http/1.0\r\n" +
"HOST: localhost\r\n" +
"cOnNeCtIoN: ClOsE\r\n" +
"\r\n");
@ -708,7 +740,7 @@ public class HttpParserTest
HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY);
parseAll(parser, buffer);
Assert.assertNull(_bad);
Assert.assertEquals("gEt", _methodOrVersion);
Assert.assertEquals("GET", _methodOrVersion);
Assert.assertEquals("/", _uriOrStatus);
Assert.assertEquals("HTTP/1.0", _versionOrReason);
Assert.assertEquals("HOST", _hdr[0]);

View File

@ -147,8 +147,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200 OK"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon"));
assertThat("Response body", response, containsString("[Name] = []"));
}
@ -165,8 +164,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon"));
assertThat("Response body", response, containsString("[Name] = []"));
}
@ -184,8 +182,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: header folding"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2.4 folding"));
assertThat("Response body", response, containsString("[Name] = [Some Value]"));
}
}