From b6df9508c6991dd672851cc405f3250d733bc03e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 14 Dec 2015 14:58:12 -0700 Subject: [PATCH] 484350 - Allow GzipHandler path include/exclude to use regex + Overhauled IncludeExclude to use java 8 predicate + Introduced PathSpecSet to standardize path IncludeExclude + GzipHandler now uses PathSpecSet for paths --- .../java/org/eclipse/jetty/http/PathMap.java | 11 +- .../jetty/http/pathmap/PathSpecSet.java | 216 ++++++++++++++++++ .../server/handler/gzip/GzipHandler.java | 51 ++++- .../server/handler/gzip/GzipHandlerTest.java | 42 ++++ .../eclipse/jetty/util/IncludeExclude.java | 72 ++++-- .../java/org/eclipse/jetty/util/RegexSet.java | 10 +- .../jetty/util/IncludeExcludeTest.java | 19 +- 7 files changed, 378 insertions(+), 43 deletions(-) create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecSet.java create mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java b/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java index bb30c3b4d15..448be2e26db 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/PathMap.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.StringTokenizer; import java.util.function.BiFunction; +import java.util.function.Predicate; import org.eclipse.jetty.util.ArrayTernaryTrie; import org.eclipse.jetty.util.RegexSet; @@ -592,9 +593,8 @@ public class PathMap extends HashMap } } - public static class PathSet extends AbstractSet + public static class PathSet extends AbstractSet implements Predicate { - public static final BiFunction MATCHER=(s,e)->{return s.containsMatch(e);}; private final PathMap _map = new PathMap<>(); @Override @@ -627,12 +627,15 @@ public class PathMap extends HashMap return _map.containsKey(o); } + @Override + public boolean test(String s) + { + return _map.containsMatch(s); + } public boolean containsMatch(String s) { return _map.containsMatch(s); } - - } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecSet.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecSet.java new file mode 100644 index 00000000000..0fd66f01d0b --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecSet.java @@ -0,0 +1,216 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.pathmap; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.Predicate; + +/** + * A Set of PathSpec strings. + *

+ * Used by {@link org.eclipse.jetty.util.IncludeExclude} logic + */ +public class PathSpecSet implements Set, Predicate +{ + private final Set specs = new TreeSet<>(); + + @Override + public boolean test(String s) + { + for (PathSpec spec : specs) + { + if (spec.matches(s)) + { + return true; + } + } + return false; + } + + @Override + public boolean isEmpty() + { + return specs.isEmpty(); + } + + @Override + public Iterator iterator() + { + return new Iterator() + { + private Iterator iter = specs.iterator(); + + @Override + public boolean hasNext() + { + return iter.hasNext(); + } + + @Override + public String next() + { + PathSpec spec = iter.next(); + if (spec == null) + { + return null; + } + return spec.getDeclaration(); + } + }; + } + + @Override + public int size() + { + return specs.size(); + } + + @Override + public boolean contains(Object o) + { + if (o instanceof PathSpec) + { + return specs.contains(o); + } + if (o instanceof String) + { + return specs.contains(toPathSpec((String)o)); + } + return false; + } + + private PathSpec asPathSpec(Object o) + { + if (o == null) + { + return null; + } + if (o instanceof PathSpec) + { + return (PathSpec)o; + } + if (o instanceof String) + { + return toPathSpec((String)o); + } + return toPathSpec(o.toString()); + } + + private PathSpec toPathSpec(String rawSpec) + { + if ((rawSpec == null) || (rawSpec.length() < 1)) + { + throw new RuntimeException("Path Spec String must start with '^', '/', or '*.': got [" + rawSpec + "]"); + } + if (rawSpec.charAt(0) == '^') + { + return new RegexPathSpec(rawSpec); + } + else + { + return new ServletPathSpec(rawSpec); + } + } + + @Override + public Object[] toArray() + { + return toArray(new String[specs.size()]); + } + + @Override + public T[] toArray(T[] a) + { + int i = 0; + for (PathSpec spec : specs) + { + a[i++] = (T)spec.getDeclaration(); + } + return a; + } + + @Override + public boolean add(String e) + { + return specs.add(toPathSpec(e)); + } + + @Override + public boolean remove(Object o) + { + return specs.remove(asPathSpec(o)); + } + + @Override + public boolean containsAll(Collection coll) + { + for (Object o : coll) + { + if (!specs.contains(asPathSpec(o))) + return false; + } + return true; + } + + @Override + public boolean addAll(Collection coll) + { + boolean ret = false; + + for (String s : coll) + { + ret |= add(s); + } + + return ret; + } + + @Override + public boolean retainAll(Collection coll) + { + List collSpecs = new ArrayList<>(); + for (Object o : coll) + { + collSpecs.add(asPathSpec(o)); + } + return specs.retainAll(collSpecs); + } + + @Override + public boolean removeAll(Collection coll) + { + List collSpecs = new ArrayList<>(); + for (Object o : coll) + { + collSpecs.add(asPathSpec(o)); + } + return specs.removeAll(collSpecs); + } + + @Override + public void clear() + { + specs.clear(); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 8a14d13ab5f..da6182604ed 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -36,7 +36,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.http.PathMap; +import org.eclipse.jetty.http.pathmap.PathSpecSet; import org.eclipse.jetty.server.HttpOutput; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.HandlerWrapper; @@ -72,9 +72,9 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory // non-static, as other GzipHandler instances may have different configurations private final ThreadLocal _deflater = new ThreadLocal(); - private final IncludeExclude _agentPatterns=new IncludeExclude<>(RegexSet.class,RegexSet.MATCHER); + private final IncludeExclude _agentPatterns=new IncludeExclude<>(RegexSet.class); private final IncludeExclude _methods = new IncludeExclude<>(); - private final IncludeExclude _paths = new IncludeExclude<>(PathMap.PathSet.class,PathMap.PathSet.MATCHER); + private final IncludeExclude _paths = new IncludeExclude(PathSpecSet.class); private final IncludeExclude _mimeTypes = new IncludeExclude<>(); private HttpField _vary; @@ -144,9 +144,27 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory /* ------------------------------------------------------------ */ /** + * Add path to excluded paths list. + *

+ * There are 2 syntaxes supported, Servlet url-pattern based, and + * Regex based. This means that the initial characters on the path spec + * line are very strict, and determine the behavior of the path matching. + *

    + *
  • If the spec starts with '^' the spec is assumed to be + * a regex based path spec and will match with normal Java regex rules.
    • + *
  • If the spec starts with '/' then spec is assumed to be + * a Servlet url-pattern rules path spec for either an exact match + * or prefix based match.
    • + *
  • If the spec starts with '*.' then spec is assumed to be + * a Servlet url-pattern rules path spec for a suffix based match.
    • + *
  • All other syntaxes are unsupported
    • + *
+ *

+ * Note: inclusion takes precedence over exclude. + * * @param pathspecs Path specs (as per servlet spec) to exclude. If a * ServletContext is available, the paths are relative to the context path, - * otherwise they are absolute. + * otherwise they are absolute.
* For backward compatibility the pathspecs may be comma separated strings, but this * will not be supported in future versions. */ @@ -191,12 +209,27 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory /* ------------------------------------------------------------ */ /** - * Add path specs to include. Inclusion takes precedence over exclusion. + * Add path specs to include. + *

+ * There are 2 syntaxes supported, Servlet url-pattern based, and + * Regex based. This means that the initial characters on the path spec + * line are very strict, and determine the behavior of the path matching. + *

    + *
  • If the spec starts with '^' the spec is assumed to be + * a regex based path spec and will match with normal Java regex rules.
    • + *
  • If the spec starts with '/' then spec is assumed to be + * a Servlet url-pattern rules path spec for either an exact match + * or prefix based match.
    • + *
  • If the spec starts with '*.' then spec is assumed to be + * a Servlet url-pattern rules path spec for a suffix based match.
    • + *
  • All other syntaxes are unsupported
    • + *
+ *

+ * Note: inclusion takes precedence over exclude. + * * @param pathspecs Path specs (as per servlet spec) to include. If a * ServletContext is available, the paths are relative to the context path, * otherwise they are absolute - * For backward compatibility the pathspecs may be comma separated strings, but this - * will not be supported in future versions. */ public void addIncludedPaths(String... pathspecs) { @@ -334,9 +367,9 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory /* ------------------------------------------------------------ */ /** - * Get the minimum reponse size. + * Get the minimum response size. * - * @return minimum reponse size + * @return minimum response size */ public int getMinGzipSize() { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java new file mode 100644 index 00000000000..0b3ac9252c2 --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java @@ -0,0 +1,42 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.server.handler.gzip; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import java.util.Arrays; + +import org.junit.Test; + +public class GzipHandlerTest +{ + @Test + public void testAddGetPaths() + { + GzipHandler gzip = new GzipHandler(); + gzip.addIncludedPaths("/foo"); + gzip.addIncludedPaths("^/bar.*$"); + + String[] includedPaths = gzip.getIncludedPaths(); + assertThat("Included Paths.size", includedPaths.length, is(2)); + assertThat("Included Paths", Arrays.asList(includedPaths), contains("/foo","^/bar.*$")); + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java index fd20040cab8..03cf0553a15 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java @@ -20,7 +20,7 @@ package org.eclipse.jetty.util; import java.util.HashSet; import java.util.Set; -import java.util.function.BiFunction; +import java.util.function.Predicate; /** Utility class to maintain a set of inclusions and exclusions. @@ -36,35 +36,77 @@ public class IncludeExclude { private final Set _includes; private final Set _excludes; - private final BiFunction,ITEM, Boolean> _matcher; - + private final Predicate _includePredicate; + private final Predicate _excludePredicate; + + private static class SetContainsPredicate implements Predicate + { + private final Set set; + + public SetContainsPredicate(Set set) + { + this.set = set; + } + + @Override + public boolean test(ITEM item) + { + return set.contains(item); + } + } + /** * Default constructor over {@link HashSet} */ public IncludeExclude() { - this(HashSet.class,null); + this(HashSet.class); } /** * Construct an IncludeExclude * @param setClass The type of {@link Set} to using internally - * @param matcher A function to test if a passed ITEM is matched by the passed SET, or null to use {@link Set#contains(Object)} + * @param predicate A predicate function to test if a passed ITEM is matched by the passed SET} */ - public > IncludeExclude(Class setClass, BiFunction matcher) + public > IncludeExclude(Class setClass) { try { _includes = setClass.newInstance(); _excludes = setClass.newInstance(); - _matcher = (BiFunction,ITEM, Boolean>)matcher; + if(_includes instanceof Predicate) { + _includePredicate = (Predicate)_includes; + } else { + _includePredicate = new SetContainsPredicate<>(_includes); + } + if(_excludes instanceof Predicate) { + _excludePredicate = (Predicate)_excludes; + } else { + _excludePredicate = new SetContainsPredicate<>(_excludes); + } } catch (InstantiationException | IllegalAccessException e) { throw new RuntimeException(e); } } - + + /** + * Construct an IncludeExclude + * + * @param includeSet the Set of items that represent the included space + * @param includePredicate the Predicate for included item testing (null for simple {@link Set#contains(Object)} test) + * @param excludeSet the Set of items that represent the excluded space + * @param excludePredicate the Predicate for excluded item testing (null for simple {@link Set#contains(Object)} test) + */ + public > IncludeExclude(Set includeSet, Predicate includePredicate, Set excludeSet, Predicate excludePredicate) + { + _includes = includeSet; + _includePredicate = includePredicate; + _excludes = excludeSet; + _excludePredicate = excludePredicate; + } + public void include(ITEM element) { _includes.add(element); @@ -89,17 +131,11 @@ public class IncludeExclude public boolean matches(ITEM e) { - if (_matcher==null) - { - if (_includes.size()>0 && !_includes.contains(e)) - return false; - return !_excludes.contains(e); - } - if (_includes.size()>0 && !_matcher.apply(_includes,e)) + if (!_includes.isEmpty() && !_includePredicate.test(e)) return false; - return !_matcher.apply(_excludes,e); + return !_excludePredicate.test(e); } - + public int size() { return _includes.size()+_excludes.size(); @@ -124,6 +160,6 @@ public class IncludeExclude @Override public String toString() { - return String.format("%s@%x{i=%s,e=%s,m=%s}",this.getClass().getSimpleName(),hashCode(),_includes,_excludes,_matcher); + return String.format("%s@%x{i=%s,ip=%s,e=%s,ep=%s}",this.getClass().getSimpleName(),hashCode(),_includes,_includePredicate,_excludes,_excludePredicate); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/RegexSet.java b/jetty-util/src/main/java/org/eclipse/jetty/util/RegexSet.java index ff72b039a43..c8db063fd77 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/RegexSet.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/RegexSet.java @@ -24,6 +24,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Predicate; import java.util.regex.Pattern; @@ -32,9 +33,8 @@ import java.util.regex.Pattern; *

* Provides the efficient {@link #matches(String)} method to check for a match against all the combined Regex's */ -public class RegexSet extends AbstractSet +public class RegexSet extends AbstractSet implements Predicate { - public static final BiFunction MATCHER=(rs,p)->{return rs.matches(p);}; private final Set _patterns=new HashSet(); private final Set _unmodifiable=Collections.unmodifiableSet(_patterns); private Pattern _pattern; @@ -98,6 +98,12 @@ public class RegexSet extends AbstractSet builder.append(")$"); _pattern = Pattern.compile(builder.toString()); } + + @Override + public boolean test(String s) + { + return _pattern!=null && _pattern.matcher(s).matches(); + } public boolean matches(String s) { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/IncludeExcludeTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/IncludeExcludeTest.java index 8da30fdb38a..4b7dde734ec 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/IncludeExcludeTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/IncludeExcludeTest.java @@ -20,7 +20,9 @@ package org.eclipse.jetty.util; import org.junit.Test; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; public class IncludeExcludeTest { @@ -29,8 +31,8 @@ public class IncludeExcludeTest { IncludeExclude ie = new IncludeExclude<>(); - assertEquals(0,ie.size()); - assertEquals(true,ie.matches("foo")); + assertThat("Empty IncludeExclude", ie.size(), is(0)); + assertThat("Matches 'foo'",ie.matches("foo"),is(true)); } @Test @@ -40,7 +42,7 @@ public class IncludeExcludeTest ie.include("foo"); ie.include("bar"); - assertEquals(2,ie.size()); + assertThat("IncludeExclude.size", ie.size(), is(2)); assertEquals(false,ie.matches("")); assertEquals(true,ie.matches("foo")); assertEquals(true,ie.matches("bar")); @@ -86,7 +88,7 @@ public class IncludeExcludeTest @Test public void testEmptyRegex() { - IncludeExclude ie = new IncludeExclude<>(RegexSet.class,RegexSet.MATCHER); + IncludeExclude ie = new IncludeExclude<>(RegexSet.class); assertEquals(0,ie.size()); assertEquals(true,ie.matches("foo")); @@ -95,7 +97,7 @@ public class IncludeExcludeTest @Test public void testIncludeRegex() { - IncludeExclude ie = new IncludeExclude<>(RegexSet.class,RegexSet.MATCHER); + IncludeExclude ie = new IncludeExclude<>(RegexSet.class); ie.include("f.."); ie.include("b((ar)|(oo))"); @@ -112,7 +114,7 @@ public class IncludeExcludeTest @Test public void testExcludeRegex() { - IncludeExclude ie = new IncludeExclude<>(RegexSet.class,RegexSet.MATCHER); + IncludeExclude ie = new IncludeExclude<>(RegexSet.class); ie.exclude("f.."); ie.exclude("b((ar)|(oo))"); @@ -130,7 +132,7 @@ public class IncludeExcludeTest @Test public void testIncludeExcludeRegex() { - IncludeExclude ie = new IncludeExclude<>(RegexSet.class,RegexSet.MATCHER); + IncludeExclude ie = new IncludeExclude<>(RegexSet.class); ie.include(".*[aeiou].*"); ie.include("[AEIOU].*"); ie.exclude("f.."); @@ -146,8 +148,5 @@ public class IncludeExcludeTest assertEquals(true,ie.matches("foobar")); assertEquals(true,ie.matches("Ant")); - } - - }