Skip to content

Commit

Permalink
Issue jetty#1966 Case Sensitive method (PR jetty#1967)
Browse files Browse the repository at this point in the history 
Issue jetty#1966 Case Sensitive method (PR jetty#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
  • Loading branch information
@gregw
gregw authored Nov 29, 2017
1 parent 70fe268 commit d4061fc
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 25 deletions.
41 changes: 30 additions & 11 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,10 @@ protected boolean complianceViolation(HttpCompliance compliance,String reason)
}

/* ------------------------------------------------------------------------------- */
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;
}

/* ------------------------------------------------------------------------------- */
Expand Down Expand Up @@ -646,9 +647,27 @@ private boolean parseLine(ByteBuffer buffer)
{
_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)
Expand Down Expand Up @@ -749,7 +768,7 @@ else if (b < HttpTokens.SPACE && b>=0)
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);
Expand Down Expand Up @@ -816,7 +835,7 @@ else if (b == HttpTokens.LINE_FEED)
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);
Expand Down Expand Up @@ -934,7 +953,7 @@ else if (values.stream().anyMatch(HttpHeaderValue.CHUNKED::is))
_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;
Expand Down Expand Up @@ -963,7 +982,7 @@ else if (values.stream().anyMatch(HttpHeaderValue.CHUNKED::is))
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);
}
}
Expand Down Expand Up @@ -1031,7 +1050,7 @@ protected boolean parseFields(ByteBuffer buffer)
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?
Expand Down Expand Up @@ -1154,13 +1173,13 @@ else if (_endOfContent == EndOfContent.UNKNOWN_CONTENT)
{
// 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);
}
}
Expand Down Expand Up @@ -1253,7 +1272,7 @@ else if (_endOfContent == EndOfContent.UNKNOWN_CONTENT)
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)
{
Expand Down
48 changes: 40 additions & 8 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public void testNoValue() throws Exception
}

@Test
public void testNoColon2616() throws Exception
public void testNoColonLegacy() throws Exception
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.0\r\n" +
Expand All @@ -360,7 +360,7 @@ public void testNoColon2616() throws Exception
"\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);
Expand All @@ -375,7 +375,7 @@ public void testNoColon2616() throws Exception
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
Expand Down Expand Up @@ -674,10 +674,42 @@ public void testHeaderTab() throws Exception
}

@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");
Expand All @@ -697,18 +729,18 @@ public void testCaseInsensitive() throws Exception
}

@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");
HttpParser.RequestHandler handler = new Handler();
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]);
Expand Down
9 changes: 3 additions & 6 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ public void testNoColonHeader_Middle() throws Exception

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] = []"));
}

Expand All @@ -165,8 +164,7 @@ public void testNoColonHeader_End() throws Exception

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] = []"));
}

Expand All @@ -184,8 +182,7 @@ public void testFoldedHeader() throws Exception

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]"));
}
}

0 comments on commit d4061fc

Please sign in to comment.