Issue #3012 Compliance modes.

Added an abstraction of general compliance mode, violation and listener

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-03-13 09:17:46 +11:00
parent 0ecbdec9d0
commit 73ad887ce8
5 changed files with 101 additions and 37 deletions

View File

@ -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<? extends ComplianceViolation> getKnown();
Set<? extends ComplianceViolation> getAllowed();
}
interface Listener
{
default void onComplianceViolation(Mode mode, ComplianceViolation violation, String details)
{
}
}
}

View File

@ -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<Violation> _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
* <p>
@ -165,6 +172,10 @@ public final class HttpCompliance
return sections;
}
private final String _name;
private final Set<Violation> _violations;
private HttpCompliance(String name, Set<Violation> 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<Violation> getAllowed()
{
return _violations;
}
@Override
public Set<Violation> getKnown()
{
return EnumSet.allOf(Violation.class);
}
// TODO javadoc
public HttpCompliance with(String name, Violation... violations)
{
EnumSet<Violation> 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<Violation> remainder = _violations.isEmpty()?EnumSet.noneOf(Violation.class):copyOf(_violations);

View File

@ -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

View File

@ -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<HttpCompliance.Violation> _complianceViolation = new ArrayList<>();
private final List<ComplianceViolation> _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);
}

View File

@ -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);