From b76240e678a717e8ad0da42d9e048464453aeb37 Mon Sep 17 00:00:00 2001 From: fb Date: Thu, 14 Dec 2017 11:48:32 +0100 Subject: [PATCH 1/3] Fixes #2022 - Ignore invalid chars in http header names, when the compliance mode is not RFC7230 Signed-off-by: fb --- .../AbstractConnectorHttpClientTransport.java | 1 + .../client/http/HttpReceiverOverHTTP.java | 3 +- .../org/eclipse/jetty/http/HttpParser.java | 8 +- .../eclipse/jetty/http/HttpParserTest.java | 74 ++++++++++++++++--- 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java index e50294ba3f5..99f96584c9e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java @@ -160,6 +160,7 @@ public abstract class AbstractConnectorHttpClientTransport extends AbstractHttpC { SocketChannelEndPoint endp = new SocketChannelEndPoint(channel, selector, key, getScheduler()); endp.setIdleTimeout(client.getIdleTimeout()); + //TODO: make compliance mode configurable return endp; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index b61997ec2f0..e95d3c276b7 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.client.HttpExchange; import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpResponse; import org.eclipse.jetty.client.HttpResponseException; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpParser; @@ -38,7 +39,7 @@ import org.eclipse.jetty.util.CompletableCallback; public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.ResponseHandler { - private final HttpParser parser = new HttpParser(this); + private final HttpParser parser = new HttpParser(this, -1, HttpCompliance.RFC2616); private ByteBuffer buffer; private boolean shutdown; 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 8dbc5024ea6..da8ae5816df 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 @@ -1287,7 +1287,13 @@ public class HttpParser break; } - throw new IllegalCharacterException(_state,b,buffer); + //Ignore all invalid characters + if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 Invalid token in header name")) + { + throw new IllegalCharacterException(_state,b,buffer); + } + + break; case VALUE: if (b>HttpTokens.SPACE || b<0) 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 a049c3cd91d..8f68fc454b9 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 @@ -21,6 +21,7 @@ package org.eclipse.jetty.http; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.eclipse.jetty.http.HttpParser.State; @@ -286,7 +287,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.containsString("Header Folding")); Assert.assertNull(_complianceViolation); } - + @Test public void testWhiteSpaceInName() throws Exception { @@ -303,7 +304,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.notNullValue()); Assert.assertThat(_bad, Matchers.containsString("Illegal character")); } - + @Test public void testWhiteSpaceAfterName() throws Exception { @@ -320,7 +321,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.notNullValue()); Assert.assertThat(_bad, Matchers.containsString("Illegal character")); } - + @Test public void testNoValue() throws Exception { @@ -377,7 +378,58 @@ public class HttpParserTest Assert.assertEquals(2, _headers); Assert.assertThat(_complianceViolation, Matchers.containsString("No colon")); } - + + @Test + public void testNoneComplientCharsInHeaderNameLegacy() throws Exception + { + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 204 No Content\r\n" + + "Access-Control-Allow-Headers : Origin\r\n" + + "Other: value\r\n" + + "\r\n"); + + HttpParser.ResponseHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY); + parseAll(parser, buffer); + + Assert.assertTrue(_headerCompleted); + Assert.assertTrue(_messageCompleted); + + Assert.assertEquals("HTTP/1.1", _methodOrVersion); + Assert.assertEquals("204", _uriOrStatus); + Assert.assertEquals("No Content", _versionOrReason); + Assert.assertEquals(null, _content); + + Assert.assertEquals(2, _headers); + System.out.println(Arrays.asList(_hdr)); + System.out.println(Arrays.asList(_val)); + Assert.assertEquals("Access-Control-Allow-Headers", _hdr[1]); + Assert.assertEquals("Origin", _val[1]); + Assert.assertEquals("Other", _hdr[2]); + Assert.assertEquals("value", _val[2]); + + Assert.assertThat(_complianceViolation, Matchers.containsString("Invalid token in header name")); + } + + @Test + public void testNoneComplientCharsInHeaderNameNoLegacy() throws Exception + { + ByteBuffer buffer = BufferUtil.toBuffer( + "HTTP/1.1 204 No Content\r\n" + + "Access-Control-Allow-Headers : Origin\r\n" + + "Other: value\r\n" + + "\r\n"); + + HttpParser.ResponseHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler); + parseAll(parser, buffer); + + Assert.assertEquals("HTTP/1.1", _methodOrVersion); + Assert.assertEquals("204", _uriOrStatus); + Assert.assertEquals("No Content", _versionOrReason); + Assert.assertThat(_bad, Matchers.containsString("Illegal character 0x20")); + } + @Test public void testNoColon7230() throws Exception { @@ -393,7 +445,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.containsString("Illegal character")); Assert.assertNull(_complianceViolation); } - + @Test public void testHeaderParseDirect() throws Exception @@ -592,7 +644,7 @@ public class HttpParserTest Assert.assertEquals(1, _headers); Assert.assertEquals(null, _bad); } - + @Test public void testResponseBufferUpgradeFrom() throws Exception { @@ -603,15 +655,15 @@ public class HttpParserTest "Sec-WebSocket-Accept: 4GnyoUP4Sc1JD+2pCbNYAhFYVVA\r\n" + "\r\n" + "FOOGRADE"); - + HttpParser.ResponseHandler handler = new Handler(); HttpParser parser = new HttpParser(handler); - + while (!parser.isState(State.END)) { parser.parseNext(buffer); } - + Assert.assertThat(BufferUtil.toUTF8String(buffer), Matchers.is("FOOGRADE")); } @@ -1366,10 +1418,10 @@ public class HttpParserTest @Test public void testResponseReasonIso8859_1() throws Exception - { + { ByteBuffer buffer = BufferUtil.toBuffer( "HTTP/1.1 302 déplacé temporairement\r\n" - + "Content-Length: 0\r\n" + + "Content-Length: 0\r\n" + "\r\n",StandardCharsets.ISO_8859_1); HttpParser.ResponseHandler handler = new Handler(); From a324a5ad77affcdb520e90d24e137e0856aa4469 Mon Sep 17 00:00:00 2001 From: fb Date: Fri, 15 Dec 2017 15:10:35 +0100 Subject: [PATCH 2/3] Implement weak http header parsing and make the compliance mode configurable in the client Signed-off-by: fb --- .../AbstractConnectorHttpClientTransport.java | 1 - .../org/eclipse/jetty/client/HttpClient.java | 25 +++++-- .../client/http/HttpReceiverOverHTTP.java | 4 +- .../client/http/HttpReceiverOverHTTPTest.java | 58 ++++++++++++++-- .../eclipse/jetty/http/HttpCompliance.java | 8 ++- .../org/eclipse/jetty/http/HttpParser.java | 51 +++++++------- .../eclipse/jetty/http/HttpParserTest.java | 68 +++++++++++++------ 7 files changed, 158 insertions(+), 57 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java index 99f96584c9e..e50294ba3f5 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectorHttpClientTransport.java @@ -160,7 +160,6 @@ public abstract class AbstractConnectorHttpClientTransport extends AbstractHttpC { SocketChannelEndPoint endp = new SocketChannelEndPoint(channel, selector, key, getScheduler()); endp.setIdleTimeout(client.getIdleTimeout()); - //TODO: make compliance mode configurable return endp; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index fc79d0ebe98..3f969c1313c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -52,10 +52,7 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.util.FormContentProvider; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpMethod; -import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.http.*; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.MappedByteBufferPool; @@ -147,6 +144,7 @@ public class HttpClient extends ContainerLifeCycle private boolean removeIdleDestinations = false; private boolean connectBlocking = false; private String name = getClass().getSimpleName() + "@" + Integer.toHexString(hashCode()); + private HttpCompliance httpCompliance = HttpCompliance.RFC7230; /** * Creates a {@link HttpClient} instance that can perform requests to non-TLS destinations only @@ -972,6 +970,25 @@ public class HttpClient extends ContainerLifeCycle { } + /** + * Gets the http compliance mode for parsing http responses. + * The default http compliance level is {@link HttpCompliance#RFC7230} which is the latest HTTP/1.1 specification + */ + public HttpCompliance getHttpCompliance() + { + return httpCompliance; + } + + /** + * Sets the http compliance mode for parsing http responses. + * This affect how weak the {@link HttpParser} parses http responses and which http protocol level is supported + * @param httpCompliance The compliance level which is used to actually parse http responses + */ + public void setHttpCompliance(HttpCompliance httpCompliance) + { + this.httpCompliance = httpCompliance; + } + /** * @return whether request events must be strictly ordered * @see #setStrictEventOrdering(boolean) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index e95d3c276b7..1402c659418 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -39,13 +39,15 @@ import org.eclipse.jetty.util.CompletableCallback; public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.ResponseHandler { - private final HttpParser parser = new HttpParser(this, -1, HttpCompliance.RFC2616); + private final HttpParser parser; private ByteBuffer buffer; private boolean shutdown; public HttpReceiverOverHTTP(HttpChannelOverHTTP channel) { super(channel); + // TODO: Seems to be not the optimal way to get the compliance mode + parser = new HttpParser(this, -1, channel.getHttpDestination().getHttpClient().getHttpCompliance()); } @Override diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java index 747aecc1288..49a0e670dd1 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java @@ -20,7 +20,7 @@ package org.eclipse.jetty.client.http; import java.io.EOFException; import java.nio.charset.StandardCharsets; -import java.util.Collections; +import java.util.*; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -33,32 +33,49 @@ import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.FutureResponseListener; -import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.*; import org.eclipse.jetty.io.ByteArrayEndPoint; import org.eclipse.jetty.toolchain.test.TestTracker; import org.eclipse.jetty.util.Promise; +import org.hamcrest.*; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +@RunWith(Parameterized.class) public class HttpReceiverOverHTTPTest { @Rule public final TestTracker tracker = new TestTracker(); + @Parameterized.Parameter(0) + public HttpCompliance compliance; + private HttpClient client; private HttpDestinationOverHTTP destination; private ByteArrayEndPoint endPoint; private HttpConnectionOverHTTP connection; - + + @Parameterized.Parameters + public static Collection parameters() throws Exception + { + return Arrays.asList( + new Object[] { HttpCompliance.LEGACY }, + new Object[] { HttpCompliance.WEAK }, + new Object[] { HttpCompliance.RFC2616 }, + new Object[] { HttpCompliance.RFC7230 } + ); + } + @Before public void init() throws Exception { client = new HttpClient(); + client.setHttpCompliance(compliance); client.start(); destination = new HttpDestinationOverHTTP(client, new Origin("http", "localhost", 8080)); destination.start(); @@ -207,6 +224,37 @@ public class HttpReceiverOverHTTPTest Assert.assertTrue(e.getCause() instanceof HttpResponseException); } } + + @Test + public void test_Receive_Invalid_Response_With_Weak_Compliance() throws Exception + { + endPoint.addInput("" + + "HTTP/1.1 200 OK\r\n" + + "Content-length : 0\r\n" + + "\r\n"); + HttpExchange exchange = newExchange(); + FutureResponseListener listener = (FutureResponseListener)exchange.getResponseListeners().get(0); + connection.getHttpChannel().receive(); + + try { + Response response = listener.get(5, TimeUnit.SECONDS); + + Assert.assertThat(compliance, Matchers.lessThanOrEqualTo(HttpCompliance.WEAK)); + Assert.assertNotNull(response); + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals("OK", response.getReason()); + Assert.assertSame(HttpVersion.HTTP_1_1, response.getVersion()); + HttpFields headers = response.getHeaders(); + Assert.assertNotNull(headers); + Assert.assertEquals(1, headers.size()); + Assert.assertEquals("0", headers.get(HttpHeader.CONTENT_LENGTH)); + } catch (Exception e) { + Assert.assertThat(compliance, Matchers.greaterThan(HttpCompliance.WEAK)); + Throwable cause = e.getCause(); + Assert.assertThat(cause, CoreMatchers.instanceOf(HttpResponseException.class)); + Assert.assertThat(cause.getMessage(), Matchers.containsString("HTTP protocol violation")); + } + } @Test public void test_FillInterested_RacingWith_BufferRelease() throws Exception diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java index 1da0d42577a..f885505f995 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java @@ -24,9 +24,11 @@ package org.eclipse.jetty.http; *
*
RFC7230
(default) Compliance with RFC7230
*
RFC2616
Wrapped/Continued headers and HTTP/0.9 supported
- *
LEGACY
(aka STRICT) Adherence to Servlet Specification requirement for + *
WEAK
Wrapped/Continued headers, HTTP/0.9 supported and make the parser more acceptable against miss + * formatted requests/responses
+ *
LEGACY
(aka STRICT) Adherence to Servlet Specification requirement for * exact case of header names, bypassing the header caches, which are case insensitive, - * otherwise equivalent to RFC2616
+ * otherwise equivalent to WEAK *
*/ -public enum HttpCompliance { LEGACY, RFC2616, RFC7230 } \ No newline at end of file +public enum HttpCompliance { LEGACY, WEAK, RFC2616, RFC7230 } \ No newline at end of file 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 da8ae5816df..9de7fd0eb36 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 @@ -35,9 +35,7 @@ import org.eclipse.jetty.util.Utf8StringBuilder; import org.eclipse.jetty.util.log.Log; 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.HttpCompliance.*; 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; @@ -82,6 +80,7 @@ import static org.eclipse.jetty.http.HttpTokens.TAB; *
*
RFC7230
(default) Compliance with RFC7230
*
RFC2616
Wrapped headers and HTTP/0.9 supported
+ *
WEAK
Wrapped headers, HTTP/0.9 supported and a weaker parsing behaviour
*
LEGACY
(aka STRICT) Adherence to Servlet Specification requirement for * exact case of header names, bypassing the header caches, which are case insensitive, * otherwise equivalent to RFC2616
@@ -116,6 +115,7 @@ public class HttpParser IN_NAME, VALUE, IN_VALUE, + OWS_AFTER_NAME, } // States @@ -333,7 +333,7 @@ public class HttpParser /* ------------------------------------------------------------------------------- */ protected String caseInsensitiveHeader(String orig, String normative) { - return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig)) + return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(WEAK,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig)) ?normative:orig; } @@ -658,6 +658,7 @@ public class HttpParser // Legacy correctly allows case sensitive header; break; + case WEAK: case RFC2616: case RFC7230: if (!method.asString().equals(_methodString) && _complianceHandler!=null) @@ -1244,6 +1245,22 @@ public class HttpParser break; case IN_NAME: + if (b>HttpTokens.SPACE && b!=HttpTokens.COLON) + { + if (_header!=null) + { + setString(_header.asString()); + _header=null; + _headerString=null; + } + + _string.append((char)b); + _length=_string.length(); + break; + } + + // Fallthrough + case OWS_AFTER_NAME: if (b==HttpTokens.COLON) { if (_headerString==null) @@ -1256,23 +1273,8 @@ public class HttpParser setState(FieldState.VALUE); break; } - - if (b>HttpTokens.SPACE) - { - if (_header!=null) - { - setString(_header.asString()); - _header=null; - _headerString=null; - } - - _string.append((char)b); - if (b>HttpTokens.SPACE) - _length=_string.length(); - break; - } - if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 No colon")) + if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 No colon")) { if (_headerString==null) { @@ -1287,13 +1289,14 @@ public class HttpParser break; } - //Ignore all invalid characters - if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 Invalid token in header name")) + //Ignore trailing whitespaces + if (b==HttpTokens.SPACE && !complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 Invalid token in header name")) { - throw new IllegalCharacterException(_state,b,buffer); + setState(FieldState.OWS_AFTER_NAME); + break; } - break; + throw new IllegalCharacterException(_state,b,buffer); case VALUE: if (b>HttpTokens.SPACE || b<0) 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 8f68fc454b9..7e1879438f4 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 @@ -304,7 +304,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.notNullValue()); Assert.assertThat(_bad, Matchers.containsString("Illegal character")); } - + @Test public void testWhiteSpaceAfterName() throws Exception { @@ -321,7 +321,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.notNullValue()); Assert.assertThat(_bad, Matchers.containsString("Illegal character")); } - + @Test public void testNoValue() throws Exception { @@ -351,7 +351,7 @@ public class HttpParserTest } @Test - public void testNoColonLegacy() throws Exception + public void testNoColonWeak() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( "GET / HTTP/1.0\r\n" + @@ -361,7 +361,7 @@ public class HttpParserTest "\r\n"); HttpParser.RequestHandler handler = new Handler(); - HttpParser parser = new HttpParser(handler,HttpCompliance.LEGACY); + HttpParser parser = new HttpParser(handler,HttpCompliance.WEAK); parseAll(parser, buffer); Assert.assertTrue(_headerCompleted); @@ -380,7 +380,36 @@ public class HttpParserTest } @Test - public void testNoneComplientCharsInHeaderNameLegacy() throws Exception + public void testNoColonWithWhitespaceWeak() throws Exception + { + ByteBuffer buffer = BufferUtil.toBuffer( + "GET / HTTP/1.0\r\n" + + "Host: localhost\r\n" + + "Name \r\n" + + "Other: value\r\n" + + "\r\n"); + + HttpParser.RequestHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler,HttpCompliance.WEAK); + 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("Name", _hdr[1]); + Assert.assertEquals("", _val[1]); + Assert.assertEquals("Other", _hdr[2]); + Assert.assertEquals("value", _val[2]); + Assert.assertEquals(2, _headers); + Assert.assertThat(_complianceViolation, Matchers.containsString("No colon")); + } + + @Test + public void testTrailingSpacesInHeaderNameInWeakMode() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( "HTTP/1.1 204 No Content\r\n" + @@ -389,7 +418,7 @@ public class HttpParserTest "\r\n"); HttpParser.ResponseHandler handler = new Handler(); - HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY); + HttpParser parser = new HttpParser(handler, -1, HttpCompliance.WEAK); parseAll(parser, buffer); Assert.assertTrue(_headerCompleted); @@ -400,19 +429,19 @@ public class HttpParserTest Assert.assertEquals("No Content", _versionOrReason); Assert.assertEquals(null, _content); - Assert.assertEquals(2, _headers); + Assert.assertEquals(1, _headers); System.out.println(Arrays.asList(_hdr)); System.out.println(Arrays.asList(_val)); - Assert.assertEquals("Access-Control-Allow-Headers", _hdr[1]); - Assert.assertEquals("Origin", _val[1]); - Assert.assertEquals("Other", _hdr[2]); - Assert.assertEquals("value", _val[2]); + Assert.assertEquals("Access-Control-Allow-Headers", _hdr[0]); + Assert.assertEquals("Origin", _val[0]); + Assert.assertEquals("Other", _hdr[1]); + Assert.assertEquals("value", _val[1]); Assert.assertThat(_complianceViolation, Matchers.containsString("Invalid token in header name")); } @Test - public void testNoneComplientCharsInHeaderNameNoLegacy() throws Exception + public void testTrailingSpacesInHeaderNameNoWeak() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( "HTTP/1.1 204 No Content\r\n" + @@ -445,7 +474,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.containsString("Illegal character")); Assert.assertNull(_complianceViolation); } - + @Test public void testHeaderParseDirect() throws Exception @@ -644,7 +673,7 @@ public class HttpParserTest Assert.assertEquals(1, _headers); Assert.assertEquals(null, _bad); } - + @Test public void testResponseBufferUpgradeFrom() throws Exception { @@ -655,15 +684,15 @@ public class HttpParserTest "Sec-WebSocket-Accept: 4GnyoUP4Sc1JD+2pCbNYAhFYVVA\r\n" + "\r\n" + "FOOGRADE"); - + HttpParser.ResponseHandler handler = new Handler(); HttpParser parser = new HttpParser(handler); - + while (!parser.isState(State.END)) { parser.parseNext(buffer); } - + Assert.assertThat(BufferUtil.toUTF8String(buffer), Matchers.is("FOOGRADE")); } @@ -2199,8 +2228,9 @@ public class HttpParserTest _methodOrVersion = version.asString(); _uriOrStatus = Integer.toString(status); _versionOrReason = reason; - _hdr = new String[9]; - _val = new String[9]; + _headers = -1; + _hdr = new String[10]; + _val = new String[10]; _messageCompleted = false; _headerCompleted = false; return false; From 7ee8bb82c645ea1f1b2d0e31456e71df02936536 Mon Sep 17 00:00:00 2001 From: fb Date: Fri, 15 Dec 2017 15:23:13 +0100 Subject: [PATCH 3/3] Same code style fixes and remove unnecessary code reformatting Signed-off-by: fb --- .../java/org/eclipse/jetty/client/HttpClient.java | 7 ++++++- .../jetty/client/http/HttpReceiverOverHTTPTest.java | 12 +++++++++--- .../main/java/org/eclipse/jetty/http/HttpParser.java | 5 ++++- .../java/org/eclipse/jetty/http/HttpParserTest.java | 2 +- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 3f969c1313c..aaab436af1e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -52,7 +52,12 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.util.FormContentProvider; -import org.eclipse.jetty.http.*; +import org.eclipse.jetty.http.HttpCompliance; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpParser; +import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.MappedByteBufferPool; diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java index 49a0e670dd1..0fc38d65e03 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java @@ -20,7 +20,9 @@ package org.eclipse.jetty.client.http; import java.io.EOFException; import java.nio.charset.StandardCharsets; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -33,11 +35,15 @@ import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.FutureResponseListener; -import org.eclipse.jetty.http.*; +import org.eclipse.jetty.http.HttpCompliance; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.ByteArrayEndPoint; import org.eclipse.jetty.toolchain.test.TestTracker; import org.eclipse.jetty.util.Promise; -import org.hamcrest.*; +import org.hamcrest.CoreMatchers; +import org.hamcrest.Matchers; import org.junit.After; import org.junit.Assert; import org.junit.Before; 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 9de7fd0eb36..bce94cede8c 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 @@ -35,7 +35,10 @@ import org.eclipse.jetty.util.Utf8StringBuilder; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import static org.eclipse.jetty.http.HttpCompliance.*; +import static org.eclipse.jetty.http.HttpCompliance.LEGACY; +import static org.eclipse.jetty.http.HttpCompliance.WEAK; +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; 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 7e1879438f4..2fa6e2c06bf 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 @@ -287,7 +287,7 @@ public class HttpParserTest Assert.assertThat(_bad, Matchers.containsString("Header Folding")); Assert.assertNull(_complianceViolation); } - + @Test public void testWhiteSpaceInName() throws Exception {