Issue #3012 Compliance modes.

clean up violation testing and reporting in HttpParser

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-03-13 09:45:51 +11:00
parent 73ad887ce8
commit e92808156d
2 changed files with 61 additions and 59 deletions

View File

@ -18,8 +18,11 @@
package org.eclipse.jetty.http;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import org.eclipse.jetty.util.log.Log;
@ -92,6 +95,9 @@ public final class HttpCompliance implements ComplianceViolation.Mode
Violation.MULTIPLE_CONTENT_LENGTHS);
public final static HttpCompliance RFC7230_LEGACY = RFC7230.with("RFC7230_LEGACY", Violation.CASE_INSENSITIVE_METHOD);
public final static List<HttpCompliance> KNOWN_MODES = Arrays.asList(RFC7230,RFC2616,LEGACY,RFC2616_LEGACY,RFC7230_LEGACY);
public static final String VIOLATIONS_ATTR = "org.eclipse.jetty.http.compliance.violations";
private static final Logger LOG = Log.getLogger(HttpParser.class);
@ -113,6 +119,8 @@ public final class HttpCompliance implements ComplianceViolation.Mode
* but not https://tools.ietf.org/html/rfc7230</dd>
* <dt>RFC7230</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc7230</dd>
* </dl>
* <dt>name</dt><dd>Any of the known modes defined in {@link HttpCompliance#KNOWN_MODES}</dd>
* </dl>
* The remainder of the list can contain then names of {@link Violation}s to include them in the mode, or prefixed
* with a '-' to exclude thm from the mode.
* <p>
@ -121,38 +129,30 @@ public final class HttpCompliance implements ComplianceViolation.Mode
{
EnumSet<Violation> sections;
String[] elements = spec.split("\\s*,\\s*");
int i=0;
switch(elements[i])
switch(elements[0])
{
case "0":
sections = noneOf(Violation.class);
i++;
break;
case "*":
sections = allOf(Violation.class);
i++;
break;
case "RFC2616":
sections = copyOf(RFC2616.getAllowed());
i++;
break;
case "RFC7230":
sections = copyOf(RFC7230.getAllowed());
i++;
break;
default:
sections = noneOf(Violation.class);
break;
{
Optional<HttpCompliance> mode =
KNOWN_MODES.stream().filter(m -> m.getName().equals(elements[0])).findFirst();
if (mode.isPresent())
sections = copyOf(mode.get().getAllowed());
else
sections = noneOf(Violation.class);
}
}
while(i<elements.length)
for (int i=1; i<elements.length; i++)
{
String element = elements[i++];
String element = elements[i];
boolean exclude = element.startsWith("-");
if (exclude)
element = element.substring(1);
@ -211,7 +211,12 @@ public final class HttpCompliance implements ComplianceViolation.Mode
return EnumSet.allOf(Violation.class);
}
// TODO javadoc
/**
* Create a new HttpCompliance mode that includes the passed {@link Violation}s.
* @param name The name of the new mode
* @param violations The violations to include
* @return A new {@link HttpCompliance} mode.
*/
public HttpCompliance with(String name, Violation... violations)
{
EnumSet<Violation> union = _violations.isEmpty()?EnumSet.noneOf(Violation.class):copyOf(_violations);
@ -219,7 +224,12 @@ public final class HttpCompliance implements ComplianceViolation.Mode
return new HttpCompliance(name, union);
}
// TODO javadoc
/**
* Create a new HttpCompliance mode that excludes the passed {@link Violation}s.
* @param name The name of the new mode
* @param violations The violations to exclude
* @return A new {@link HttpCompliance} mode.
*/
public HttpCompliance without(String name, Violation... violations)
{
EnumSet<Violation> remainder = _violations.isEmpty()?EnumSet.noneOf(Violation.class):copyOf(_violations);

View File

@ -36,7 +36,9 @@ import org.eclipse.jetty.util.log.Logger;
import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_SENSITIVE_FIELD_NAME;
import static org.eclipse.jetty.http.HttpCompliance.Violation.MULTIPLE_CONTENT_LENGTHS;
import static org.eclipse.jetty.http.HttpCompliance.Violation.NO_COLON_AFTER_FIELD_NAME;
import static org.eclipse.jetty.http.HttpCompliance.Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH;
import static org.eclipse.jetty.http.HttpCompliance.Violation.WHITESPACE_AFTER_FIELD_NAME;
/* ------------------------------------------------------------ */
@ -146,9 +148,9 @@ public class HttpParser
private final HttpHandler _handler;
private final RequestHandler _requestHandler;
private final ResponseHandler _responseHandler;
private final ComplianceViolation.Listener _complianceHandler;
private final ComplianceViolation.Listener _complianceListener;
private final int _maxHeaderBytes;
private final HttpCompliance _compliance;
private final HttpCompliance _complianceMode;
private HttpField _field;
private HttpHeader _header;
private String _headerString;
@ -283,8 +285,8 @@ public class HttpParser
_requestHandler=requestHandler;
_responseHandler=responseHandler;
_maxHeaderBytes=maxHeaderBytes;
_compliance=compliance;
_complianceHandler=(ComplianceViolation.Listener)(_handler instanceof ComplianceViolation.Listener?_handler:null);
_complianceMode =compliance;
_complianceListener =(ComplianceViolation.Listener)(_handler instanceof ComplianceViolation.Listener?_handler:null);
}
/* ------------------------------------------------------------------------------- */
@ -295,45 +297,32 @@ public class HttpParser
protected void checkViolation(Violation violation) throws BadMessageException
{
if (violation.isAllowedBy(_compliance))
{
if (_complianceHandler!=null)
_complianceHandler.onComplianceViolation(_compliance, violation, violation.getDescription());
return;
}
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,violation.getDescription());
if (violation.isAllowedBy(_complianceMode))
reportComplianceViolation(violation, violation.getDescription());
else
throw new BadMessageException(HttpStatus.BAD_REQUEST_400,violation.getDescription());
}
/* ------------------------------------------------------------------------------- */
/** Check RFC compliance violation
* @param violation The compliance violation
* @return True if the current compliance level is set so as to Not allow this violation
*/
protected boolean violationAllowed(Violation violation)
protected void reportComplianceViolation(Violation violation)
{
if (violation.isAllowedBy(_compliance))
{
if (_complianceHandler!=null)
_complianceHandler.onComplianceViolation(_compliance, violation, violation.getDescription());
return true;
}
return false;
reportComplianceViolation(violation,violation.getDescription());
}
/* ------------------------------------------------------------------------------- */
protected void handleViolation(Violation section, String reason)
protected void reportComplianceViolation(Violation violation, String reason)
{
if (_complianceHandler!=null)
_complianceHandler.onComplianceViolation(_compliance,section,reason);
if (_complianceListener !=null)
_complianceListener.onComplianceViolation(_complianceMode,violation,reason);
}
/* ------------------------------------------------------------------------------- */
protected String caseInsensitiveHeader(String orig, String normative)
{
if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_compliance))
if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode))
return normative;
if (!orig.equals(normative))
handleViolation(CASE_SENSITIVE_FIELD_NAME,orig);
reportComplianceViolation(CASE_SENSITIVE_FIELD_NAME,orig);
return orig;
}
@ -618,13 +607,13 @@ public class HttpParser
_length=_string.length();
_methodString=takeString();
if (Violation.CASE_INSENSITIVE_METHOD.isAllowedBy(_compliance))
if (Violation.CASE_INSENSITIVE_METHOD.isAllowedBy(_complianceMode))
{
HttpMethod method=HttpMethod.INSENSITIVE_CACHE.get(_methodString);
if (method!=null)
{
if (!method.asString().equals(_methodString))
handleViolation(Violation.CASE_INSENSITIVE_METHOD,_methodString);
reportComplianceViolation(Violation.CASE_INSENSITIVE_METHOD,_methodString);
_methodString = method.asString();
}
}
@ -1002,7 +991,7 @@ public class HttpParser
if (!(_field instanceof HostPortHttpField) && _valueString!=null && !_valueString.isEmpty())
{
_field=new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_compliance)?_headerString:_header.asString(),
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode)?_headerString:_header.asString(),
_valueString);
add_to_connection_trie=_fieldCache!=null;
}
@ -1216,13 +1205,13 @@ public class HttpParser
String n = cached_field.getName();
String v = cached_field.getValue();
if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_compliance))
if (CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode))
{
// Have to get the fields exactly from the buffer to match case
String en = BufferUtil.toString(buffer,buffer.position()-1,n.length(),StandardCharsets.US_ASCII);
if (!n.equals(en))
{
handleViolation(CASE_SENSITIVE_FIELD_NAME,en);
reportComplianceViolation(CASE_SENSITIVE_FIELD_NAME,en);
n = en;
cached_field = new HttpField(cached_field.getHeader(),n,v);
}
@ -1295,9 +1284,10 @@ public class HttpParser
case SPACE:
case HTAB:
//Ignore trailing whitespaces ?
if (violationAllowed(Violation.WHITESPACE_AFTER_FIELD_NAME))
if (WHITESPACE_AFTER_FIELD_NAME.isAllowedBy(_complianceMode))
{
_headerString=takeString();
reportComplianceViolation(WHITESPACE_AFTER_FIELD_NAME,"Space after "+_headerString);
_header=HttpHeader.CACHE.get(_headerString);
_length=-1;
setState(FieldState.WS_AFTER_NAME);
@ -1319,11 +1309,12 @@ public class HttpParser
_valueString="";
_length=-1;
if (violationAllowed(Violation.NO_COLON_AFTER_FIELD_NAME))
if (NO_COLON_AFTER_FIELD_NAME.isAllowedBy(_complianceMode))
{
reportComplianceViolation(NO_COLON_AFTER_FIELD_NAME,"Field "+_headerString);
setState(FieldState.FIELD);
break;
}
}
throw new IllegalCharacterException(_state,t,buffer);
case ALPHA:
@ -1351,11 +1342,12 @@ public class HttpParser
break;
case LF:
if (violationAllowed(Violation.NO_COLON_AFTER_FIELD_NAME))
if (NO_COLON_AFTER_FIELD_NAME.isAllowedBy(_complianceMode))
{
reportComplianceViolation(NO_COLON_AFTER_FIELD_NAME,"Field "+_headerString);
setState(FieldState.FIELD);
break;
}
}
throw new IllegalCharacterException(_state,t,buffer);
default: