pass existing compliance tests

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2017-12-24 18:00:03 +01:00
parent d8dead35ae
commit 7e516e29fb
7 changed files with 54 additions and 67 deletions

View File

@ -35,15 +35,12 @@ import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.client.util.FutureResponseListener;
import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpComplianceSection;
import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.io.ByteArrayEndPoint; import org.eclipse.jetty.io.ByteArrayEndPoint;
import org.eclipse.jetty.toolchain.test.TestTracker; import org.eclipse.jetty.toolchain.test.TestTracker;
import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.Promise;
import org.hamcrest.CoreMatchers;
import org.hamcrest.Matchers;
import org.junit.After; import org.junit.After;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
@ -54,12 +51,7 @@ import org.junit.runners.Parameterized;
@RunWith(Parameterized.class) @RunWith(Parameterized.class)
public class HttpReceiverOverHTTPTest public class HttpReceiverOverHTTPTest
{ {
static
{
HttpCompliance.CUSTOM0.sections().remove(HttpComplianceSection.RFC7230_3_2_4_NO_WS_AFTER_FIELD_NAME);
}
@Rule @Rule
public final TestTracker tracker = new TestTracker(); public final TestTracker tracker = new TestTracker();
@ -76,7 +68,6 @@ public class HttpReceiverOverHTTPTest
{ {
return Arrays.asList( return Arrays.asList(
new Object[] { HttpCompliance.LEGACY }, new Object[] { HttpCompliance.LEGACY },
new Object[] { HttpCompliance.CUSTOM0 },
new Object[] { HttpCompliance.RFC2616 }, new Object[] { HttpCompliance.RFC2616 },
new Object[] { HttpCompliance.RFC7230 } new Object[] { HttpCompliance.RFC7230 }
); );
@ -235,37 +226,6 @@ public class HttpReceiverOverHTTPTest
Assert.assertTrue(e.getCause() instanceof HttpResponseException); 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 @Test
public void test_FillInterested_RacingWith_BufferRelease() throws Exception public void test_FillInterested_RacingWith_BufferRelease() throws Exception

View File

@ -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 // TODO in Jetty-10 convert this enum to a class so that extra custom modes can be defined dynamically
LEGACY(sectionsBySpec("LEGACY")), 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")), CUSTOM0(sectionsByProperty("CUSTOM0")),
CUSTOM1(sectionsByProperty("CUSTOM1")), CUSTOM1(sectionsByProperty("CUSTOM1")),
CUSTOM2(sectionsByProperty("CUSTOM2")), CUSTOM2(sectionsByProperty("CUSTOM2")),
@ -59,17 +64,25 @@ public enum HttpCompliance
int i=0; int i=0;
switch(elements[i]) switch(elements[i])
{ {
case "0":
sections = EnumSet.noneOf(HttpComplianceSection.class);
i++;
break;
case "*":
i++;
sections = EnumSet.allOf(HttpComplianceSection.class);
break;
case "RFC2616": case "RFC2616":
sections = EnumSet.complementOf(EnumSet.of( sections = EnumSet.complementOf(EnumSet.of(
HttpComplianceSection.RFC7230_3_2_4_NO_WS_AFTER_FIELD_NAME,
HttpComplianceSection.RFC7230_3_2_4_NO_FOLDING, HttpComplianceSection.RFC7230_3_2_4_NO_FOLDING,
HttpComplianceSection.RFC7230_A2_NO_HTTP_9)); HttpComplianceSection.RFC7230_A2_NO_HTTP_9));
i++; i++;
break; break;
case "RFC7230": case "RFC7230":
case "*":
i++; i++;
sections = EnumSet.allOf(HttpComplianceSection.class); sections = EnumSet.allOf(HttpComplianceSection.class);
break; break;
@ -79,10 +92,6 @@ public enum HttpCompliance
i++; i++;
break; break;
case "0":
sections = EnumSet.noneOf(HttpComplianceSection.class);
i++;
break;
default: default:
sections = EnumSet.noneOf(HttpComplianceSection.class); sections = EnumSet.noneOf(HttpComplianceSection.class);
break; break;
@ -135,8 +144,6 @@ public enum HttpCompliance
return __required.get(section); return __required.get(section);
} }
private final EnumSet<HttpComplianceSection> _sections; private final EnumSet<HttpComplianceSection> _sections;
private HttpCompliance(EnumSet<HttpComplianceSection> sections) private HttpCompliance(EnumSet<HttpComplianceSection> sections)

View File

@ -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"), 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) HttpComplianceSection(String url,String description)
{ {
this.url = url;
this.description = description;
} }
public String getURL()
{
return url;
}
public String getDescription()
{
return description;
}
} }

View File

@ -35,8 +35,6 @@ import org.eclipse.jetty.util.Utf8StringBuilder;
import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger; 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.CARRIAGE_RETURN;
import static org.eclipse.jetty.http.HttpTokens.LINE_FEED; import static org.eclipse.jetty.http.HttpTokens.LINE_FEED;
import static org.eclipse.jetty.http.HttpTokens.SPACE; import static org.eclipse.jetty.http.HttpTokens.SPACE;
@ -1068,7 +1066,7 @@ public class HttpParser
case HttpTokens.SPACE: case HttpTokens.SPACE:
case HttpTokens.TAB: 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"); throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding");
// header value without name - continuation? // header value without name - continuation?
@ -1279,6 +1277,7 @@ public class HttpParser
} }
// Fallthrough // Fallthrough
case WS_AFTER_NAME: case WS_AFTER_NAME:
if (b==HttpTokens.COLON) if (b==HttpTokens.COLON)
{ {
@ -1293,7 +1292,7 @@ public class HttpParser
break; break;
} }
if (b==HttpTokens.LINE_FEED && !complianceViolation(HttpComplianceSection.RFC7230_3_2_FIELD_COLON,null)) if (b==HttpTokens.LINE_FEED)
{ {
if (_headerString==null) if (_headerString==null)
{ {
@ -1304,8 +1303,11 @@ public class HttpParser
_valueString=""; _valueString="";
_length=-1; _length=-1;
setState(FieldState.FIELD); if (!complianceViolation(HttpComplianceSection.RFC7230_3_2_FIELD_COLON,_headerString))
break; {
setState(FieldState.FIELD);
break;
}
} }
//Ignore trailing whitespaces //Ignore trailing whitespaces

View File

@ -27,6 +27,7 @@ import java.util.List;
import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HostPortHttpField;
import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpComplianceSection;
import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpGenerator; import org.eclipse.jetty.http.HttpGenerator;
@ -505,7 +506,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
} }
@Override @Override
public void onComplianceViolation(HttpCompliance compliance, HttpCompliance required, String reason) public void onComplianceViolation(HttpCompliance compliance, HttpComplianceSection violation, String reason)
{ {
if (_httpConnection.isRecordHttpComplianceViolations()) if (_httpConnection.isRecordHttpComplianceViolations())
{ {
@ -513,10 +514,11 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
{ {
_complianceViolations = new ArrayList<>(); _complianceViolations = new ArrayList<>();
} }
String violation = String.format("%s<%s: %s for %s", compliance, required, reason, getHttpTransport()); String record = String.format("%s (see %s) in mode %s for %s in %s",
_complianceViolations.add(violation); violation.getDescription(), violation.getURL(), compliance, reason, getHttpTransport());
_complianceViolations.add(record);
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug(violation); LOG.debug(record);
} }
} }
} }

