Skip to content

Commit

Permalink
URI compliance modes for #6001 (#6006)
Browse files Browse the repository at this point in the history 
* Fix #4275 separate compliance modes for ambiguous URI segments and separators

default modes allows both ambiguous separators and segments, but still forbids ambiguous parameters

Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@gregw @joakime
gregw and joakime committed Mar 2, 2021
1 parent d9eefc9 commit 06e1a7e
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 114 deletions.
26 changes: 23 additions & 3 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View file
Expand Up @@ -51,7 +51,8 @@ public interface HttpURI
enum Ambiguous
{
SEGMENT,
SEPARATOR
SEPARATOR,
PARAM
}

static Mutable build()
Expand Down Expand Up @@ -139,7 +140,7 @@ static Immutable from(String scheme, String host, int port, String pathQuery)
boolean isAbsolute();

/**
* @return True if the URI has either an {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}.
* @return True if the URI has either an {@link #hasAmbiguousParameter()}, {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}.
*/
boolean isAmbiguous();

Expand All @@ -153,6 +154,11 @@ static Immutable from(String scheme, String host, int port, String pathQuery)
*/
boolean hasAmbiguousSeparator();

/**
* @return True if the URI has a possibly ambiguous path parameter like '..;'
*/
boolean hasAmbiguousParameter();

default URI toURI()
{
try
Expand Down Expand Up @@ -374,6 +380,12 @@ public boolean hasAmbiguousSeparator()
return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR);
}

@Override
public boolean hasAmbiguousParameter()
{
return _ambiguous.contains(Ambiguous.PARAM);
}

@Override
public String toString()
{
Expand Down Expand Up @@ -709,6 +721,12 @@ public boolean hasAmbiguousSeparator()
return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR);
}

@Override
public boolean hasAmbiguousParameter()
{
return _ambiguous.contains(Ambiguous.PARAM);
}

public Mutable normalize()
{
HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme);
Expand Down Expand Up @@ -1241,8 +1259,10 @@ private void checkSegment(String uri, int segment, int end, boolean param)
if (!_ambiguous.contains(Ambiguous.SEGMENT))
{
Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment);
if (ambiguous == Boolean.TRUE || (param && ambiguous == Boolean.FALSE))
if (ambiguous == Boolean.TRUE)
_ambiguous.add(Ambiguous.SEGMENT);
else if (param && ambiguous == Boolean.FALSE)
_ambiguous.add(Ambiguous.PARAM);
}
}
}
Expand Down
96 changes: 68 additions & 28 deletions jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java
View file
Expand Up @@ -15,7 +15,6 @@

import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -30,27 +29,39 @@

/**
* URI compliance modes for Jetty request handling.
* A Compliance mode consists of a set of {@link Violation}s which are applied
* A Compliance mode consists of a set of {@link Violation}s which are allowed
* when the mode is enabled.
*/
public final class UriCompliance implements ComplianceViolation.Mode
{
protected static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class);

// 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.
/**
* These are URI compliance violations, which may be allowed by the compliance mode. Currently all these
* violations are for additional criteria in excess of the strict requirements of rfc3986.
*/
public enum Violation implements ComplianceViolation
{
/**
* Ambiguous path segments e.g. <code>/foo/%2e%2e/bar</code>
*/
AMBIGUOUS_PATH_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path segment"),
AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator");
/**
* Ambiguous path separator within a URI segment e.g. <code>/foo/b%2fr</code>
*/
AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator"),
/**
* Ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
*/
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter");

private final String url;
private final String description;
private final String _url;
private final String _description;

Violation(String url, String description)
{
this.url = url;
this.description = description;
_url = url;
_description = description;
}

@Override
Expand All @@ -62,24 +73,50 @@ public String getName()
@Override
public String getURL()
{
return url;
return _url;
}

@Override
public String getDescription()
{
return description;
return _description;
}
}

