From 915af401e1471242b5f3a087d906618145880e9d Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Mar 2018 13:22:01 +1100 Subject: [PATCH] HttpParser cleanup #2232 Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpParser.java | 207 +++++++++--------- .../eclipse/jetty/http/HttpParserTest.java | 17 +- 2 files changed, 119 insertions(+), 105 deletions(-) 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 fdbab907d06..ddff6e12dba 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 @@ -36,6 +36,7 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import static org.eclipse.jetty.http.HttpTokens.CARRIAGE_RETURN; +import static org.eclipse.jetty.http.HttpTokens.COLON; import static org.eclipse.jetty.http.HttpTokens.LINE_FEED; import static org.eclipse.jetty.http.HttpTokens.SPACE; import static org.eclipse.jetty.http.HttpTokens.TAB; @@ -1074,7 +1075,7 @@ public class HttpParser throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding"); // header value without name - continuation? - if (_valueString==null) + if (_valueString==null || _valueString.isEmpty()) { _string.setLength(0); _length=0; @@ -1166,10 +1167,6 @@ public class HttpParser default: { - // now handle the ch - if (bHttpTokens.SPACE && b!=HttpTokens.COLON) + switch(b) { - if (_header!=null) - { - setString(_header.asString()); - _header=null; - _headerString=null; - } - - _string.append((char)b); - _length=_string.length(); - break; - } - - // Fallthrough - - case WS_AFTER_NAME: - if (b==HttpTokens.COLON) - { - if (_headerString==null) - { + case SPACE: + case TAB: + //Ignore trailing whitespaces ? + if (!complianceViolation(HttpComplianceSection.NO_WS_AFTER_FIELD_NAME,null)) + { + _headerString=takeString(); + _header=HttpHeader.CACHE.get(_headerString); + _length=-1; + setState(FieldState.WS_AFTER_NAME); + break; + } + throw new IllegalCharacterException(_state,b,buffer); + + case COLON: _headerString=takeString(); _header=HttpHeader.CACHE.get(_headerString); - } - _length=-1; - - setState(FieldState.VALUE); - break; - } - - if (b==HttpTokens.LINE_FEED) - { - if (_headerString==null) - { - _headerString=takeString(); - _header=HttpHeader.CACHE.get(_headerString); - } - _string.setLength(0); - _valueString=""; - _length=-1; - - if (!complianceViolation(HttpComplianceSection.FIELD_COLON,_headerString)) - { - setState(FieldState.FIELD); + _length=-1; + setState(FieldState.VALUE); break; - } - } + + case LINE_FEED: + _headerString=takeString(); + _header=HttpHeader.CACHE.get(_headerString); + _string.setLength(0); + _valueString=""; + _length=-1; - //Ignore trailing whitespaces - if (b==HttpTokens.SPACE && !complianceViolation(HttpComplianceSection.NO_WS_AFTER_FIELD_NAME,null)) + if (!complianceViolation(HttpComplianceSection.FIELD_COLON,_headerString)) + { + setState(FieldState.FIELD); + break; + } + throw new IllegalCharacterException(_state,b,buffer); + + default: + if (b<0) + throw new IllegalCharacterException(_state,b,buffer); + + _string.append((char)b); + _length=_string.length(); + break; + } + break; + + case WS_AFTER_NAME: + + switch(b) { - setState(FieldState.WS_AFTER_NAME); - break; - } + case SPACE: + case TAB: + break; - throw new IllegalCharacterException(_state,b,buffer); + case COLON: + setState(FieldState.VALUE); + break; + + case LINE_FEED: + if (!complianceViolation(HttpComplianceSection.FIELD_COLON,_headerString)) + { + setState(FieldState.FIELD); + break; + } + throw new IllegalCharacterException(_state,b,buffer); + + default: + throw new IllegalCharacterException(_state,b,buffer); + } + break; case VALUE: - if (b>HttpTokens.SPACE || b<0) + switch(b) { - _string.append((char)(0xff&b)); - _length=_string.length(); - setState(FieldState.IN_VALUE); - break; - } - - if (b==HttpTokens.SPACE || b==HttpTokens.TAB) - break; - - if (b==HttpTokens.LINE_FEED) - { - _string.setLength(0); - _valueString=""; - _length=-1; - - setState(FieldState.FIELD); - break; - } - throw new IllegalCharacterException(_state,b,buffer); - - case IN_VALUE: - if (b>=HttpTokens.SPACE || b<0 || b==HttpTokens.TAB) - { - if (_valueString!=null) - { - setString(_valueString); - _valueString=null; - _field=null; - } - _string.append((char)(0xff&b)); - if (b>HttpTokens.SPACE || b<0) - _length=_string.length(); - break; - } - - if (b==HttpTokens.LINE_FEED) - { - if (_length > 0) - { - _valueString=takeString(); + case LINE_FEED: + _string.setLength(0); + _valueString=""; _length=-1; - } - setState(FieldState.FIELD); + + setState(FieldState.FIELD); + break; + + case SPACE: + case TAB: + break; + + default: + _string.append((char)(0xff&b)); + _length=_string.length(); + setState(FieldState.IN_VALUE); break; } - - throw new IllegalCharacterException(_state,b,buffer); - + break; + + case IN_VALUE: + switch(b) + { + case LINE_FEED: + if (_length > 0) + { + _valueString=takeString(); + _length=-1; + } + setState(FieldState.FIELD); + break; + + case SPACE: + case TAB: + _string.append((char)(0xff&b)); + break; + + default: + _string.append((char)(0xff&b)); + _length=_string.length(); + break; + } + break; + default: throw new IllegalStateException(_state.toString()); 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 98e1f9a9044..e4d95a2eb89 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 @@ -18,6 +18,7 @@ package org.eclipse.jetty.http; +import static org.eclipse.jetty.http.HttpComplianceSection.NO_FIELD_FOLDING; import static org.hamcrest.Matchers.contains; import java.nio.ByteBuffer; @@ -261,6 +262,8 @@ public class HttpParserTest "Host: localhost\r\n" + "Name: value\r\n" + " extra\r\n" + + "Name2: \r\n" + + "\tvalue2\r\n" + "\r\n"); HttpParser.RequestHandler handler = new Handler(); @@ -270,10 +273,12 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.nullValue()); Assert.assertEquals("Host", _hdr[0]); Assert.assertEquals("localhost", _val[0]); + Assert.assertEquals(2, _headers); Assert.assertEquals("Name", _hdr[1]); Assert.assertEquals("value extra", _val[1]); - Assert.assertEquals(1, _headers); - Assert.assertThat(_complianceViolation, contains(HttpComplianceSection.NO_FIELD_FOLDING)); + Assert.assertEquals("Name2", _hdr[2]); + Assert.assertEquals("value2", _val[2]); + Assert.assertThat(_complianceViolation, contains(NO_FIELD_FOLDING,NO_FIELD_FOLDING)); } @Test @@ -399,7 +404,7 @@ public class HttpParserTest ByteBuffer buffer = BufferUtil.toBuffer( "HTTP/1.1 204 No Content\r\n" + "Access-Control-Allow-Headers : Origin\r\n" + - "Other: value\r\n" + + "Other\t : value\r\n" + "\r\n"); HttpParser.ResponseHandler handler = new Handler(); @@ -422,7 +427,7 @@ public class HttpParserTest Assert.assertEquals("Other", _hdr[1]); Assert.assertEquals("value", _val[1]); - Assert.assertThat(_complianceViolation, contains(HttpComplianceSection.NO_WS_AFTER_FIELD_NAME)); + Assert.assertThat(_complianceViolation, contains(HttpComplianceSection.NO_WS_AFTER_FIELD_NAME,HttpComplianceSection.NO_WS_AFTER_FIELD_NAME)); } @Test @@ -709,7 +714,9 @@ public class HttpParserTest public void testBadHeaderEncoding() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( - "GET / HTTP/1.0\r\nH\u00e6der0: value0\r\n\n\n"); + "GET / HTTP/1.0\r\n" + + "H\u00e6der0: value0\r\n" + + "\n\n"); HttpParser.RequestHandler handler = new Handler(); HttpParser parser = new HttpParser(handler);