Fix #6305 Optimise isProtectedTarget (#6306)

* Fix #6305 Optimise isProtectedTarget Fix #6305 Optimise isProtectedTarget by using case insensitive Index. Signed-off-by: Greg Wilkins <gregw@webtide.com> * Fix #6305 Optimise isProtectedTarget updates from review Signed-off-by: Greg Wilkins <gregw@webtide.com> * Fix #6305 Optimise isProtectedTarget updates from review Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · May 28, 2021 · 0d71185 · 0d71185
1 parent 9e03775
commit 0d71185
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 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
@@ -73,6 +73,7 @@
 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 enum ContextStatus
         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 enum ContextStatus
     private final List<ServletRequestAttributeListener> _servletRequestAttributeListeners = new CopyOnWriteArrayList<>();
     private final List<ContextScopeListener> _contextListeners = new CopyOnWriteArrayList<>();
     private final Set<EventListener> _durableListeners = new HashSet<>();
-    private String[] _protectedTargets;
+    private Index<ProtectedTargetType> _protectedTargets = Index.empty(false);
     private final CopyOnWriteArrayList<AliasCheck> _aliasChecks = new CopyOnWriteArrayList<>();
 
     public enum Availability
@@ -1474,52 +1485,50 @@ public void handle(Runnable runnable)
      */
     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;
     }
 
     /**
      * @param targets Array of URL prefix. Each prefix is in the form /path and will match either /path exactly or /path/anything
      */
     public void setProtectedTargets(String[] targets)
     {
-        if (targets == null)
+        Index.Builder<ProtectedTargetType> builder = new Index.Builder<>();
+        if (targets != null)
         {
-            _protectedTargets = null;
-            return;
+            for (String t : targets)
+            {
+                if (!t.startsWith("/"))
+                    throw new IllegalArgumentException("Bad protected target: " + t);
+
+                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 = Arrays.copyOf(targets, targets.length);
+        _protectedTargets = builder.caseSensitive(false).build();
     }
 
     public String[] getProtectedTargets()
     {
         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
@@ -646,9 +646,20 @@ public void testProtected() throws Exception
         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
@@ -73,7 +73,8 @@ public interface Index<V>
     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 @@ static <V> Mutable<V> buildCaseSensitiveMutableVisibleAsciiAlphabet(int maxCapac
         return new ArrayTrie<>(true, maxCapacity);
     }
 
+    static <V> Index<V> empty(boolean caseSensitive)
+    {
+        return EmptyTrie.instance(caseSensitive);
+    }
+
     /**
      * Builder of {@link Index} instances.
      * @param <V> the entry type