From 0d71185e10344c286387bab24d4ec6e9fba1eacb Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 28 May 2021 18:01:25 +1000 Subject: [PATCH] Fix #6305 Optimise isProtectedTarget (#6306) * Fix #6305 Optimise isProtectedTarget Fix #6305 Optimise isProtectedTarget by using case insensitive Index. Signed-off-by: Greg Wilkins * Fix #6305 Optimise isProtectedTarget updates from review Signed-off-by: Greg Wilkins * Fix #6305 Optimise isProtectedTarget updates from review Signed-off-by: Greg Wilkins --- .../jetty/server/handler/ContextHandler.java | 61 +++++++++++-------- .../server/handler/ContextHandlerTest.java | 13 +++- .../java/org/eclipse/jetty/util/Index.java | 8 ++- 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index ef4eb96ab6a..b011b5a445a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -73,6 +73,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.AttributesMap; +import org.eclipse.jetty.util.Index; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.StringUtil; @@ -179,6 +180,16 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu DESTROYED } + /** + * The type of protected target match + * @see #_protectedTargets + */ + private enum ProtectedTargetType + { + EXACT, + PREFIX + } + protected ContextStatus _contextStatus = ContextStatus.NOTSET; protected Context _scontext; private final AttributesMap _attributes; @@ -214,7 +225,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu private final List _servletRequestAttributeListeners = new CopyOnWriteArrayList<>(); private final List _contextListeners = new CopyOnWriteArrayList<>(); private final Set _durableListeners = new HashSet<>(); - private String[] _protectedTargets; + private Index _protectedTargets = Index.empty(false); private final CopyOnWriteArrayList _aliasChecks = new CopyOnWriteArrayList<>(); public enum Availability @@ -1474,30 +1485,17 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public boolean isProtectedTarget(String target) { - if (target == null || _protectedTargets == null) + if (target == null || _protectedTargets.isEmpty()) return false; - while (target.startsWith("//")) - { + if (target.startsWith("//")) + // ignore empty segments which may be discard by file system target = URIUtil.compactPath(target); - } - for (int i = 0; i < _protectedTargets.length; i++) - { - String t = _protectedTargets[i]; - if (StringUtil.startsWithIgnoreCase(target, t)) - { - if (target.length() == t.length()) - return true; + ProtectedTargetType type = _protectedTargets.getBest(target); - // Check that the target prefix really is a path segment, thus - // it can end with /, a query, a target or a parameter - char c = target.charAt(t.length()); - if (c == '/' || c == '?' || c == '#' || c == ';') - return true; - } - } - return false; + return type == ProtectedTargetType.PREFIX || + type == ProtectedTargetType.EXACT && _protectedTargets.get(target) == ProtectedTargetType.EXACT; } /** @@ -1505,13 +1503,22 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setProtectedTargets(String[] targets) { - if (targets == null) + Index.Builder builder = new Index.Builder<>(); + if (targets != null) { - _protectedTargets = null; - return; - } + for (String t : targets) + { + if (!t.startsWith("/")) + throw new IllegalArgumentException("Bad protected target: " + t); - _protectedTargets = Arrays.copyOf(targets, targets.length); + builder.with(t, ProtectedTargetType.EXACT); + builder.with(t + "/", ProtectedTargetType.PREFIX); + builder.with(t + "?", ProtectedTargetType.PREFIX); + builder.with(t + "#", ProtectedTargetType.PREFIX); + builder.with(t + ";", ProtectedTargetType.PREFIX); + } + } + _protectedTargets = builder.caseSensitive(false).build(); } public String[] getProtectedTargets() @@ -1519,7 +1526,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu if (_protectedTargets == null) return null; - return Arrays.copyOf(_protectedTargets, _protectedTargets.length); + return _protectedTargets.keySet().stream() + .filter(s -> _protectedTargets.get(s) == ProtectedTargetType.EXACT) + .toArray(String[]::new); } @Override diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index 0de5ac136fc..31b4f575bfa 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -646,9 +646,20 @@ public class ContextHandlerTest String[] protectedTargets = {"/foo-inf", "/bar-inf"}; handler.setProtectedTargets(protectedTargets); + assertTrue(handler.isProtectedTarget("/foo-inf")); + assertTrue(handler.isProtectedTarget("/Foo-Inf")); + assertTrue(handler.isProtectedTarget("/FOO-INF")); + assertTrue(handler.isProtectedTarget("/foo-inf/")); + assertTrue(handler.isProtectedTarget("/FOO-inf?")); + assertTrue(handler.isProtectedTarget("/FOO-INF;")); + assertTrue(handler.isProtectedTarget("/foo-INF#")); + assertTrue(handler.isProtectedTarget("//foo-inf")); + assertTrue(handler.isProtectedTarget("//foo-inf//some//path")); + assertTrue(handler.isProtectedTarget("///foo-inf")); assertTrue(handler.isProtectedTarget("/foo-inf/x/y/z")); - assertFalse(handler.isProtectedTarget("/foo/x/y/z")); assertTrue(handler.isProtectedTarget("/foo-inf?x=y&z=1")); + + assertFalse(handler.isProtectedTarget("/foo/x/y/z")); assertFalse(handler.isProtectedTarget("/foo-inf-bar")); protectedTargets = new String[4]; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java index 244bcae5ddd..1b79b0fa21c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java @@ -73,7 +73,8 @@ public interface Index V getBest(String s, int offset, int len); /** - * Get the best match from key in a String. + * Get the best match from key in a String, which may be + * a prefix match or an exact match. * * @param s The string * @return The value or null if not found @@ -267,6 +268,11 @@ public interface Index return new ArrayTrie<>(true, maxCapacity); } + static Index empty(boolean caseSensitive) + { + return EmptyTrie.instance(caseSensitive); + } + /** * Builder of {@link Index} instances. * @param the entry type