From 73ad887ce8bd414da7ff8264014564cfafa9d6d9 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 13 Mar 2019 09:17:46 +1100 Subject: [PATCH] Issue #3012 Compliance modes. Added an abstraction of general compliance mode, violation and listener Signed-off-by: Greg Wilkins --- .../jetty/http/ComplianceViolation.java | 47 +++++++++++++++++ .../eclipse/jetty/http/HttpCompliance.java | 51 ++++++++++++++----- .../org/eclipse/jetty/http/HttpParser.java | 23 +++------ .../eclipse/jetty/http/HttpParserTest.java | 10 ++-- .../jetty/server/HttpChannelOverHttp.java | 7 +-- 5 files changed, 101 insertions(+), 37 deletions(-) create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java b/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java new file mode 100644 index 00000000000..2e8cbd84d29 --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java @@ -0,0 +1,47 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// +package org.eclipse.jetty.http; + +import java.util.Set; + +public interface ComplianceViolation +{ + String getName(); + String getURL(); + String getDescription(); + + default boolean isAllowedBy(Mode mode) + { + return mode.allows(this); + } + + interface Mode + { + String getName(); + boolean allows(ComplianceViolation violation); + Set getKnown(); + Set getAllowed(); + } + + interface Listener + { + default void onComplianceViolation(Mode mode, ComplianceViolation violation, String details) + { + } + } +} 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 fb2ef8d1546..ade65209b21 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,12 +38,12 @@ import static java.util.EnumSet.of; * A Compliance mode consists of a set of {@link Violation}s which are applied * when the mode is enabled. */ -public final class HttpCompliance +public final class HttpCompliance implements ComplianceViolation.Mode { // These are compliance violations, which may optionally be allowed by the compliance mode, which mean that // the relevant section of the RFC is not strictly adhered to. - public enum Violation + public enum Violation implements ComplianceViolation { CASE_SENSITIVE_FIELD_NAME("https://tools.ietf.org/html/rfc7230#section-3.2", "Field name is case-insensitive"), CASE_INSENSITIVE_METHOD("https://tools.ietf.org/html/rfc7230#section-3.1.1", "Method is case-sensitive"), @@ -54,8 +54,8 @@ public final class HttpCompliance WHITESPACE_AFTER_FIELD_NAME("https://tools.ietf.org/html/rfc7230#section-3.2.4", "Whitespace not allowed after field name"), NO_COLON_AFTER_FIELD_NAME("https://tools.ietf.org/html/rfc7230#section-3.2", "Fields must have a Colon"); - public final String url; - public final String description; + private final String url; + private final String description; Violation(String url, String description) { @@ -63,18 +63,23 @@ public final class HttpCompliance this.description = description; } - public boolean isAllowedBy(HttpCompliance compliance) + @Override + public String getName() { - return compliance.isAllowed(this); + return name(); } - } - private final String _name; - private final Set _violations; + @Override + public String getURL() + { + return url; + } - public boolean isAllowed(Violation violation) - { - return _violations.contains(violation); + @Override + public String getDescription() + { + return description; + } } public final static HttpCompliance RFC7230 = new HttpCompliance("RFC7230", noneOf(Violation.class)); @@ -96,6 +101,8 @@ public final class HttpCompliance return violationBySpec(s==null?"*":s); } + + /** * Create violation set from string *

