From 3085e710fa0b6a6eabeb715d7a2cb1fd6a637e98 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 May 2022 10:31:40 -0500 Subject: [PATCH] Issue #7891 - Improving MatchedResource Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/AbstractPathSpec.java | 9 +- .../jetty/http/pathmap/MatchedResource.java | 37 +++- .../jetty/http/pathmap/PathMappings.java | 204 ++++++++++++------ .../eclipse/jetty/http/pathmap/PathSpec.java | 12 ++ .../jetty/http/pathmap/PathSpecSet.java | 4 +- .../jetty/http/pathmap/PathMappingsTest.java | 22 +- .../server/NativeWebSocketConfiguration.java | 15 +- .../server/WebSocketUpgradeFilter.java | 4 +- .../WebSocketUpgradeHandlerWrapper.java | 4 +- 9 files changed, 219 insertions(+), 92 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java index 82eadddc0b13..06f74f1c4474 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java @@ -36,7 +36,12 @@ public int compareTo(PathSpec other) return diff; // Path Spec Name (alphabetical) - return getDeclaration().compareTo(other.getDeclaration()); + diff = getDeclaration().compareTo(other.getDeclaration()); + if (diff != 0) + return diff; + + // Path Implementation + return getClass().getName().compareTo(other.getClass().getName()); } @Override @@ -55,7 +60,7 @@ public final boolean equals(Object obj) @Override public final int hashCode() { - return Objects.hash(getDeclaration()); + return Objects.hash(this.getClass().getSimpleName() + ":" + getDeclaration()); } @Override diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java index a2811a0ec0c9..7f0bbff7b825 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java @@ -18,34 +18,53 @@ package org.eclipse.jetty.http.pathmap; +import java.util.Map; + public class MatchedResource { - private final MappedResource mappedResource; + private final E resource; + private final PathSpec pathSpec; private final MatchedPath matchedPath; - public MatchedResource(MappedResource resource, MatchedPath matchedPath) + public MatchedResource(E resource, PathSpec pathSpec, MatchedPath matchedPath) { - this.mappedResource = resource; + this.resource = resource; + this.pathSpec = pathSpec; this.matchedPath = matchedPath; } - public MappedResource getMappedResource() + public static MatchedResource of(Map.Entry mapping, MatchedPath matchedPath) { - return this.mappedResource; + return new MatchedResource<>(mapping.getValue(), mapping.getKey(), matchedPath); } public PathSpec getPathSpec() { - return mappedResource.getPathSpec(); + return this.pathSpec; } public E getResource() { - return mappedResource.getResource(); + return this.resource; + } + + /** + * Return the portion of the path that matches a path spec. + * + * @return the path name portion of the match. + */ + public String getPathMatch() + { + return matchedPath.getPathMatch(); } - public MatchedPath getMatchedPath() + /** + * Return the portion of the path that is after the path spec. + * + * @return the path info portion of the match, or null if there is no portion after the {@link #getPathMatch()} + */ + public String getPathInfo() { - return matchedPath; + return matchedPath.getPathInfo(); } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index 48c97153fade..e65b1b73330b 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -23,7 +23,6 @@ import java.util.Comparator; import java.util.Iterator; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.TreeSet; import java.util.function.Predicate; @@ -49,8 +48,11 @@ public class PathMappings implements Iterable>, Dumpable private static final Logger LOG = Log.getLogger(PathMappings.class); private final Set> _mappings = new TreeSet<>(Comparator.comparing(MappedResource::getPathSpec)); + private boolean _optimizedExact = true; private Trie> _exactMap = new ArrayTernaryTrie<>(false); + private boolean _optimizedPrefix = true; private Trie> _prefixMap = new ArrayTernaryTrie<>(false); + private boolean _optimizedSuffix = true; private Trie> _suffixMap = new ArrayTernaryTrie<>(false); @Override @@ -88,6 +90,26 @@ public void removeIf(Predicate> predicate) _mappings.removeIf(predicate); } + /** + * Return a list of MatchedResource matches for the specified path. + * + * @param path the path to return matches on + * @return the list of mapped resource the path matches on + */ + public List> getMatchedList(String path) + { + List> ret = new ArrayList<>(); + for (MappedResource mr : _mappings) + { + MatchedPath matchedPath = mr.getPathSpec().matched(path); + if (matchedPath != null) + { + ret.add(new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath)); + } + } + return ret; + } + /** * Return a list of MappedResource matches for the specified path. * @@ -108,11 +130,11 @@ public List> getMatches(String path) ret.add(mr); break; case DEFAULT: - if (isRootPath || mr.getPathSpec().matches(path)) + if (isRootPath || mr.getPathSpec().matched(path) != null) ret.add(mr); break; default: - if (mr.getPathSpec().matches(path)) + if (mr.getPathSpec().matched(path) != null) ret.add(mr); break; } @@ -122,7 +144,7 @@ public List> getMatches(String path) public MatchedResource getMatched(String path) { - MatchedPath matchedPath = null; + MatchedPath matchedPath; PathSpecGroup lastGroup = null; // Search all the mappings @@ -136,53 +158,80 @@ public MatchedResource getMatched(String path) { case EXACT: { - int i = path.length(); - final Trie> exact_map = _exactMap; - while (i >= 0) + if (_optimizedExact) { - MappedResource candidate = exact_map.getBest(path, 0, i); - if (candidate == null) - break; - - matchedPath = candidate.getPathSpec().matched(path); + int i = path.length(); + final Trie> exact_map = _exactMap; + while (i >= 0) + { + MappedResource candidate = exact_map.getBest(path, 0, i); + if (candidate == null) + break; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); + i = candidate.getPathSpec().getDeclaration().length() - 1; + } + } + else + { + matchedPath = mr.getPathSpec().matched(path); if (matchedPath != null) - return new MatchedResource<>(candidate, matchedPath); - i = candidate.getPathSpec().getPrefix().length() - 1; + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); } break; } case PREFIX_GLOB: { - int i = path.length(); - final Trie> prefix_map = _prefixMap; - while (i >= 0) + if (_optimizedPrefix) { - MappedResource candidate = prefix_map.getBest(path, 0, i); - if (candidate == null) - break; - - matchedPath = candidate.getPathSpec().matched(path); + int i = path.length(); + final Trie> prefix_map = _prefixMap; + while (i >= 0) + { + MappedResource candidate = prefix_map.getBest(path, 0, i); + if (candidate == null) + break; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); + i = candidate.getPathSpec().getPrefix().length() - 1; + } + } + else + { + matchedPath = mr.getPathSpec().matched(path); if (matchedPath != null) - return new MatchedResource<>(candidate, matchedPath); - i = candidate.getPathSpec().getPrefix().length() - 1; + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); } break; } case SUFFIX_GLOB: { - int i = 0; - final Trie> suffix_map = _suffixMap; - while ((i = path.indexOf('.', i + 1)) > 0) + if (_optimizedSuffix) { - MappedResource candidate = suffix_map.get(path, i + 1, path.length() - i - 1); - if (candidate == null) - break; - - matchedPath = candidate.getPathSpec().matched(path); + int i = 0; + final Trie> suffix_map = _suffixMap; + while ((i = path.indexOf('.', i + 1)) > 0) + { + MappedResource candidate = suffix_map.get(path, i + 1, path.length() - i - 1); + if (candidate == null) + break; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); + } + } + else + { + matchedPath = mr.getPathSpec().matched(path); if (matchedPath != null) - return new MatchedResource<>(candidate, matchedPath); + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); } break; } @@ -193,7 +242,7 @@ public MatchedResource getMatched(String path) matchedPath = mr.getPathSpec().matched(path); if (matchedPath != null) - return new MatchedResource<>(mr, matchedPath); + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); lastGroup = group; } @@ -207,7 +256,7 @@ public MatchedResource getMatched(String path) @Deprecated public MappedResource getMatch(String path) { - return getMatched(path).getMappedResource(); + throw new UnsupportedOperationException("Use .getMatched(String) instead"); } @Override @@ -216,32 +265,27 @@ public Iterator> iterator() return _mappings.iterator(); } + /** + * @deprecated use {@link PathSpec#from(String)} instead + */ + @Deprecated public static PathSpec asPathSpec(String pathSpecString) { - if (pathSpecString == null) - throw new RuntimeException("Path Spec String must start with '^', '/', or '*.': got [" + pathSpecString + "]"); - - if (pathSpecString.length() == 0) - return new ServletPathSpec(""); - - return pathSpecString.charAt(0) == '^' ? new RegexPathSpec(pathSpecString) : new ServletPathSpec(pathSpecString); + return PathSpec.from(pathSpecString); } public E get(PathSpec spec) { - Optional optionalResource = _mappings.stream() + return _mappings.stream() .filter(mappedResource -> mappedResource.getPathSpec().equals(spec)) - .map(mappedResource -> mappedResource.getResource()) - .findFirst(); - if (!optionalResource.isPresent()) - return null; - - return optionalResource.get(); + .map(MappedResource::getResource) + .findFirst() + .orElse(null); } public boolean put(String pathSpecString, E resource) { - return put(asPathSpec(pathSpecString), resource); + return put(PathSpec.from(pathSpecString), resource); } public boolean put(PathSpec pathSpec, E resource) @@ -250,24 +294,48 @@ public boolean put(PathSpec pathSpec, E resource) switch (pathSpec.getGroup()) { case EXACT: - String exact = pathSpec.getPrefix(); - while (exact != null && !_exactMap.put(exact, entry)) + if (pathSpec instanceof ServletPathSpec) + { + String exact = pathSpec.getDeclaration(); + while (exact != null && !_exactMap.put(exact, entry)) + { + _exactMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_exactMap, 1.5); + } + } + else { - _exactMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_exactMap, 1.5); + // This is not a Servlet mapping, turn off optimization on Exact + _optimizedExact = false; } break; case PREFIX_GLOB: - String prefix = pathSpec.getPrefix(); - while (prefix != null && !_prefixMap.put(prefix, entry)) + if (pathSpec instanceof ServletPathSpec) + { + String prefix = pathSpec.getPrefix(); + while (prefix != null && !_prefixMap.put(prefix, entry)) + { + _prefixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + } + } + else { - _prefixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + // This is not a Servlet mapping, turn off optimization on Prefix + _optimizedPrefix = false; } break; case SUFFIX_GLOB: - String suffix = pathSpec.getSuffix(); - while (suffix != null && !_suffixMap.put(suffix, entry)) + if (pathSpec instanceof ServletPathSpec) { - _suffixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + String suffix = pathSpec.getSuffix(); + while (suffix != null && !_suffixMap.put(suffix, entry)) + { + _suffixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + } + } + else + { + // This is not a Servlet mapping, turn off optimization on Suffix + _optimizedSuffix = false; } break; default: @@ -282,21 +350,31 @@ public boolean put(PathSpec pathSpec, E resource) @SuppressWarnings("incomplete-switch") public boolean remove(PathSpec pathSpec) { - String prefix = pathSpec.getPrefix(); - String suffix = pathSpec.getSuffix(); switch (pathSpec.getGroup()) { case EXACT: - if (prefix != null) - _exactMap.remove(prefix); + String exact = pathSpec.getDeclaration(); + if (exact != null) + { + _exactMap.remove(exact); + // TODO: recalculate _optimizeExact + } break; case PREFIX_GLOB: + String prefix = pathSpec.getPrefix(); if (prefix != null) + { _prefixMap.remove(prefix); + // TODO: recalculate _optimizePrefix + } break; case SUFFIX_GLOB: + String suffix = pathSpec.getSuffix(); if (suffix != null) + { _suffixMap.remove(suffix); + // TODO: recalculate _optimizeSuffix + } break; } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java index bb3d5057c985..0d8afcd4bc79 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java @@ -25,6 +25,17 @@ */ public interface PathSpec extends Comparable { + static PathSpec from(String pathSpecString) + { + if (pathSpecString == null) + throw new RuntimeException("Path Spec String must start with '^', '/', or '*.': got [" + pathSpecString + "]"); + + if (pathSpecString.length() == 0) + return new ServletPathSpec(""); + + return pathSpecString.charAt(0) == '^' ? new RegexPathSpec(pathSpecString) : new ServletPathSpec(pathSpecString); + } + /** * The length of the spec. * @@ -101,6 +112,7 @@ public interface PathSpec extends Comparable /** * Get the complete matched details of the provided path. + * * @param path the path to test * @return the matched details, if a match was possible, or null if not able to be matched. */ 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 index 5d09dc19f05e..cf601207d62e 100644 --- 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 @@ -55,13 +55,13 @@ private PathSpec asPathSpec(Object o) return (PathSpec)o; } - return PathMappings.asPathSpec(Objects.toString(o)); + return PathSpec.from(Objects.toString(o)); } @Override public boolean add(String s) { - return specs.put(PathMappings.asPathSpec(s), Boolean.TRUE); + return specs.put(PathSpec.from(s), Boolean.TRUE); } @Override diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java index 678df783b17c..ef9d25a143f9 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java @@ -267,8 +267,8 @@ public void testPutRejectsDuplicates() assertThat(p.put(new UriTemplatePathSpec("/a/{var2}/c"), "resourceAA"), is(false)); assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceB"), is(true)); assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceBB"), is(false)); - assertThat(p.put(new ServletPathSpec("/a/b/c"), "resourceBB"), is(false)); - assertThat(p.put(new RegexPathSpec("/a/b/c"), "resourceBB"), is(false)); + assertThat(p.put(new ServletPathSpec("/a/b/c"), "resourceBB"), is(true)); + assertThat(p.put(new RegexPathSpec("/a/b/c"), "resourceBB"), is(true)); assertThat(p.put(new ServletPathSpec("/*"), "resourceC"), is(true)); assertThat(p.put(new RegexPathSpec("/(.*)"), "resourceCC"), is(true)); @@ -418,14 +418,14 @@ public void testRemoveServletPathSpec() @Test public void testAsPathSpec() { - assertThat(PathMappings.asPathSpec(""), instanceOf(ServletPathSpec.class)); - assertThat(PathMappings.asPathSpec("/"), instanceOf(ServletPathSpec.class)); - assertThat(PathMappings.asPathSpec("/*"), instanceOf(ServletPathSpec.class)); - assertThat(PathMappings.asPathSpec("/foo/*"), instanceOf(ServletPathSpec.class)); - assertThat(PathMappings.asPathSpec("*.jsp"), instanceOf(ServletPathSpec.class)); - - assertThat(PathMappings.asPathSpec("^$"), instanceOf(RegexPathSpec.class)); - assertThat(PathMappings.asPathSpec("^.*"), instanceOf(RegexPathSpec.class)); - assertThat(PathMappings.asPathSpec("^/"), instanceOf(RegexPathSpec.class)); + assertThat(PathSpec.from(""), instanceOf(ServletPathSpec.class)); + assertThat(PathSpec.from("/"), instanceOf(ServletPathSpec.class)); + assertThat(PathSpec.from("/*"), instanceOf(ServletPathSpec.class)); + assertThat(PathSpec.from("/foo/*"), instanceOf(ServletPathSpec.class)); + assertThat(PathSpec.from("*.jsp"), instanceOf(ServletPathSpec.class)); + + assertThat(PathSpec.from("^$"), instanceOf(RegexPathSpec.class)); + assertThat(PathSpec.from("^.*"), instanceOf(RegexPathSpec.class)); + assertThat(PathSpec.from("^/"), instanceOf(RegexPathSpec.class)); } } diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java index 039711e63a6a..80892cf3b147 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java @@ -75,18 +75,31 @@ public WebSocketServerFactory getFactory() return this.factory; } + /** + * Get the matching {@link MatchedResource} for the provided target. + * + * @param target the target path + * @return the matching resource, or null if no match. + */ + public MatchedResource getMatched(String target) + { + return this.mappings.getMatched(target); + } + /** * Get the matching {@link MappedResource} for the provided target. * * @param target the target path * @return the matching resource, or null if no match. + * @deprecated use {@link #getMatched(String)} instead. */ + @Deprecated public MappedResource getMatch(String target) { MatchedResource matched = this.mappings.getMatched(target); if (matched == null) return null; - return matched.getMappedResource(); + return new MappedResource<>(matched.getPathSpec(), matched.getResource()); } /** diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java index 5ba28f44a4f1..166a376787c4 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java @@ -31,7 +31,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; @@ -302,7 +302,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha target = target + httpreq.getPathInfo(); } - MappedResource resource = configuration.getMatch(target); + MatchedResource resource = configuration.getMatched(target); if (resource == null) { // no match. diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeHandlerWrapper.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeHandlerWrapper.java index 9ec8fa21bdcd..9c7875d8f1ea 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeHandlerWrapper.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeHandlerWrapper.java @@ -23,7 +23,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; @@ -89,7 +89,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques { if (configuration.getFactory().isUpgradeRequest(request, response)) { - MappedResource resource = configuration.getMatch(target); + MatchedResource resource = configuration.getMatched(target); if (resource == null) { // no match.