View File

@ -105,7 +105,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
_channel = newHttpChannel(); _channel = newHttpChannel();
_input = _channel.getRequest().getHttpInput(); _input = _channel.getRequest().getHttpInput();
_parser = newHttpParser(compliance); _parser = newHttpParser(compliance);
_recordHttpComplianceViolations=recordComplianceViolations; _recordHttpComplianceViolations = recordComplianceViolations;
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("New HTTP Connection {}", this); LOG.debug("New HTTP Connection {}", this);
} }

View File

@ -147,7 +147,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString()); String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200 OK")); assertThat("Response status", response, containsString("HTTP/1.1 200 OK"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon")); assertThat("Response headers", response, containsString("X-Http-Violation-0: Fields must have a Colon"));
assertThat("Response body", response, containsString("[Name] = []")); assertThat("Response body", response, containsString("[Name] = []"));
} }
@ -164,7 +164,7 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString()); String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200")); assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon")); assertThat("Response headers", response, containsString("X-Http-Violation-0: Fields must have a Colon"));
assertThat("Response body", response, containsString("[Name] = []")); assertThat("Response body", response, containsString("[Name] = []"));
} }
@ -182,7 +182,8 @@ public class ComplianceViolations2616Test
String response = connector.getResponse(req1.toString()); String response = connector.getResponse(req1.toString());
assertThat("Response status", response, containsString("HTTP/1.1 200")); assertThat("Response status", response, containsString("HTTP/1.1 200"));
assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2.4 folding")); assertThat("Response headers", response, containsString("X-Http-Violation-0: No line Folding (see https://tools.ietf.org/html/rfc7230#section-3.2.4) in mode RFC2616 for"));
assertThat("Response body", response, containsString("[Name] = [Some Value]")); assertThat("Response body", response, containsString("[Name] = [Some Value]"));
} }
} }