@@ -165,6 +172,10 @@ public final class HttpCompliance return sections; } + + private final String _name; + private final Set _violations; + private HttpCompliance(String name, Set violations) { Objects.nonNull(violations); @@ -172,6 +183,13 @@ public final class HttpCompliance _violations = unmodifiableSet(copyOf(violations)); } + @Override + public boolean allows(ComplianceViolation violation) + { + return _violations.contains(violation); + } + + @Override public String getName() { return _name; @@ -181,11 +199,19 @@ public final class HttpCompliance * Get the set of {@link Violation}s allowed by this compliance mode. * @return The immutable set of {@link Violation}s allowed by this compliance mode. */ + @Override public Set getAllowed() { return _violations; } + @Override + public Set getKnown() + { + return EnumSet.allOf(Violation.class); + } + + // TODO javadoc public HttpCompliance with(String name, Violation... violations) { EnumSet union = _violations.isEmpty()?EnumSet.noneOf(Violation.class):copyOf(_violations); @@ -193,6 +219,7 @@ public final class HttpCompliance return new HttpCompliance(name, union); } + // TODO javadoc public HttpCompliance without(String name, Violation... violations) { EnumSet remainder = _violations.isEmpty()?EnumSet.noneOf(Violation.class):copyOf(_violations); 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 aec795421ad..395cb3a195f 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 @@ -146,7 +146,7 @@ public class HttpParser private final HttpHandler _handler; private final RequestHandler _requestHandler; private final ResponseHandler _responseHandler; - private final ComplianceHandler _complianceHandler; + private final ComplianceViolation.Listener _complianceHandler; private final int _maxHeaderBytes; private final HttpCompliance _compliance; private HttpField _field; @@ -284,7 +284,7 @@ public class HttpParser _responseHandler=responseHandler; _maxHeaderBytes=maxHeaderBytes; _compliance=compliance; - _complianceHandler=(ComplianceHandler)(_handler instanceof ComplianceHandler?_handler:null); + _complianceHandler=(ComplianceViolation.Listener)(_handler instanceof ComplianceViolation.Listener?_handler:null); } /* ------------------------------------------------------------------------------- */ @@ -298,10 +298,10 @@ public class HttpParser if (violation.isAllowedBy(_compliance)) { if (_complianceHandler!=null) - _complianceHandler.onComplianceViolation(_compliance, violation, violation.description); + _complianceHandler.onComplianceViolation(_compliance, violation, violation.getDescription()); return; } - throw new BadMessageException(HttpStatus.BAD_REQUEST_400,violation.description); + throw new BadMessageException(HttpStatus.BAD_REQUEST_400,violation.getDescription()); } /* ------------------------------------------------------------------------------- */ @@ -314,7 +314,7 @@ public class HttpParser if (violation.isAllowedBy(_compliance)) { if (_complianceHandler!=null) - _complianceHandler.onComplianceViolation(_compliance, violation, violation.description); + _complianceHandler.onComplianceViolation(_compliance, violation, violation.getDescription()); return true; } return false; @@ -956,7 +956,7 @@ public class HttpParser { checkViolation(MULTIPLE_CONTENT_LENGTHS); if (convertContentLength(_valueString)!=_contentLength) - throw new BadMessageException(HttpStatus.BAD_REQUEST_400,MULTIPLE_CONTENT_LENGTHS.description); + throw new BadMessageException(HttpStatus.BAD_REQUEST_400,MULTIPLE_CONTENT_LENGTHS.getDescription()); } _hasContentLength = true; @@ -1942,17 +1942,6 @@ public class HttpParser boolean startResponse(HttpVersion version, int status, String reason); } - /* ------------------------------------------------------------------------------- */ - /* ------------------------------------------------------------------------------- */ - /* ------------------------------------------------------------------------------- */ - public interface ComplianceHandler extends HttpHandler - { - default void onComplianceViolation(HttpCompliance compliance, HttpCompliance.Violation violation, String details) - { - - } - } - /* ------------------------------------------------------------------------------- */ @SuppressWarnings("serial") private static class IllegalCharacterException extends BadMessageException 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 9189cb0e389..f9a8f95c0ba 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 @@ -30,10 +30,10 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import static org.eclipse.jetty.http.HttpCompliance.Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH; -import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_SENSITIVE_FIELD_NAME; import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_INSENSITIVE_METHOD; +import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_SENSITIVE_FIELD_NAME; import static org.eclipse.jetty.http.HttpCompliance.Violation.MULTILINE_FIELD_VALUE; +import static org.eclipse.jetty.http.HttpCompliance.Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; @@ -2188,9 +2188,9 @@ public class HttpParserTest private boolean _early; private boolean _headerCompleted; private boolean _messageCompleted; - private final List _complianceViolation = new ArrayList<>(); + private final List _complianceViolation = new ArrayList<>(); - private class Handler implements HttpParser.RequestHandler, HttpParser.ResponseHandler, HttpParser.ComplianceHandler + private class Handler implements HttpParser.RequestHandler, HttpParser.ResponseHandler, ComplianceViolation.Listener { private boolean _headerCacheCaseSensitive; @@ -2316,7 +2316,7 @@ public class HttpParserTest } @Override - public void onComplianceViolation(HttpCompliance compliance, HttpCompliance.Violation violation, String reason) + public void onComplianceViolation(ComplianceViolation.Mode mode, ComplianceViolation violation, String reason) { _complianceViolation.add(violation); } 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 b0f0759227b..fe2abc7d17e 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 @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.List; import org.eclipse.jetty.http.BadMessageException; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpField; @@ -46,7 +47,7 @@ import org.eclipse.jetty.util.log.Logger; /** * A HttpChannel customized to be transported over the HTTP/1 protocol */ -public class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandler, HttpParser.ComplianceHandler +public class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandler, ComplianceViolation.Listener { private static final Logger LOG = Log.getLogger(HttpChannelOverHttp.class); private final static HttpField PREAMBLE_UPGRADE_H2C = new HttpField(HttpHeader.UPGRADE, "h2c"); @@ -521,7 +522,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque } @Override - public void onComplianceViolation(HttpCompliance compliance, HttpCompliance.Violation violation, String details) + public void onComplianceViolation(ComplianceViolation.Mode mode, ComplianceViolation violation, String details) { if (_httpConnection.isRecordHttpComplianceViolations()) { @@ -530,7 +531,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque _complianceViolations = new ArrayList<>(); } String record = String.format("%s (see %s) in mode %s for %s in %s", - violation.description, violation.url, compliance, details, getHttpTransport()); + violation.getDescription(), violation.getURL(), mode, details, getHttpTransport()); _complianceViolations.add(record); if (LOG.isDebugEnabled()) LOG.debug(record);