From 7e516e29fbdeff1b6a02bb3230aa16effaabe6da Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sun, 24 Dec 2017 18:00:03 +0100 Subject: [PATCH] pass existing compliance tests Signed-off-by: Greg Wilkins --- .../client/http/HttpReceiverOverHTTPTest.java | 42 +------------------ .../eclipse/jetty/http/HttpCompliance.java | 29 ++++++++----- .../jetty/http/HttpComplianceSection.java | 17 +++++++- .../org/eclipse/jetty/http/HttpParser.java | 14 ++++--- .../jetty/server/HttpChannelOverHttp.java | 10 +++-- .../eclipse/jetty/server/HttpConnection.java | 2 +- .../servlet/ComplianceViolations2616Test.java | 7 ++-- 7 files changed, 54 insertions(+), 67 deletions(-) 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 e9b09d4167d..ff4235943bb 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 @@ -35,15 +35,12 @@ import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.http.HttpCompliance; -import org.eclipse.jetty.http.HttpComplianceSection; 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.CoreMatchers; -import org.hamcrest.Matchers; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -54,12 +51,7 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class HttpReceiverOverHTTPTest -{ - static - { - HttpCompliance.CUSTOM0.sections().remove(HttpComplianceSection.RFC7230_3_2_4_NO_WS_AFTER_FIELD_NAME); - } - +{ @Rule public final TestTracker tracker = new TestTracker(); @@ -76,7 +68,6 @@ public class HttpReceiverOverHTTPTest { return Arrays.asList( new Object[] { HttpCompliance.LEGACY }, - new Object[] { HttpCompliance.CUSTOM0 }, new Object[] { HttpCompliance.RFC2616 }, new Object[] { HttpCompliance.RFC7230 } ); @@ -235,37 +226,6 @@ 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.CUSTOM0)); - 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.CUSTOM0)); - 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 3ab9f05d5f0..59a3f5f1cd4 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 @@ -38,8 +38,13 @@ public enum HttpCompliance { // TODO in Jetty-10 convert this enum to a class so that extra custom modes can be defined dynamically LEGACY(sectionsBySpec("LEGACY")), - RFC2616(sectionsBySpec("RFC2616")), - RFC7230(sectionsBySpec("RFC7230,-RFC7230_3_1_1_METHOD_CASE_SENSITIVE")), // TODO fix in Jetty-10 + + // Jetty's historic RFC2616 support incorrectly allowed no colon and case insensitive methods + RFC2616(sectionsBySpec("RFC2616,-RFC7230_3_2_FIELD_COLON,-RFC7230_3_1_1_METHOD_CASE_SENSITIVE")), + + // TODO Jetty's current RFC7230 support incorrectly handles methods case insensitively + RFC7230(sectionsBySpec("RFC7230,-RFC7230_3_1_1_METHOD_CASE_SENSITIVE")), + CUSTOM0(sectionsByProperty("CUSTOM0")), CUSTOM1(sectionsByProperty("CUSTOM1")), CUSTOM2(sectionsByProperty("CUSTOM2")), @@ -59,17 +64,25 @@ public enum HttpCompliance int i=0; switch(elements[i]) - { + { + case "0": + sections = EnumSet.noneOf(HttpComplianceSection.class); + i++; + break; + + case "*": + i++; + sections = EnumSet.allOf(HttpComplianceSection.class); + break; + case "RFC2616": sections = EnumSet.complementOf(EnumSet.of( - HttpComplianceSection.RFC7230_3_2_4_NO_WS_AFTER_FIELD_NAME, HttpComplianceSection.RFC7230_3_2_4_NO_FOLDING, HttpComplianceSection.RFC7230_A2_NO_HTTP_9)); i++; break; case "RFC7230": - case "*": i++; sections = EnumSet.allOf(HttpComplianceSection.class); break; @@ -79,10 +92,6 @@ public enum HttpCompliance i++; break; - case "0": - sections = EnumSet.noneOf(HttpComplianceSection.class); - i++; - break; default: sections = EnumSet.noneOf(HttpComplianceSection.class); break; @@ -135,8 +144,6 @@ public enum HttpCompliance return __required.get(section); } - - private final EnumSet _sections; private HttpCompliance(EnumSet sections) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java index fae7c613f2b..0f1b1a63f6a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java @@ -31,8 +31,23 @@ public enum HttpComplianceSection RFC7230_A2_NO_HTTP_9("https://tools.ietf.org/html/rfc7230#appendix-A.2","No HTTP/0.9"), ; + final String url; + final String description; + HttpComplianceSection(String url,String description) { - + this.url = url; + this.description = description; } + + public String getURL() + { + return url; + } + + public String getDescription() + { + return description; + } + } \ 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 f1578f4f396..68b65f9611d 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,8 +35,6 @@ 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.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; @@ -1068,7 +1066,7 @@ public class HttpParser case HttpTokens.SPACE: case HttpTokens.TAB: { - if (complianceViolation(HttpComplianceSection.RFC7230_3_2_4_NO_FOLDING,"Folded "+_headerString)) + if (complianceViolation(HttpComplianceSection.RFC7230_3_2_4_NO_FOLDING,_headerString)) throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding"); // header value without name - continuation? @@ -1279,6 +1277,7 @@ public class HttpParser } // Fallthrough + case WS_AFTER_NAME: if (b==HttpTokens.COLON) { @@ -1293,7 +1292,7 @@ public class HttpParser break; } - if (b==HttpTokens.LINE_FEED && !complianceViolation(HttpComplianceSection.RFC7230_3_2_FIELD_COLON,null)) + if (b==HttpTokens.LINE_FEED) { if (_headerString==null) { @@ -1304,8 +1303,11 @@ public class HttpParser _valueString=""; _length=-1; - setState(FieldState.FIELD); - break; + if (!complianceViolation(HttpComplianceSection.RFC7230_3_2_FIELD_COLON,_headerString)) + { + setState(FieldState.FIELD); + break; + } } //Ignore trailing whitespaces diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java index 54dd7139615..d69234ec91e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java @@ -27,6 +27,7 @@ import java.util.List; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpCompliance; +import org.eclipse.jetty.http.HttpComplianceSection; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpGenerator; @@ -505,7 +506,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque } @Override - public void onComplianceViolation(HttpCompliance compliance, HttpCompliance required, String reason) + public void onComplianceViolation(HttpCompliance compliance, HttpComplianceSection violation, String reason) { if (_httpConnection.isRecordHttpComplianceViolations()) { @@ -513,10 +514,11 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque { _complianceViolations = new ArrayList<>(); } - String violation = String.format("%s<%s: %s for %s", compliance, required, reason, getHttpTransport()); - _complianceViolations.add(violation); + String record = String.format("%s (see %s) in mode %s for %s in %s", + violation.getDescription(), violation.getURL(), compliance, reason, getHttpTransport()); + _complianceViolations.add(record); if (LOG.isDebugEnabled()) - LOG.debug(violation); + LOG.debug(record); } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index b7f51632dd7..91d7563f319 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -105,7 +105,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http _channel = newHttpChannel(); _input = _channel.getRequest().getHttpInput(); _parser = newHttpParser(compliance); - _recordHttpComplianceViolations=recordComplianceViolations; + _recordHttpComplianceViolations = recordComplianceViolations; if (LOG.isDebugEnabled()) LOG.debug("New HTTP Connection {}", this); } 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 6b805040edb..17cabe80b39 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,7 +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