diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index d2db919ccbbc..8dbc5024ea66 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -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; } /* ------------------------------------------------------------------------------- */ @@ -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) @@ -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); @@ -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); @@ -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; @@ -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); } } @@ -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? @@ -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); } } @@ -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) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index b2d65e39d2a0..a049c3cd91dd 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -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" + @@ -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); @@ -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 @@ -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"); @@ -697,10 +729,10 @@ 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"); @@ -708,7 +740,7 @@ public void testCaseSensitiveLegacy() throws Exception 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]); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java index 68e72bf3084d..6b805040edbd 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java @@ -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