public static final UriCompliance SAFE = new UriCompliance("SAFE", noneOf(Violation.class));
public static final UriCompliance STRICT = new UriCompliance("STRICT", allOf(Violation.class));
private static final List<UriCompliance> KNOWN_MODES = Arrays.asList(SAFE, STRICT);
/**
* The default compliance mode that extends RFC3986 compliance with additional violations to avoid ambiguous URIs
*/
public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", noneOf(Violation.class));

/**
* LEGACY compliance mode that disallows only ambiguous path parameters as per Jetty-9.4
*/
public static final UriCompliance LEGACY = new UriCompliance("LEGACY", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR));

/**
* Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations
*/
public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", allOf(Violation.class));

/**
* @deprecated equivalent to DEFAULT
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated
public static final UriCompliance SAFE = new UriCompliance("SAFE", DEFAULT.getAllowed());

/**
* @deprecated equivalent to RFC3986
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated
public static final UriCompliance STRICT = new UriCompliance("STRICT", RFC3986.getAllowed());

private static final AtomicInteger __custom = new AtomicInteger();

public static UriCompliance valueOf(String name)
{
for (UriCompliance compliance : KNOWN_MODES)
for (UriCompliance compliance : Arrays.asList(DEFAULT, LEGACY, RFC3986, STRICT, SAFE))
{
if (compliance.getName().equals(name))
return compliance;
Expand All @@ -91,20 +128,23 @@ public static UriCompliance valueOf(String name)
/**
* Create compliance set from string.
* <p>
* Format:
* Format: &lt;BASE&gt;[,[-]&lt;violation&gt;]...
* </p>
* <p>BASE is one of:</p>
* <dl>
* <dt>0</dt><dd>No {@link Violation}s</dd>
* <dt>*</dt><dd>All {@link Violation}s</dd>
* <dt>RFC2616</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc2616,
* 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>
* <dt>name</dt><dd>Any of the known modes defined in {@link UriCompliance#KNOWN_MODES}</dd>
* <dt>&lt;name&gt;</dt><dd>The name of a static instance of {@link UriCompliance} (e.g. {@link UriCompliance#RFC3986}).
* </dl>
* <p>
* 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.
* with a '-' to exclude thm from the mode. Examples are:
* </p>
* <dl>
* <dt><code>0,AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Only allow {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* <dt><code>*,-AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Only all except {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* <dt><code>RFC3986,AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Same as RFC3986 plus {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* </dl>
*
* @param spec A string in the format of a comma separated list starting with one of the following strings:
* @return the compliance from the string spec
Expand Down Expand Up @@ -150,19 +190,19 @@ public static UriCompliance from(String spec)
}

private final String _name;
private final Set<Violation> _violations;
private final Set<Violation> _allowed;

private UriCompliance(String name, Set<Violation> violations)
{
Objects.requireNonNull(violations);
_name = name;
_violations = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations));
_allowed = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations));
}

@Override
public boolean allows(ComplianceViolation violation)
{
return violation instanceof Violation && _violations.contains(violation);
return violation instanceof Violation && _allowed.contains(violation);
}

@Override
Expand All @@ -179,7 +219,7 @@ public String getName()
@Override
public Set<Violation> getAllowed()
{
return _violations;
return _allowed;
}

@Override
Expand All @@ -197,7 +237,7 @@ public Set<Violation> getKnown()
*/
public UriCompliance with(String name, Violation... violations)
{
Set<Violation> union = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations);
Set<Violation> union = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed);
union.addAll(copyOf(violations));
return new UriCompliance(name, union);
}
Expand All @@ -211,15 +251,15 @@ public UriCompliance with(String name, Violation... violations)
*/
public UriCompliance without(String name, Violation... violations)
{
Set<Violation> remainder = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations);
Set<Violation> remainder = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed);
remainder.removeAll(copyOf(violations));
return new UriCompliance(name, remainder);
}

@Override
public String toString()
{
return String.format("%s%s", _name, _violations);
return String.format("%s%s", _name, _allowed);
}

private static Set<Violation> copyOf(Violation[] violations)
Expand Down

0 comments on commit 06e1a7e

Please sign in to comment.