From 23c7b1846d91ff7bc664fbf1ea9a391b6a62da49 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 28 Apr 2022 16:22:51 -0500 Subject: [PATCH 01/17] More work on MatchedResult Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/MatchedPath.java | 41 +++++++ .../eclipse/jetty/http/pathmap/PathSpec.java | 13 +++ .../jetty/http/pathmap/RegexPathSpec.java | 105 ++++++++++++++++-- .../jetty/http/pathmap/ServletPathSpec.java | 84 ++++++++++++++ .../http/pathmap/UriTemplatePathSpec.java | 93 ++++++++++++++-- .../http/pathmap/ServletPathSpecTest.java | 103 ++++++++++++----- .../test/resources/jetty-logging.properties | 1 + 7 files changed, 396 insertions(+), 44 deletions(-) create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java new file mode 100644 index 000000000000..87422c042bbb --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java @@ -0,0 +1,41 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// 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; + +public interface MatchedPath +{ + /** + * @return the raw input path as provided. + */ + String getPath(); + + /** + * Return the portion of the path that matches a path spec. + * + * @return the path name portion of the match. + */ + String getPathMatch(); + + /** + * 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()} + */ + String getPathInfo(); +} 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 26e7d83ef803..bb3d5057c985 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 @@ -53,7 +53,9 @@ public interface PathSpec extends Comparable * * @param path the path to match against * @return the path info portion of the string + * @deprecated use {@link #matched(String)} instead */ + @Deprecated String getPathInfo(String path); /** @@ -61,7 +63,9 @@ public interface PathSpec extends Comparable * * @param path the path to match against * @return the match, or null if no match at all + * @deprecated use {@link #matched(String)} instead */ + @Deprecated String getPathMatch(String path); /** @@ -90,6 +94,15 @@ public interface PathSpec extends Comparable * * @param path the path to test * @return true if the path matches this path spec, false otherwise + * @deprecated use {@link #matched(String)} instead */ + @Deprecated boolean matches(String path); + + /** + * 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. + */ + MatchedPath matched(String path); } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index b893096e6bfb..416f0deb0465 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -21,8 +21,13 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + public class RegexPathSpec extends AbstractPathSpec { + private static final Logger LOG = Log.getLogger(UriTemplatePathSpec.class); + private final String _declaration; private final PathSpecGroup _group; private final int _pathDepth; @@ -87,11 +92,27 @@ else if (Pattern.matches("^g+l+$", sig)) _pathDepth = pathDepth; _specLength = specLength; _pattern = pattern; + + if (LOG.isDebugEnabled()) + { + LOG.debug("Creating RegexPathSpec[{}] (signature: [{}], group: {})", + _declaration, sig, _group); + } } protected Matcher getMatcher(String path) { - return _pattern.matcher(path); + int idx = path.indexOf('?'); + if (idx >= 0) + { + // match only non-query part + return _pattern.matcher(path.substring(0, idx)); + } + else + { + // match entire path + return _pattern.matcher(path); + } } @Override @@ -181,16 +202,84 @@ public Pattern getPattern() @Override public boolean matches(final String path) { - int idx = path.indexOf('?'); - if (idx >= 0) + return getMatcher(path).matches(); + } + + @Override + public MatchedPath matched(String path) + { + Matcher matcher = getMatcher(path); + if (matcher.matches()) { - // match only non-query part - return getMatcher(path.substring(0, idx)).matches(); + return new RegexMatchedPath(this, path, matcher); } - else + return null; + } + + public class RegexMatchedPath implements MatchedPath + { + private final RegexPathSpec pathSpec; + private final String path; + private final Matcher matcher; + + public RegexMatchedPath(RegexPathSpec regexPathSpec, String path, Matcher matcher) { - // match entire path - return getMatcher(path).matches(); + this.pathSpec = regexPathSpec; + this.path = path; + this.matcher = matcher; + } + + @Override + public String getPath() + { + return this.path; + } + + @Override + public String getPathMatch() + { + String p = matcher.group("name"); + if (p != null) + { + return p; + } + + if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1) + { + int idx = matcher.start(1); + if (idx > 0) + { + if (this.path.charAt(idx - 1) == '/') + idx--; + return this.path.substring(0, idx); + } + } + + // default is the full path + return this.path; + } + + @Override + public String getPathInfo() + { + String p = matcher.group("info"); + if (p != null) + { + return p; + } + + // Path Info only valid for PREFIX_GLOB + if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1) + { + String pathInfo = matcher.group(1); + if ("".equals(pathInfo)) + return "/"; + else + return pathInfo; + } + + // default is null + return null; } } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index 3e8c72106995..e3b23f4a86f3 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -128,6 +128,12 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) _specLength = specLength; _prefix = prefix; _suffix = suffix; + + if (LOG.isDebugEnabled()) + { + LOG.debug("Creating ServletPathSpec[{}] (group: {}, prefix: \"{}\", suffix: \"{}\")", + _declaration, _group, _prefix, _suffix); + } } private static void assertValidServletPathSpec(String servletPathSpec) @@ -194,6 +200,10 @@ public int getPathDepth() return _pathDepth; } + /** + * @deprecated use {@link #matched(String)}#{@link MatchedPath#getPathInfo()} instead. + */ + @Deprecated @Override public String getPathInfo(String path) { @@ -212,6 +222,10 @@ public String getPathInfo(String path) } } + /** + * @deprecated use {@link #matched(String)}#{@link MatchedPath#getPathMatch()} instead. + */ + @Deprecated @Override public String getPathMatch(String path) { @@ -270,6 +284,44 @@ private boolean isWildcardMatch(String path) return false; } + @Override + public MatchedPath matched(String path) + { + switch (_group) + { + case EXACT: + if (_declaration.equals(path)) + return new ServletMatchedPath(path, path, null); + break; + case PREFIX_GLOB: + if (isWildcardMatch(path)) + { + String pathMatch = path; + String pathInfo = null; + if (path.length() != (_specLength - 2)) + { + pathMatch = path.substring(0, _specLength - 2); + pathInfo = path.substring(_specLength - 2); + } + return new ServletMatchedPath(path, pathMatch, pathInfo); + } + break; + case SUFFIX_GLOB: + if (path.regionMatches((path.length() - _specLength) + 1, _declaration, 1, _specLength - 1)) + return new ServletMatchedPath(path, path, null); + break; + case ROOT: + // Only "/" matches + if ("/".equals(path)) + return new ServletMatchedPath(path, "", path); + break; + case DEFAULT: + // If we reached this point, then everything matches + return new ServletMatchedPath(path, path, null); + } + return null; + } + @Override public boolean matches(String path) { @@ -291,4 +343,36 @@ public boolean matches(String path) return false; } } + + public static class ServletMatchedPath implements MatchedPath + { + private final String path; + private final String servletName; + private final String pathInfo; + + public ServletMatchedPath(String inputPath, String servletName, String pathInfo) + { + this.path = inputPath; + this.servletName = servletName; + this.pathInfo = pathInfo; + } + + @Override + public String getPath() + { + return this.path; + } + + @Override + public String getPathMatch() + { + return this.servletName; + } + + @Override + public String getPathInfo() + { + return this.pathInfo; + } + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java index 4736d9b4a191..384603b3020d 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java @@ -202,6 +202,12 @@ else if (Pattern.matches("^v+e+", sig)) _pattern = pattern; _variables = variables; _logicalDeclaration = logicalSignature.toString(); + + if (LOG.isDebugEnabled()) + { + LOG.debug("Creating UriTemplatePathSpec[{}] (regex: \"{}\", signature: [{}], group: {}, variables: [{}])", + _declaration, regex, sig, _group, String.join(", ", _variables)); + } } /** @@ -301,7 +307,9 @@ public Map getPathParams(String path) Map ret = new HashMap<>(); int groupCount = matcher.groupCount(); for (int i = 1; i <= groupCount; i++) + { ret.put(_variables[i - 1], matcher.group(i)); + } return ret; } return null; @@ -309,7 +317,17 @@ public Map getPathParams(String path) protected Matcher getMatcher(String path) { - return _pattern.matcher(path); + int idx = path.indexOf('?'); + if (idx >= 0) + { + // match only non-query part + return _pattern.matcher(path.substring(0, idx)); + } + else + { + // match entire path + return _pattern.matcher(path); + } } @Override @@ -399,17 +417,18 @@ public Pattern getPattern() @Override public boolean matches(final String path) { - int idx = path.indexOf('?'); - if (idx >= 0) - { - // match only non-query part - return getMatcher(path.substring(0, idx)).matches(); - } - else + return getMatcher(path).matches(); + } + + @Override + public MatchedPath matched(String path) + { + Matcher matcher = getMatcher(path); + if (matcher.matches()) { - // match entire path - return getMatcher(path).matches(); + return new UriTemplatePathSpec.UriTemplateMatchedPath(this, path, matcher); } + return null; } public int getVariableCount() @@ -421,4 +440,58 @@ public String[] getVariables() { return _variables; } + + public static class UriTemplateMatchedPath implements MatchedPath + { + private final UriTemplatePathSpec pathSpec; + private final String path; + private final Matcher matcher; + + public UriTemplateMatchedPath(UriTemplatePathSpec uriTemplatePathSpec, String path, Matcher matcher) + { + this.pathSpec = uriTemplatePathSpec; + this.path = path; + this.matcher = matcher; + } + + @Override + public String getPath() + { + return this.path; + } + + @Override + public String getPathMatch() + { + // TODO: UriTemplatePathSpec has no concept of prefix/suffix, this should be simplified + // TODO: Treat all UriTemplatePathSpec matches as exact when it comes to pathMatch/pathInfo + if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1) + { + int idx = matcher.start(1); + if (idx > 0) + { + if (path.charAt(idx - 1) == '/') + idx--; + return path.substring(0, idx); + } + } + return path; + } + + @Override + public String getPathInfo() + { + // TODO: UriTemplatePathSpec has no concept of prefix/suffix, this should be simplified + // TODO: Treat all UriTemplatePathSpec matches as exact when it comes to pathMatch/pathInfo + if (pathSpec.getGroup() == PathSpecGroup.PREFIX_GLOB && matcher.groupCount() >= 1) + { + String pathInfo = matcher.group(1); + if ("".equals(pathInfo)) + return "/"; + else + return pathInfo; + } + return null; + } + } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java index e80fe94bd6b1..a5610fe7758d 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java @@ -24,6 +24,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -46,13 +47,13 @@ private void assertBadServletPathSpec(String pathSpec) private void assertMatches(ServletPathSpec spec, String path) { String msg = String.format("Spec(\"%s\").matches(\"%s\")", spec.getDeclaration(), path); - assertThat(msg, spec.matches(path), is(true)); + assertThat(msg, spec.matched(path), not(nullValue())); } private void assertNotMatches(ServletPathSpec spec, String path) { String msg = String.format("!Spec(\"%s\").matches(\"%s\")", spec.getDeclaration(), path); - assertThat(msg, spec.matches(path), is(false)); + assertThat(msg, spec.matched(path), is(nullValue())); } @Test @@ -112,16 +113,37 @@ public void testExactPathSpec() @Test public void testGetPathInfo() { - assertEquals(null, new ServletPathSpec("/Foo/bar").getPathInfo("/Foo/bar"), "pathInfo exact"); - assertEquals("/bar", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/bar"), "pathInfo prefix"); - assertEquals("/*", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/*"), "pathInfo prefix"); - assertEquals("/", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/"), "pathInfo prefix"); - assertEquals(null, new ServletPathSpec("/Foo/*").getPathInfo("/Foo"), "pathInfo prefix"); - assertEquals(null, new ServletPathSpec("*.ext").getPathInfo("/Foo/bar.ext"), "pathInfo suffix"); - assertEquals(null, new ServletPathSpec("/").getPathInfo("/Foo/bar.ext"), "pathInfo default"); - assertEquals("/", new ServletPathSpec("").getPathInfo("/"), "pathInfo root"); - assertEquals("", new ServletPathSpec("").getPathInfo(""), "pathInfo root"); - assertEquals("/xxx/zzz", new ServletPathSpec("/*").getPathInfo("/xxx/zzz"), "pathInfo default"); + // assertEquals(null, new ServletPathSpec("/Foo/bar").getPathInfo("/Foo/bar"), "pathInfo exact"); + ServletPathSpec spec = new ServletPathSpec("/Foo/bar"); + assertThat("PathInfo exact", spec.matched("/Foo/bar").getPathInfo(), is(nullValue())); + + // assertEquals("/bar", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/bar"), "pathInfo prefix"); + // assertEquals("/*", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/*"), "pathInfo prefix"); + // assertEquals("/", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/"), "pathInfo prefix"); + // assertEquals(null, new ServletPathSpec("/Foo/*").getPathInfo("/Foo"), "pathInfo prefix"); + spec = new ServletPathSpec("/Foo/*"); + assertThat("PathInfo prefix", spec.matched("/Foo/bar").getPathInfo(), is("/bar")); + assertThat("PathInfo prefix", spec.matched("/Foo/*").getPathInfo(), is("/*")); + assertThat("PathInfo prefix", spec.matched("/Foo/").getPathInfo(), is("/")); + assertThat("PathInfo prefix", spec.matched("/Foo").getPathInfo(), is(nullValue())); + + // assertEquals(null, new ServletPathSpec("*.ext").getPathInfo("/Foo/bar.ext"), "pathInfo suffix"); + spec = new ServletPathSpec("*.ext"); + assertThat("PathInfo suffix", spec.matched("/Foo/bar.ext").getPathInfo(), is(nullValue())); + + // assertEquals(null, new ServletPathSpec("/").getPathInfo("/Foo/bar.ext"), "pathInfo default"); + spec = new ServletPathSpec("/"); + assertThat("PathInfo default", spec.matched("/Foo/bar.ext").getPathInfo(), is(nullValue())); + + // assertEquals("/", new ServletPathSpec("").getPathInfo("/"), "pathInfo root"); + // assertEquals("", new ServletPathSpec("").getPathInfo(""), "pathInfo root"); + spec = new ServletPathSpec(""); + assertThat("PathInfo root", spec.matched("/").getPathInfo(), is("/")); + assertThat("PathInfo root", spec.matched(""), is(nullValue())); // does not match // TODO: verify with greg + + // assertEquals("/xxx/zzz", new ServletPathSpec("/*").getPathInfo("/xxx/zzz"), "pathInfo default"); + spec = new ServletPathSpec("/*"); + assertThat("PathInfo default", spec.matched("/xxx/zzz").getPathInfo(), is("/xxx/zzz")); } @Test @@ -143,15 +165,35 @@ public void testRootPathSpec() @Test public void testPathMatch() { - assertEquals("/Foo/bar", new ServletPathSpec("/Foo/bar").getPathMatch("/Foo/bar"), "pathMatch exact"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/bar"), "pathMatch prefix"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/"), "pathMatch prefix"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo"), "pathMatch prefix"); - assertEquals("/Foo/bar.ext", new ServletPathSpec("*.ext").getPathMatch("/Foo/bar.ext"), "pathMatch suffix"); - assertEquals("/Foo/bar.ext", new ServletPathSpec("/").getPathMatch("/Foo/bar.ext"), "pathMatch default"); - assertEquals("", new ServletPathSpec("").getPathMatch("/"), "pathInfo root"); - assertEquals("", new ServletPathSpec("").getPathMatch(""), "pathInfo root"); - assertEquals("", new ServletPathSpec("/*").getPathMatch("/xxx/zzz"), "pathMatch default"); + // assertEquals("/Foo/bar", new ServletPathSpec("/Foo/bar").getPathMatch("/Foo/bar"), "pathMatch exact"); + ServletPathSpec spec = new ServletPathSpec("/Foo/bar"); + assertThat("PathMatch exact", spec.matched("/Foo/bar").getPathMatch(), is("/Foo/bar")); + + // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/bar"), "pathMatch prefix"); + // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/"), "pathMatch prefix"); + // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo"), "pathMatch prefix"); + spec = new ServletPathSpec("/Foo/*"); + assertThat("PathMatch prefix", spec.matched("/Foo/bar").getPathMatch(), is("/Foo")); + assertThat("PathMatch prefix", spec.matched("/Foo/").getPathMatch(), is("/Foo")); + assertThat("PathMatch prefix", spec.matched("/Foo").getPathMatch(), is("/Foo")); + + // assertEquals("/Foo/bar.ext", new ServletPathSpec("*.ext").getPathMatch("/Foo/bar.ext"), "pathMatch suffix"); + spec = new ServletPathSpec("*.ext"); + assertThat("PathMatch suffix", spec.matched("/Foo/bar.ext").getPathMatch(), is("/Foo/bar.ext")); + + // assertEquals("/Foo/bar.ext", new ServletPathSpec("/").getPathMatch("/Foo/bar.ext"), "pathMatch default"); + spec = new ServletPathSpec("/"); + assertThat("PathMatch default", spec.matched("/Foo/bar.ext").getPathMatch(), is("/Foo/bar.ext")); + + // assertEquals("", new ServletPathSpec("").getPathMatch("/"), "pathInfo root"); + // assertEquals("", new ServletPathSpec("").getPathMatch(""), "pathInfo root"); + spec = new ServletPathSpec(""); + assertThat("PathMatch root", spec.matched("/").getPathMatch(), is("")); + assertThat("PathMatch root", spec.matched(""), is(nullValue())); // does not match // TODO: verify with greg + + // assertEquals("", new ServletPathSpec("/*").getPathMatch("/xxx/zzz"), "pathMatch default"); + spec = new ServletPathSpec("/*"); + assertThat("PathMatch default", spec.matched("/xxx/zzz").getPathMatch(), is("")); } @Test @@ -168,9 +210,17 @@ public void testPrefixPathSpec() assertMatches(spec, "/downloads"); - assertEquals("/", spec.getPathInfo("/downloads/"), "Spec.pathInfo"); - assertEquals("/distribution.zip", spec.getPathInfo("/downloads/distribution.zip"), "Spec.pathInfo"); - assertEquals("/dist/9.0/distribution.tar.gz", spec.getPathInfo("/downloads/dist/9.0/distribution.tar.gz"), "Spec.pathInfo"); + MatchedPath matched = spec.matched("/downloads/"); + assertThat("matched.pathMatch", matched.getPathMatch(), is("/downloads")); + assertThat("matched.pathInfo", matched.getPathInfo(), is("/")); + + matched = spec.matched("/downloads/distribution.zip"); + assertThat("matched.pathMatch", matched.getPathMatch(), is("/downloads")); + assertThat("matched.pathInfo", matched.getPathInfo(), is("/distribution.zip")); + + matched = spec.matched("/downloads/dist/9.0/distribution.tar.gz"); + assertThat("matched.pathMatch", matched.getPathMatch(), is("/downloads")); + assertThat("matched.pathInfo", matched.getPathInfo(), is("/dist/9.0/distribution.tar.gz")); } @Test @@ -187,7 +237,9 @@ public void testSuffixPathSpec() assertNotMatches(spec, "/downloads/distribution.tgz"); assertNotMatches(spec, "/abs/path"); - assertEquals(null, spec.getPathInfo("/downloads/distribution.tar.gz"), "Spec.pathInfo"); + MatchedPath matched = spec.matched("/downloads/distribution.tar.gz"); + assertThat("Suffix.pathMatch", matched.getPathMatch(), is("/downloads/distribution.tar.gz")); + assertThat("Suffix.pathInfo", matched.getPathInfo(), is(nullValue())); } @Test @@ -201,5 +253,4 @@ public void testEquals() assertThat(new ServletPathSpec("/bar/foo"), not(equalTo(new ServletPathSpec("/foo/bar")))); assertThat(new ServletPathSpec("/foo"), not(equalTo(new RegexPathSpec("/foo")))); } - } diff --git a/jetty-http/src/test/resources/jetty-logging.properties b/jetty-http/src/test/resources/jetty-logging.properties index 799aa62aed31..43542ff2bfc6 100644 --- a/jetty-http/src/test/resources/jetty-logging.properties +++ b/jetty-http/src/test/resources/jetty-logging.properties @@ -2,3 +2,4 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG +org.eclipse.jetty.http.pathmap.LEVEL=DEBUG From c664d494d2854a1fcaa496bd5652d713454ff32e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 2 May 2022 12:03:32 -0500 Subject: [PATCH 02/17] Issue #7891 - Improved PathSpec usage Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/MatchedPath.java | 18 ++- .../jetty/http/pathmap/MatchedResource.java | 51 +++++++ .../jetty/http/pathmap/PathMappings.java | 37 +++-- .../jetty/http/pathmap/PathSpecSet.java | 10 +- .../jetty/http/pathmap/RegexPathSpec.java | 6 - .../jetty/http/pathmap/ServletPathSpec.java | 20 +-- .../http/pathmap/UriTemplatePathSpec.java | 6 - .../jetty/http/pathmap/PathMappingsTest.java | 142 +++++------------- .../pathmap/ServletPathSpecOrderTest.java | 5 +- .../http/pathmap/ServletPathSpecTest.java | 102 +++++-------- .../test/resources/jetty-logging.properties | 2 +- .../jetty/server/AbstractNCSARequestLog.java | 2 +- .../jetty/server/CustomRequestLog.java | 2 +- .../org/eclipse/jetty/servlet/Invoker.java | 8 +- .../eclipse/jetty/servlet/ServletHandler.java | 44 ++++-- .../server/NativeWebSocketConfiguration.java | 2 +- 16 files changed, 225 insertions(+), 232 deletions(-) create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java index 87422c042bbb..b8e5ffdd5895 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java @@ -20,10 +20,20 @@ public interface MatchedPath { - /** - * @return the raw input path as provided. - */ - String getPath(); + MatchedPath EMPTY = new MatchedPath() + { + @Override + public String getPathMatch() + { + return null; + } + + @Override + public String getPathInfo() + { + return null; + } + }; /** * Return the portion of the path that matches a path spec. 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 new file mode 100644 index 000000000000..a2811a0ec0c9 --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedResource.java @@ -0,0 +1,51 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// 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; + +public class MatchedResource +{ + private final MappedResource mappedResource; + private final MatchedPath matchedPath; + + public MatchedResource(MappedResource resource, MatchedPath matchedPath) + { + this.mappedResource = resource; + this.matchedPath = matchedPath; + } + + public MappedResource getMappedResource() + { + return this.mappedResource; + } + + public PathSpec getPathSpec() + { + return mappedResource.getPathSpec(); + } + + public E getResource() + { + return mappedResource.getResource(); + } + + public MatchedPath getMatchedPath() + { + return matchedPath; + } +} 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 ed7f83b354ff..48c97153fade 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 @@ -120,8 +120,9 @@ public List> getMatches(String path) return ret; } - public MappedResource getMatch(String path) + public MatchedResource getMatched(String path) { + MatchedPath matchedPath = null; PathSpecGroup lastGroup = null; // Search all the mappings @@ -142,8 +143,10 @@ public MappedResource getMatch(String path) MappedResource candidate = exact_map.getBest(path, 0, i); if (candidate == null) break; - if (candidate.getPathSpec().matches(path)) - return candidate; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate, matchedPath); i = candidate.getPathSpec().getPrefix().length() - 1; } break; @@ -158,8 +161,10 @@ public MappedResource getMatch(String path) MappedResource candidate = prefix_map.getBest(path, 0, i); if (candidate == null) break; - if (candidate.getPathSpec().matches(path)) - return candidate; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate, matchedPath); i = candidate.getPathSpec().getPrefix().length() - 1; } break; @@ -172,8 +177,12 @@ public MappedResource getMatch(String path) while ((i = path.indexOf('.', i + 1)) > 0) { MappedResource candidate = suffix_map.get(path, i + 1, path.length() - i - 1); - if (candidate != null && candidate.getPathSpec().matches(path)) - return candidate; + if (candidate == null) + break; + + matchedPath = candidate.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(candidate, matchedPath); } break; } @@ -182,8 +191,9 @@ public MappedResource getMatch(String path) } } - if (mr.getPathSpec().matches(path)) - return mr; + matchedPath = mr.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(mr, matchedPath); lastGroup = group; } @@ -191,6 +201,15 @@ public MappedResource getMatch(String path) return null; } + /** + * @deprecated use {@link #getMatched(String)} instead + */ + @Deprecated + public MappedResource getMatch(String path) + { + return getMatched(path).getMappedResource(); + } + @Override public Iterator> iterator() { 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 724889585ef5..5574cfd0e8da 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 @@ -20,6 +20,7 @@ import java.util.AbstractSet; import java.util.Iterator; +import java.util.Objects; import java.util.function.Predicate; /** @@ -34,7 +35,7 @@ public class PathSpecSet extends AbstractSet implements Predicate pathmap, String path, String expectedValue) { - String msg = String.format(".getMatch(\"%s\")", path); - MappedResource match = pathmap.getMatch(path); - assertThat(msg, match, notNullValue()); - String actualMatch = match.getResource(); + String msg = String.format(".getMatched(\"%s\")", path); + MatchedResource matched = pathmap.getMatched(path); + assertThat(msg, matched, notNullValue()); + String actualMatch = matched.getResource(); assertEquals(expectedValue, actualMatch, msg); } - public void dumpMappings(PathMappings p) - { - for (MappedResource res : p) - { - System.out.printf(" %s%n", res); - } - } - /** * Test the match order rules with a mixed Servlet and regex path specs - *

+ * *

    *
  • Exact match
    • *
  • Longest prefix match
    • @@ -107,7 +96,7 @@ public void testServletMatchDefault() /** * Test the match order rules with a mixed Servlet and URI Template path specs - *

    + * *

      *
    • Exact match
      • *
    • Longest prefix match
      • @@ -141,7 +130,7 @@ public void testMixedMatchUriOrder() /** * Test the match order rules for URI Template based specs - *

      + * *

        *
      • Exact match
        • *
      • Longest prefix match
        • @@ -169,7 +158,7 @@ public void testUriTemplateMatchOrder() } @Test - public void testPathMap() throws Exception + public void testPathMap() { PathMappings p = new PathMappings<>(); @@ -185,74 +174,38 @@ public void testPathMap() throws Exception p.put(new ServletPathSpec(""), "10"); p.put(new ServletPathSpec("/\u20ACuro/*"), "11"); - assertEquals("/Foo/bar", new ServletPathSpec("/Foo/bar").getPathMatch("/Foo/bar"), "pathMatch exact"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/bar"), "pathMatch prefix"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/"), "pathMatch prefix"); - assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo"), "pathMatch prefix"); - assertEquals("/Foo/bar.ext", new ServletPathSpec("*.ext").getPathMatch("/Foo/bar.ext"), "pathMatch suffix"); - assertEquals("/Foo/bar.ext", new ServletPathSpec("/").getPathMatch("/Foo/bar.ext"), "pathMatch default"); - - assertEquals(null, new ServletPathSpec("/Foo/bar").getPathInfo("/Foo/bar"), "pathInfo exact"); - assertEquals("/bar", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/bar"), "pathInfo prefix"); - assertEquals("/*", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/*"), "pathInfo prefix"); - assertEquals("/", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/"), "pathInfo prefix"); - assertEquals(null, new ServletPathSpec("/Foo/*").getPathInfo("/Foo"), "pathInfo prefix"); - assertEquals(null, new ServletPathSpec("*.ext").getPathInfo("/Foo/bar.ext"), "pathInfo suffix"); - assertEquals(null, new ServletPathSpec("/").getPathInfo("/Foo/bar.ext"), "pathInfo default"); - p.put(new ServletPathSpec("/*"), "0"); // assertEquals("1", p.get("/abs/path"), "Get absolute path"); - assertEquals("/abs/path", p.getMatch("/abs/path").getPathSpec().getDeclaration(), "Match absolute path"); - assertEquals("1", p.getMatch("/abs/path").getResource(), "Match absolute path"); - assertEquals("0", p.getMatch("/abs/path/xxx").getResource(), "Mismatch absolute path"); - assertEquals("0", p.getMatch("/abs/pith").getResource(), "Mismatch absolute path"); - assertEquals("2", p.getMatch("/abs/path/longer").getResource(), "Match longer absolute path"); - assertEquals("0", p.getMatch("/abs/path/").getResource(), "Not exact absolute path"); - assertEquals("0", p.getMatch("/abs/path/xxx").getResource(), "Not exact absolute path"); - - assertEquals("3", p.getMatch("/animal/bird/eagle/bald").getResource(), "Match longest prefix"); - assertEquals("4", p.getMatch("/animal/fish/shark/grey").getResource(), "Match longest prefix"); - assertEquals("5", p.getMatch("/animal/insect/bug").getResource(), "Match longest prefix"); - assertEquals("5", p.getMatch("/animal").getResource(), "mismatch exact prefix"); - assertEquals("5", p.getMatch("/animal/").getResource(), "mismatch exact prefix"); - - assertEquals("0", p.getMatch("/suffix/path.tar.gz").getResource(), "Match longest suffix"); - assertEquals("0", p.getMatch("/suffix/path.gz").getResource(), "Match longest suffix"); - assertEquals("5", p.getMatch("/animal/path.gz").getResource(), "prefix rather than suffix"); - - assertEquals("0", p.getMatch("/Other/path").getResource(), "default"); - - assertEquals("", new ServletPathSpec("/*").getPathMatch("/xxx/zzz"), "pathMatch /*"); - assertEquals("/xxx/zzz", new ServletPathSpec("/*").getPathInfo("/xxx/zzz"), "pathInfo /*"); - - assertTrue(new ServletPathSpec("/").matches("/anything"), "match /"); - assertTrue(new ServletPathSpec("/*").matches("/anything"), "match /*"); - assertTrue(new ServletPathSpec("/foo").matches("/foo"), "match /foo"); - assertTrue(!new ServletPathSpec("/foo").matches("/bar"), "!match /foo"); - assertTrue(new ServletPathSpec("/foo/*").matches("/foo"), "match /foo/*"); - assertTrue(new ServletPathSpec("/foo/*").matches("/foo/"), "match /foo/*"); - assertTrue(new ServletPathSpec("/foo/*").matches("/foo/anything"), "match /foo/*"); - assertTrue(!new ServletPathSpec("/foo/*").matches("/bar"), "!match /foo/*"); - assertTrue(!new ServletPathSpec("/foo/*").matches("/bar/"), "!match /foo/*"); - assertTrue(!new ServletPathSpec("/foo/*").matches("/bar/anything"), "!match /foo/*"); - assertTrue(new ServletPathSpec("*.foo").matches("anything.foo"), "match *.foo"); - assertTrue(!new ServletPathSpec("*.foo").matches("anything.bar"), "!match *.foo"); - assertTrue(new ServletPathSpec("/On*").matches("/On*"), "match /On*"); - assertTrue(!new ServletPathSpec("/On*").matches("/One"), "!match /One"); - - assertEquals("10", p.getMatch("/").getResource(), "match / with ''"); - - assertTrue(new ServletPathSpec("").matches("/"), "match \"\""); + assertEquals("/abs/path", p.getMatched("/abs/path").getPathSpec().getDeclaration(), "Match absolute path"); + assertEquals("1", p.getMatched("/abs/path").getResource(), "Match absolute path"); + assertEquals("0", p.getMatched("/abs/path/xxx").getResource(), "Mismatch absolute path"); + assertEquals("0", p.getMatched("/abs/pith").getResource(), "Mismatch absolute path"); + assertEquals("2", p.getMatched("/abs/path/longer").getResource(), "Match longer absolute path"); + assertEquals("0", p.getMatched("/abs/path/").getResource(), "Not exact absolute path"); + assertEquals("0", p.getMatched("/abs/path/xxx").getResource(), "Not exact absolute path"); + + assertEquals("3", p.getMatched("/animal/bird/eagle/bald").getResource(), "Match longest prefix"); + assertEquals("4", p.getMatched("/animal/fish/shark/grey").getResource(), "Match longest prefix"); + assertEquals("5", p.getMatched("/animal/insect/bug").getResource(), "Match longest prefix"); + assertEquals("5", p.getMatched("/animal").getResource(), "mismatch exact prefix"); + assertEquals("5", p.getMatched("/animal/").getResource(), "mismatch exact prefix"); + + assertEquals("0", p.getMatched("/suffix/path.tar.gz").getResource(), "Match longest suffix"); + assertEquals("0", p.getMatched("/suffix/path.gz").getResource(), "Match longest suffix"); + assertEquals("5", p.getMatched("/animal/path.gz").getResource(), "prefix rather than suffix"); + + assertEquals("0", p.getMatched("/Other/path").getResource(), "default"); + + assertEquals("10", p.getMatched("/").getResource(), "match / with ''"); } /** * See JIRA issue: JETTY-88. * - * @throws Exception failed test */ @Test - public void testPathMappingsOnlyMatchOnDirectoryNames() throws Exception + public void testPathMappingsOnlyMatchOnDirectoryNames() { ServletPathSpec spec = new ServletPathSpec("/xyz/*"); @@ -271,40 +224,25 @@ public void testPathMappingsOnlyMatchOnDirectoryNames() throws Exception } @Test - public void testPrecidenceVsOrdering() throws Exception + public void testPrecedenceVsOrdering() { PathMappings p = new PathMappings<>(); p.put(new ServletPathSpec("/dump/gzip/*"), "prefix"); p.put(new ServletPathSpec("*.txt"), "suffix"); - assertEquals(null, p.getMatch("/foo/bar")); - assertEquals("prefix", p.getMatch("/dump/gzip/something").getResource()); - assertEquals("suffix", p.getMatch("/foo/something.txt").getResource()); - assertEquals("prefix", p.getMatch("/dump/gzip/something.txt").getResource()); + assertNull(p.getMatched("/foo/bar")); + assertEquals("prefix", p.getMatched("/dump/gzip/something").getResource()); + assertEquals("suffix", p.getMatched("/foo/something.txt").getResource()); + assertEquals("prefix", p.getMatched("/dump/gzip/something.txt").getResource()); p = new PathMappings<>(); p.put(new ServletPathSpec("*.txt"), "suffix"); p.put(new ServletPathSpec("/dump/gzip/*"), "prefix"); - assertEquals(null, p.getMatch("/foo/bar")); - assertEquals("prefix", p.getMatch("/dump/gzip/something").getResource()); - assertEquals("suffix", p.getMatch("/foo/something.txt").getResource()); - assertEquals("prefix", p.getMatch("/dump/gzip/something.txt").getResource()); - } - - @ParameterizedTest - @ValueSource(strings = { - "*", - "/foo/*/bar", - "*/foo", - "*.foo/*" - }) - public void testBadPathSpecs(String str) - { - assertThrows(IllegalArgumentException.class, () -> - { - new ServletPathSpec(str); - }); + assertNull(p.getMatched("/foo/bar")); + assertEquals("prefix", p.getMatched("/dump/gzip/something").getResource()); + assertEquals("suffix", p.getMatched("/foo/something.txt").getResource()); + assertEquals("prefix", p.getMatched("/dump/gzip/something.txt").getResource()); } @Test diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecOrderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecOrderTest.java index cfcf6084f6ab..09832048edb0 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecOrderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecOrderTest.java @@ -29,7 +29,7 @@ import static org.hamcrest.Matchers.is; /** - * Tests of {@link PathMappings#getMatch(String)}, with a focus on correct mapping selection order + * Tests of {@link PathMappings#getMatched(String)}, with a focus on correct mapping selection order */ @SuppressWarnings("Duplicates") public class ServletPathSpecOrderTest @@ -58,6 +58,7 @@ public static Stream data() data.add(Arguments.of("/Other/path", "default")); // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck data.add(Arguments.of("/\u20ACuro/path", "money")); + // @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck data.add(Arguments.of("/", "root")); // Extra tests @@ -92,6 +93,6 @@ public static Stream data() @MethodSource("data") public void testMatch(String inputPath, String expectedResource) { - assertThat("Match on [" + inputPath + "]", mappings.getMatch(inputPath).getResource(), is(expectedResource)); + assertThat("Match on [" + inputPath + "]", mappings.getMatched(inputPath).getResource(), is(expectedResource)); } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java index a5610fe7758d..1cdf5bed34b8 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java @@ -19,6 +19,8 @@ package org.eclipse.jetty.http.pathmap; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -26,24 +28,12 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ServletPathSpecTest { - private void assertBadServletPathSpec(String pathSpec) - { - try - { - new ServletPathSpec(pathSpec); - fail("Expected IllegalArgumentException for a bad servlet pathspec on: " + pathSpec); - } - catch (IllegalArgumentException e) - { - // expected path - System.out.println(e); - } - } - private void assertMatches(ServletPathSpec spec, String path) { String msg = String.format("Spec(\"%s\").matches(\"%s\")", spec.getDeclaration(), path); @@ -56,34 +46,21 @@ private void assertNotMatches(ServletPathSpec spec, String path) assertThat(msg, spec.matched(path), is(nullValue())); } - @Test - public void testBadServletPathSpecA() - { - assertBadServletPathSpec("foo"); - } - - @Test - public void testBadServletPathSpecB() - { - assertBadServletPathSpec("/foo/*.do"); - } - - @Test - public void testBadServletPathSpecC() - { - assertBadServletPathSpec("foo/*.do"); - } - - @Test - public void testBadServletPathSpecD() - { - assertBadServletPathSpec("foo/*.*do"); - } - - @Test - public void testBadServletPathSpecE() + @ParameterizedTest + @ValueSource(strings = { + "foo", + "/foo/*.do", + "foo/*.do", + "foo/*.*do", + "*", + "*do", + "/foo/*/bar", + "*/foo", + "*.foo/*" + }) + public void testBadPathSpecs(String str) { - assertBadServletPathSpec("*do"); + assertThrows(IllegalArgumentException.class, () -> new ServletPathSpec(str)); } @Test @@ -113,35 +90,25 @@ public void testExactPathSpec() @Test public void testGetPathInfo() { - // assertEquals(null, new ServletPathSpec("/Foo/bar").getPathInfo("/Foo/bar"), "pathInfo exact"); ServletPathSpec spec = new ServletPathSpec("/Foo/bar"); assertThat("PathInfo exact", spec.matched("/Foo/bar").getPathInfo(), is(nullValue())); - // assertEquals("/bar", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/bar"), "pathInfo prefix"); - // assertEquals("/*", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/*"), "pathInfo prefix"); - // assertEquals("/", new ServletPathSpec("/Foo/*").getPathInfo("/Foo/"), "pathInfo prefix"); - // assertEquals(null, new ServletPathSpec("/Foo/*").getPathInfo("/Foo"), "pathInfo prefix"); spec = new ServletPathSpec("/Foo/*"); assertThat("PathInfo prefix", spec.matched("/Foo/bar").getPathInfo(), is("/bar")); assertThat("PathInfo prefix", spec.matched("/Foo/*").getPathInfo(), is("/*")); assertThat("PathInfo prefix", spec.matched("/Foo/").getPathInfo(), is("/")); assertThat("PathInfo prefix", spec.matched("/Foo").getPathInfo(), is(nullValue())); - // assertEquals(null, new ServletPathSpec("*.ext").getPathInfo("/Foo/bar.ext"), "pathInfo suffix"); spec = new ServletPathSpec("*.ext"); assertThat("PathInfo suffix", spec.matched("/Foo/bar.ext").getPathInfo(), is(nullValue())); - // assertEquals(null, new ServletPathSpec("/").getPathInfo("/Foo/bar.ext"), "pathInfo default"); spec = new ServletPathSpec("/"); assertThat("PathInfo default", spec.matched("/Foo/bar.ext").getPathInfo(), is(nullValue())); - // assertEquals("/", new ServletPathSpec("").getPathInfo("/"), "pathInfo root"); - // assertEquals("", new ServletPathSpec("").getPathInfo(""), "pathInfo root"); spec = new ServletPathSpec(""); assertThat("PathInfo root", spec.matched("/").getPathInfo(), is("/")); assertThat("PathInfo root", spec.matched(""), is(nullValue())); // does not match // TODO: verify with greg - // assertEquals("/xxx/zzz", new ServletPathSpec("/*").getPathInfo("/xxx/zzz"), "pathInfo default"); spec = new ServletPathSpec("/*"); assertThat("PathInfo default", spec.matched("/xxx/zzz").getPathInfo(), is("/xxx/zzz")); } @@ -165,33 +132,24 @@ public void testRootPathSpec() @Test public void testPathMatch() { - // assertEquals("/Foo/bar", new ServletPathSpec("/Foo/bar").getPathMatch("/Foo/bar"), "pathMatch exact"); ServletPathSpec spec = new ServletPathSpec("/Foo/bar"); assertThat("PathMatch exact", spec.matched("/Foo/bar").getPathMatch(), is("/Foo/bar")); - // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/bar"), "pathMatch prefix"); - // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo/"), "pathMatch prefix"); - // assertEquals("/Foo", new ServletPathSpec("/Foo/*").getPathMatch("/Foo"), "pathMatch prefix"); spec = new ServletPathSpec("/Foo/*"); assertThat("PathMatch prefix", spec.matched("/Foo/bar").getPathMatch(), is("/Foo")); assertThat("PathMatch prefix", spec.matched("/Foo/").getPathMatch(), is("/Foo")); assertThat("PathMatch prefix", spec.matched("/Foo").getPathMatch(), is("/Foo")); - // assertEquals("/Foo/bar.ext", new ServletPathSpec("*.ext").getPathMatch("/Foo/bar.ext"), "pathMatch suffix"); spec = new ServletPathSpec("*.ext"); assertThat("PathMatch suffix", spec.matched("/Foo/bar.ext").getPathMatch(), is("/Foo/bar.ext")); - // assertEquals("/Foo/bar.ext", new ServletPathSpec("/").getPathMatch("/Foo/bar.ext"), "pathMatch default"); spec = new ServletPathSpec("/"); assertThat("PathMatch default", spec.matched("/Foo/bar.ext").getPathMatch(), is("/Foo/bar.ext")); - // assertEquals("", new ServletPathSpec("").getPathMatch("/"), "pathInfo root"); - // assertEquals("", new ServletPathSpec("").getPathMatch(""), "pathInfo root"); spec = new ServletPathSpec(""); assertThat("PathMatch root", spec.matched("/").getPathMatch(), is("")); assertThat("PathMatch root", spec.matched(""), is(nullValue())); // does not match // TODO: verify with greg - // assertEquals("", new ServletPathSpec("/*").getPathMatch("/xxx/zzz"), "pathMatch default"); spec = new ServletPathSpec("/*"); assertThat("PathMatch default", spec.matched("/xxx/zzz").getPathMatch(), is("")); } @@ -223,6 +181,28 @@ public void testPrefixPathSpec() assertThat("matched.pathInfo", matched.getPathInfo(), is("/dist/9.0/distribution.tar.gz")); } + @Test + public void testMatches() + { + assertTrue(new ServletPathSpec("/").matches("/anything"), "match /"); + assertTrue(new ServletPathSpec("/*").matches("/anything"), "match /*"); + assertTrue(new ServletPathSpec("/foo").matches("/foo"), "match /foo"); + assertFalse(new ServletPathSpec("/foo").matches("/bar"), "!match /foo"); + assertTrue(new ServletPathSpec("/foo/*").matches("/foo"), "match /foo/*"); + assertTrue(new ServletPathSpec("/foo/*").matches("/foo/"), "match /foo/*"); + assertTrue(new ServletPathSpec("/foo/*").matches("/foo/anything"), "match /foo/*"); + assertFalse(new ServletPathSpec("/foo/*").matches("/bar"), "!match /foo/*"); + assertFalse(new ServletPathSpec("/foo/*").matches("/bar/"), "!match /foo/*"); + assertFalse(new ServletPathSpec("/foo/*").matches("/bar/anything"), "!match /foo/*"); + assertTrue(new ServletPathSpec("*.foo").matches("anything.foo"), "match *.foo"); + assertFalse(new ServletPathSpec("*.foo").matches("anything.bar"), "!match *.foo"); + assertTrue(new ServletPathSpec("/On*").matches("/On*"), "match /On*"); + assertFalse(new ServletPathSpec("/On*").matches("/One"), "!match /One"); + + assertTrue(new ServletPathSpec("").matches("/"), "match \"\""); + + } + @Test public void testSuffixPathSpec() { diff --git a/jetty-http/src/test/resources/jetty-logging.properties b/jetty-http/src/test/resources/jetty-logging.properties index 43542ff2bfc6..8d7b23b70226 100644 --- a/jetty-http/src/test/resources/jetty-logging.properties +++ b/jetty-http/src/test/resources/jetty-logging.properties @@ -2,4 +2,4 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG -org.eclipse.jetty.http.pathmap.LEVEL=DEBUG +#org.eclipse.jetty.http.pathmap.LEVEL=DEBUG diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java index cf963fea62b9..ed5203615e48 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java @@ -105,7 +105,7 @@ public void log(Request request, Response response) { try { - if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null) + if (_ignorePathMap != null && _ignorePathMap.getMatched(request.getRequestURI()) != null) return; if (!isEnabled()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java index dce0fa846314..2ddaa215a69f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java @@ -327,7 +327,7 @@ public void log(Request request, Response response) { try { - if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null) + if (_ignorePathMap != null && _ignorePathMap.getMatched(request.getRequestURI()) != null) return; StringBuilder sb = _buffers.get(); diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java index 97fb156b8c68..cd6fd5a779de 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java @@ -31,7 +31,7 @@ import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.server.Dispatcher; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; @@ -71,7 +71,7 @@ public class Invoker extends HttpServlet private ContextHandler _contextHandler; private ServletHandler _servletHandler; - private MappedResource _invokerEntry; + private MatchedResource _invokerEntry; private Map _parameters; private boolean _nonContextServlets; private boolean _verbose; @@ -167,11 +167,11 @@ protected void service(HttpServletRequest request, HttpServletResponse response) synchronized (_servletHandler) { // find the entry for the invoker (me) - _invokerEntry = _servletHandler.getMappedServlet(servletPath); + _invokerEntry = _servletHandler.getMatchedServlet(servletPath); // Check for existing mapping (avoid threaded race). String path = URIUtil.addPaths(servletPath, servlet); - MappedResource entry = _servletHandler.getMappedServlet(path); + MatchedResource entry = _servletHandler.getMatchedServlet(path); if (entry != null && !entry.equals(_invokerEntry)) { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index ce9d4b0f01ad..8cd15c29c025 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -50,6 +50,8 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.MatchedPath; +import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.http.pathmap.PathMappings; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.http.pathmap.ServletPathSpec; @@ -369,13 +371,13 @@ public FilterHolder[] getFilters() * * @param target Path within _context or servlet name * @return PathMap Entries pathspec to ServletHolder - * @deprecated Use {@link #getMappedServlet(String)} + * @deprecated Use {@link #getMatchedServlet(String)} instead */ @Deprecated public MappedResource getHolderEntry(String target) { if (target.startsWith("/")) - return getMappedServlet(target); + return getMatchedServlet(target).getMappedResource(); return null; } @@ -465,16 +467,15 @@ public void doScope(String target, Request baseRequest, HttpServletRequest reque ServletHolder servletHolder = null; UserIdentity.Scope oldScope = null; - MappedResource mapping = getMappedServlet(target); - if (mapping != null) + MatchedResource matched = getMatchedServlet(target); + if (matched != null) { - servletHolder = mapping.getResource(); + servletHolder = matched.getResource(); - if (mapping.getPathSpec() != null) + if (matched.getPathSpec() != null) { - PathSpec pathSpec = mapping.getPathSpec(); - String servletPath = pathSpec.getPathMatch(target); - String pathInfo = pathSpec.getPathInfo(target); + String servletPath = matched.getMatchedPath().getPathMatch(); + String pathInfo = matched.getMatchedPath().getPathInfo(); if (DispatcherType.INCLUDE.equals(type)) { @@ -558,24 +559,39 @@ public void doHandle(String target, Request baseRequest, HttpServletRequest requ } /** - * ServletHolder matching path. + * ServletHolder matching target path. * * @param target Path within _context or servlet name - * @return MappedResource to the ServletHolder. Named servlets have a null PathSpec + * @return MatchedResource, pointing to the {@link MappedResource} for the {@link ServletHolder}, and also the pathspec specific name/info sections for the match. + * Named servlets have a null PathSpec and {@link MatchedResource}. */ - public MappedResource getMappedServlet(String target) + public MatchedResource getMatchedServlet(String target) { if (target.startsWith("/")) { if (_servletPathMap == null) return null; - return _servletPathMap.getMatch(target); + return _servletPathMap.getMatched(target); } ServletHolder holder = _servletNameMap.get(target); if (holder == null) return null; - return new MappedResource<>(null, holder); + MappedResource mappedResource = new MappedResource<>(null, holder); + return new MatchedResource<>(mappedResource, MatchedPath.EMPTY); + } + + /** + * ServletHolder matching path. + * + * @param target Path within _context or servlet name + * @return MappedResource to the ServletHolder. Named servlets have a null PathSpec + * @deprecated use {@link #getMatchedServlet(String)} instead + */ + @Deprecated + public MappedResource getMappedServlet(String target) + { + return getMatchedServlet(target).getMappedResource(); } protected FilterChain getFilterChain(Request baseRequest, String pathInContext, ServletHolder servletHolder) 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 4f751fca3628..17f0772c5b31 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 @@ -82,7 +82,7 @@ public WebSocketServerFactory getFactory() */ public MappedResource getMatch(String target) { - return this.mappings.getMatch(target); + return this.mappings.getMatched(target).getMappedResource(); } /** From 77b47a253d4a42aa0c9df30bb86bef143b843b81 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 3 May 2022 12:07:43 -0500 Subject: [PATCH 03/17] Issue #7891 - Improved RegexPathSpec signature detection Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/RegexPathSpec.java | 80 ++++++++++++++++--- .../jetty/http/pathmap/PathMappingsTest.java | 32 +++++--- .../jetty/http/pathmap/RegexPathSpecTest.java | 27 ++++++- 3 files changed, 117 insertions(+), 22 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index 1ae3adbebd5d..f8fd9a048aca 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.http.pathmap; +import java.util.HashMap; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -28,6 +30,20 @@ public class RegexPathSpec extends AbstractPathSpec { private static final Logger LOG = Log.getLogger(UriTemplatePathSpec.class); + private static final Map FORBIDDEN_ESCAPED = new HashMap<>(); + + static + { + FORBIDDEN_ESCAPED.put('s', "any whitespace"); + FORBIDDEN_ESCAPED.put('n', "newline"); + FORBIDDEN_ESCAPED.put('r', "carriage return"); + FORBIDDEN_ESCAPED.put('t', "tab"); + FORBIDDEN_ESCAPED.put('f', "form-feed"); + FORBIDDEN_ESCAPED.put('b', "bell"); + FORBIDDEN_ESCAPED.put('e', "escape"); + FORBIDDEN_ESCAPED.put('c', "control char"); + } + private final String _declaration; private final PathSpecGroup _group; private final int _pathDepth; @@ -43,34 +59,78 @@ public RegexPathSpec(String regex) declaration = regex; int specLength = declaration.length(); // build up a simple signature we can use to identify the grouping - boolean inGrouping = false; + boolean inTextList = false; + boolean inQuantifier = false; StringBuilder signature = new StringBuilder(); int pathDepth = 0; + char last = 0; for (int i = 0; i < declaration.length(); i++) { char c = declaration.charAt(i); switch (c) { - case '[': - inGrouping = true; + case '^': // ignore anchors + case '$': // ignore anchors + case '\'': // ignore escaping break; - case ']': - inGrouping = false; + case '+': // single char quantifier + case '?': // single char quantifier + case '*': // single char quantifier + case '|': // alternate match token + case '.': // any char token signature.append('g'); // glob break; - case '*': + case '{': + inQuantifier = true; + break; + case '}': + inQuantifier = false; + break; + case '[': + inTextList = true; + break; + case ']': + inTextList = false; signature.append('g'); // glob break; case '/': - if (!inGrouping) + if (!inTextList && !inQuantifier) pathDepth++; break; default: - if (!inGrouping && Character.isLetterOrDigit(c)) - signature.append('l'); // literal (exact) + if (!inTextList && !inQuantifier && Character.isLetterOrDigit(c)) + { + if (last == '\\') // escaped + { + String forbiddenReason = FORBIDDEN_ESCAPED.get(c); + if (forbiddenReason != null) + { + throw new IllegalArgumentException(String.format("%s does not support \\%c (%s) for \"%s\"", + this.getClass().getSimpleName(), c, forbiddenReason)); + } + switch (c) + { + case 'S': // any non-whitespace + case 'd': // any digits + case 'D': // any non-digits + case 'w': // any word + case 'W': // any non-word + signature.append('g'); // glob + break; + default: + signature.append('l'); // literal (exact) + break; + } + } + else + { + signature.append('l'); // literal (exact) + } + } break; } + last = c; } Pattern pattern = Pattern.compile(declaration); @@ -82,7 +142,7 @@ public RegexPathSpec(String regex) group = PathSpecGroup.EXACT; else if (Pattern.matches("^l*g+", sig)) group = PathSpecGroup.PREFIX_GLOB; - else if (Pattern.matches("^g+l+$", sig)) + else if (Pattern.matches("^g+l+.*", sig)) group = PathSpecGroup.SUFFIX_GLOB; else group = PathSpecGroup.MIDDLE_GLOB; 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 67efc3fcc8d8..678df783b17c 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 @@ -29,7 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck +// @chxxeckstyle-disable-check : AvoidEscapedUnicodeCharactersCheck public class PathMappingsTest { private void assertMatch(PathMappings pathmap, String path, String expectedValue) @@ -64,8 +64,6 @@ public void testMixedMatchOrder() p.put(new RegexPathSpec("^/animal/.*/cam$"), "animalCam"); p.put(new RegexPathSpec("^/entrance/cam$"), "entranceCam"); - // dumpMappings(p); - assertMatch(p, "/animal/bird/eagle", "birds"); assertMatch(p, "/animal/fish/bass/sea", "fishes"); assertMatch(p, "/animal/peccary/javalina/evolution", "animals"); @@ -116,8 +114,6 @@ public void testMixedMatchUriOrder() p.put(new UriTemplatePathSpec("/animal/{type}/{name}/cam"), "animalCam"); p.put(new UriTemplatePathSpec("/entrance/cam"), "entranceCam"); - // dumpMappings(p); - assertMatch(p, "/animal/bird/eagle", "birds"); assertMatch(p, "/animal/fish/bass/sea", "fishes"); assertMatch(p, "/animal/peccary/javalina/evolution", "animals"); @@ -148,8 +144,6 @@ public void testUriTemplateMatchOrder() p.put(new UriTemplatePathSpec("/{var1}/d"), "endpointD"); p.put(new UriTemplatePathSpec("/b/{var2}"), "endpointE"); - // dumpMappings(p); - assertMatch(p, "/a/b/c", "endpointB"); assertMatch(p, "/a/d/c", "endpointA"); assertMatch(p, "/a/x/y", "endpointC"); @@ -157,6 +151,26 @@ public void testUriTemplateMatchOrder() assertMatch(p, "/b/d", "endpointE"); } + /** + * Test the match order rules for mixed Servlet and Regex path specs + */ + @Test + public void testServletAndRegexMatchOrder() + { + PathMappings p = new PathMappings<>(); + + p.put(new ServletPathSpec("/a/*"), "endpointA"); + p.put(new RegexPathSpec("^.*/middle/.*$"), "middle"); + p.put(new ServletPathSpec("*.do"), "endpointDo"); + p.put(new ServletPathSpec("/"), "default"); + + assertMatch(p, "/a/b/c", "endpointA"); + assertMatch(p, "/a/middle/c", "endpointA"); + assertMatch(p, "/b/middle/c", "middle"); + assertMatch(p, "/x/y.do", "endpointDo"); + assertMatch(p, "/b/d", "default"); + } + @Test public void testPathMap() { @@ -172,11 +186,12 @@ public void testPathMap() p.put(new ServletPathSpec("/"), "8"); // p.put(new ServletPathSpec("/XXX:/YYY"), "9"); // special syntax from Jetty 3.1.x p.put(new ServletPathSpec(""), "10"); + // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck p.put(new ServletPathSpec("/\u20ACuro/*"), "11"); + // @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck p.put(new ServletPathSpec("/*"), "0"); - // assertEquals("1", p.get("/abs/path"), "Get absolute path"); assertEquals("/abs/path", p.getMatched("/abs/path").getPathSpec().getDeclaration(), "Match absolute path"); assertEquals("1", p.getMatched("/abs/path").getResource(), "Match absolute path"); assertEquals("0", p.getMatched("/abs/path/xxx").getResource(), "Mismatch absolute path"); @@ -202,7 +217,6 @@ public void testPathMap() /** * See JIRA issue: JETTY-88. - * */ @Test public void testPathMappingsOnlyMatchOnDirectoryNames() diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java index 514f67a199d4..30b395bb039d 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java @@ -22,10 +22,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class RegexPathSpecTest @@ -33,13 +34,13 @@ public class RegexPathSpecTest public static void assertMatches(PathSpec spec, String path) { String msg = String.format("Spec(\"%s\").matches(\"%s\")", spec.getDeclaration(), path); - assertThat(msg, spec.matches(path), is(true)); + assertNotNull(spec.matched(path), msg); } public static void assertNotMatches(PathSpec spec, String path) { String msg = String.format("!Spec(\"%s\").matches(\"%s\")", spec.getDeclaration(), path); - assertThat(msg, spec.matches(path), is(false)); + assertNull(spec.matched(path), msg); } @Test @@ -151,6 +152,26 @@ public void testSuffixSpec() assertNotMatches(spec, "/aa/bb.do/more"); } + @Test + public void testSuffixSpecMiddle() + { + RegexPathSpec spec = new RegexPathSpec("^.*/middle/.*$"); + assertEquals("^.*/middle/.*$", spec.getDeclaration(), "Spec.pathSpec"); + assertEquals("^.*/middle/.*$", spec.getPattern().pattern(), "Spec.pattern"); + assertEquals(2, spec.getPathDepth(), "Spec.pathDepth"); + assertEquals(PathSpecGroup.SUFFIX_GLOB, spec.getGroup(), "Spec.group"); + + assertMatches(spec, "/a/middle/c.do"); + assertMatches(spec, "/a/b/c/d/middle/e/f"); + assertMatches(spec, "/middle/"); + + assertNotMatches(spec, "/a.do"); + assertNotMatches(spec, "/a"); + assertNotMatches(spec, "/aa"); + assertNotMatches(spec, "/aa/bb"); + assertNotMatches(spec, "/aa/bb.do/more"); + } + @Test public void testEquals() { From ad9b83d4f1537ea4ef6fffeac677cd41ac3c554d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 3 May 2022 13:12:50 -0500 Subject: [PATCH 04/17] Issue #7891 - Improved RegexPathSpec signature detection with groups Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/RegexPathSpec.java | 6 +++-- .../jetty/http/pathmap/RegexPathSpecTest.java | 24 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index f8fd9a048aca..c33e153d5079 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -73,6 +73,8 @@ public RegexPathSpec(String regex) case '^': // ignore anchors case '$': // ignore anchors case '\'': // ignore escaping + case '(': // ignore grouping + case ')': // ignore grouping break; case '+': // single char quantifier case '?': // single char quantifier @@ -107,7 +109,7 @@ public RegexPathSpec(String regex) if (forbiddenReason != null) { throw new IllegalArgumentException(String.format("%s does not support \\%c (%s) for \"%s\"", - this.getClass().getSimpleName(), c, forbiddenReason)); + this.getClass().getSimpleName(), c, forbiddenReason, declaration)); } switch (c) { @@ -123,7 +125,7 @@ public RegexPathSpec(String regex) break; } } - else + else // not escaped { signature.append('l'); // literal (exact) } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java index 30b395bb039d..243e66e619a1 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java @@ -166,10 +166,26 @@ public void testSuffixSpecMiddle() assertMatches(spec, "/middle/"); assertNotMatches(spec, "/a.do"); - assertNotMatches(spec, "/a"); - assertNotMatches(spec, "/aa"); - assertNotMatches(spec, "/aa/bb"); - assertNotMatches(spec, "/aa/bb.do/more"); + assertNotMatches(spec, "/a/middle"); + assertNotMatches(spec, "/middle"); + } + + @Test + public void testSuffixSpecMiddleWithGroupings() + { + RegexPathSpec spec = new RegexPathSpec("^(.*)/middle/(.*)$"); + assertEquals("^(.*)/middle/(.*)$", spec.getDeclaration(), "Spec.pathSpec"); + assertEquals("^(.*)/middle/(.*)$", spec.getPattern().pattern(), "Spec.pattern"); + assertEquals(2, spec.getPathDepth(), "Spec.pathDepth"); + assertEquals(PathSpecGroup.SUFFIX_GLOB, spec.getGroup(), "Spec.group"); + + assertMatches(spec, "/a/middle/c.do"); + assertMatches(spec, "/a/b/c/d/middle/e/f"); + assertMatches(spec, "/middle/"); + + assertNotMatches(spec, "/a.do"); + assertNotMatches(spec, "/a/middle"); + assertNotMatches(spec, "/middle"); } @Test From 2edc71edf1b1cb15dfe23fb2bfe1ff68dbcdebea Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 3 May 2022 14:49:50 -0500 Subject: [PATCH 05/17] Issue #7891 - Fixing typo in PathSpecSet Signed-off-by: Joakim Erdfelt --- .../main/java/org/eclipse/jetty/http/pathmap/PathSpecSet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5574cfd0e8da..5d09dc19f05e 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 @@ -35,7 +35,7 @@ public class PathSpecSet extends AbstractSet implements Predicate Date: Tue, 3 May 2022 15:20:37 -0500 Subject: [PATCH 06/17] Issue #7891 - Fixing comparison in Invoker Signed-off-by: Joakim Erdfelt --- .../src/main/java/org/eclipse/jetty/servlet/Invoker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java index cd6fd5a779de..bf65276aa8c8 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java @@ -173,7 +173,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response) String path = URIUtil.addPaths(servletPath, servlet); MatchedResource entry = _servletHandler.getMatchedServlet(path); - if (entry != null && !entry.equals(_invokerEntry)) + if (entry != null && !entry.getMappedResource().equals(_invokerEntry.getMappedResource())) { // Use the holder holder = entry.getResource(); From 6c2968ae0c4b9cdaf66610f8147bd967e140282d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 4 May 2022 07:07:33 -0500 Subject: [PATCH 07/17] Issue #7891 - Correct no-match NativeWebSocketConfiguration condition Signed-off-by: Joakim Erdfelt --- .../websocket/server/NativeWebSocketConfiguration.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 17f0772c5b31..039711e63a6a 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 @@ -22,6 +22,7 @@ import javax.servlet.ServletContext; import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.http.pathmap.PathMappings; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.http.pathmap.RegexPathSpec; @@ -82,7 +83,10 @@ public WebSocketServerFactory getFactory() */ public MappedResource getMatch(String target) { - return this.mappings.getMatched(target).getMappedResource(); + MatchedResource matched = this.mappings.getMatched(target); + if (matched == null) + return null; + return matched.getMappedResource(); } /** From 7adf903589c4a31f592dd536f539f37c15aaa3ca Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 4 May 2022 07:12:48 -0500 Subject: [PATCH 08/17] Issue #7891 - Introduce MatchedPath.from Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/MatchedPath.java | 30 ++++++++++++++++ .../jetty/http/pathmap/ServletPathSpec.java | 34 +++---------------- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java index b8e5ffdd5895..44f7e57e9b0a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java @@ -33,8 +33,38 @@ public String getPathInfo() { return null; } + + @Override + public String toString() + { + return "MatchedPath.EMPTY"; + } }; + static MatchedPath from(String pathMatch, String pathInfo) + { + return new MatchedPath() + { + @Override + public String getPathMatch() + { + return pathMatch; + } + + @Override + public String getPathInfo() + { + return pathInfo; + } + + @Override + public String toString() + { + return "MatchedPath.from[pathMatch=" + pathMatch + ", pathInfo=" + pathInfo + "]"; + } + }; + } + /** * Return the portion of the path that matches a path spec. * diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index 1c4c663fddce..f78f26cf345b 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -291,7 +291,7 @@ public MatchedPath matched(String path) { case EXACT: if (_declaration.equals(path)) - return new ServletMatchedPath(path, null); // TODO: return final matchedpath + return MatchedPath.from(path, null); break; case PREFIX_GLOB: if (isWildcardMatch(path)) @@ -303,21 +303,21 @@ public MatchedPath matched(String path) pathMatch = path.substring(0, _specLength - 2); pathInfo = path.substring(_specLength - 2); } - return new ServletMatchedPath(pathMatch, pathInfo); + return MatchedPath.from(pathMatch, pathInfo); } break; case SUFFIX_GLOB: if (path.regionMatches((path.length() - _specLength) + 1, _declaration, 1, _specLength - 1)) - return new ServletMatchedPath(path, null); + return MatchedPath.from(path, null); break; case ROOT: // Only "/" matches if ("/".equals(path)) - return new ServletMatchedPath("", path); // TODO: review this + return MatchedPath.from("", path); break; case DEFAULT: // If we reached this point, then everything matches - return new ServletMatchedPath(path, null); + return MatchedPath.from(path, null); } return null; } @@ -343,28 +343,4 @@ public boolean matches(String path) return false; } } - - public static class ServletMatchedPath implements MatchedPath - { - private final String servletName; - private final String pathInfo; - - public ServletMatchedPath(String servletName, String pathInfo) - { - this.servletName = servletName; - this.pathInfo = pathInfo; - } - - @Override - public String getPathMatch() - { - return this.servletName; - } - - @Override - public String getPathInfo() - { - return this.pathInfo; - } - } } From b8137c5a141e4af2f045ff97c2a20e2034bd311c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 4 May 2022 07:16:03 -0500 Subject: [PATCH 09/17] Issue #7891 - Add toString() impls Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/http/pathmap/RegexPathSpec.java | 10 ++++++++++ .../jetty/http/pathmap/UriTemplatePathSpec.java | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index c33e153d5079..140a544187eb 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -337,5 +337,15 @@ public String getPathInfo() // default is null return null; } + + @Override + public String toString() + { + return "RegexMatchedPath[" + + "pathSpec=" + pathSpec + + ", path=\"" + path + "\"" + + ", matcher=" + matcher + + ']'; + } } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java index 23442801e7db..05aff17e690e 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java @@ -487,5 +487,15 @@ public String getPathInfo() } return null; } + + @Override + public String toString() + { + return "UriTemplateMatchedPath[" + + "pathSpec=" + pathSpec + + ", path=\"" + path + "\"" + + ", matcher=" + matcher + + ']'; + } } } From 3085e710fa0b6a6eabeb715d7a2cb1fd6a637e98 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 May 2022 10:31:40 -0500 Subject: [PATCH 10/17] 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. From 498b5c380d3fafaa2abead48a0b76c851d784aea Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 May 2022 12:21:59 -0500 Subject: [PATCH 11/17] Issue #7891 - Fixing compilation issues Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/PathMappings.java | 42 +++++++++---------- .../org/eclipse/jetty/servlet/Invoker.java | 2 +- .../eclipse/jetty/servlet/ServletHandler.java | 15 ++++--- 3 files changed, 29 insertions(+), 30 deletions(-) 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 e65b1b73330b..17682b8cf59c 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 @@ -150,6 +150,7 @@ public MatchedResource getMatched(String path) // Search all the mappings for (MappedResource mr : _mappings) { + boolean skipMatch = false; PathSpecGroup group = mr.getPathSpec().getGroup(); if (group != lastGroup) { @@ -161,6 +162,7 @@ public MatchedResource getMatched(String path) if (_optimizedExact) { int i = path.length(); + final Trie> exact_map = _exactMap; while (i >= 0) { @@ -170,15 +172,13 @@ public MatchedResource getMatched(String path) matchedPath = candidate.getPathSpec().matched(path); if (matchedPath != null) + { return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); - i = candidate.getPathSpec().getDeclaration().length() - 1; + } + i--; } - } - else - { - matchedPath = mr.getPathSpec().matched(path); - if (matchedPath != null) - return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); + // If we reached here, there's NO optimized EXACT Match possible, skip simple match below + skipMatch = true; } break; } @@ -198,14 +198,10 @@ public MatchedResource getMatched(String path) matchedPath = candidate.getPathSpec().matched(path); if (matchedPath != null) return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); - i = candidate.getPathSpec().getPrefix().length() - 1; + i--; } - } - else - { - matchedPath = mr.getPathSpec().matched(path); - if (matchedPath != null) - return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); + // If we reached here, there's NO optimized PREFIX Match possible, skip simple match below + skipMatch = true; } break; } @@ -226,12 +222,8 @@ public MatchedResource getMatched(String path) if (matchedPath != null) return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); } - } - else - { - matchedPath = mr.getPathSpec().matched(path); - if (matchedPath != null) - return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); + // If we reached here, there's NO optimized SUFFIX Match possible, skip simple match below + skipMatch = true; } break; } @@ -240,9 +232,13 @@ public MatchedResource getMatched(String path) } } - matchedPath = mr.getPathSpec().matched(path); - if (matchedPath != null) - return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); + // Only perform simple match if optimized match lets us + if (!skipMatch) + { + matchedPath = mr.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); + } lastGroup = group; } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java index bf65276aa8c8..788cefaf8733 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java @@ -173,7 +173,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response) String path = URIUtil.addPaths(servletPath, servlet); MatchedResource entry = _servletHandler.getMatchedServlet(path); - if (entry != null && !entry.getMappedResource().equals(_invokerEntry.getMappedResource())) + if (entry != null && !entry.getResource().equals(_invokerEntry.getResource())) { // Use the holder holder = entry.getResource(); diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 8cd15c29c025..0b1e29963608 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -377,7 +377,10 @@ public FilterHolder[] getFilters() public MappedResource getHolderEntry(String target) { if (target.startsWith("/")) - return getMatchedServlet(target).getMappedResource(); + { + MatchedResource matchedResource = getMatchedServlet(target); + return new MappedResource<>(matchedResource.getPathSpec(), matchedResource.getResource()); + } return null; } @@ -474,8 +477,8 @@ public void doScope(String target, Request baseRequest, HttpServletRequest reque if (matched.getPathSpec() != null) { - String servletPath = matched.getMatchedPath().getPathMatch(); - String pathInfo = matched.getMatchedPath().getPathInfo(); + String servletPath = matched.getPathMatch(); + String pathInfo = matched.getPathInfo(); if (DispatcherType.INCLUDE.equals(type)) { @@ -577,8 +580,7 @@ public MatchedResource getMatchedServlet(String target) ServletHolder holder = _servletNameMap.get(target); if (holder == null) return null; - MappedResource mappedResource = new MappedResource<>(null, holder); - return new MatchedResource<>(mappedResource, MatchedPath.EMPTY); + return new MatchedResource<>(holder, null, MatchedPath.EMPTY); } /** @@ -591,7 +593,8 @@ public MatchedResource getMatchedServlet(String target) @Deprecated public MappedResource getMappedServlet(String target) { - return getMatchedServlet(target).getMappedResource(); + MatchedResource matchedResource = getMatchedServlet(target); + return new MappedResource<>(matchedResource.getPathSpec(), matchedResource.getResource()); } protected FilterChain getFilterChain(Request baseRequest, String pathInContext, ServletHolder servletHolder) From fc053414c839205fd61322bf2c2be7cb4b877b5c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 May 2022 12:25:29 -0500 Subject: [PATCH 12/17] Issue #7891 - Better group match optimization logic Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/PathMappings.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) 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 17682b8cf59c..f98165672465 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 @@ -147,11 +147,17 @@ public MatchedResource getMatched(String path) MatchedPath matchedPath; PathSpecGroup lastGroup = null; + boolean skipRestOfGroup = false; // Search all the mappings for (MappedResource mr : _mappings) { - boolean skipMatch = false; PathSpecGroup group = mr.getPathSpec().getGroup(); + if (group == lastGroup && skipRestOfGroup) + { + continue; // skip + } + + // Run servlet spec optimizations on first hit of specific groups if (group != lastGroup) { // New group in list, so let's look for an optimization @@ -178,7 +184,7 @@ public MatchedResource getMatched(String path) i--; } // If we reached here, there's NO optimized EXACT Match possible, skip simple match below - skipMatch = true; + skipRestOfGroup = true; } break; } @@ -201,7 +207,7 @@ public MatchedResource getMatched(String path) i--; } // If we reached here, there's NO optimized PREFIX Match possible, skip simple match below - skipMatch = true; + skipRestOfGroup = true; } break; } @@ -223,7 +229,7 @@ public MatchedResource getMatched(String path) return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); } // If we reached here, there's NO optimized SUFFIX Match possible, skip simple match below - skipMatch = true; + skipRestOfGroup = true; } break; } @@ -232,13 +238,9 @@ public MatchedResource getMatched(String path) } } - // Only perform simple match if optimized match lets us - if (!skipMatch) - { - matchedPath = mr.getPathSpec().matched(path); - if (matchedPath != null) - return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); - } + matchedPath = mr.getPathSpec().matched(path); + if (matchedPath != null) + return new MatchedResource<>(mr.getResource(), mr.getPathSpec(), matchedPath); lastGroup = group; } From 20d5c8cf8f77c856bdd77a99d63024a587e88f14 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 May 2022 12:26:15 -0500 Subject: [PATCH 13/17] Issue #7891 - Better group match optimization logic Signed-off-by: Joakim Erdfelt --- .../main/java/org/eclipse/jetty/http/pathmap/PathMappings.java | 3 +++ 1 file changed, 3 insertions(+) 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 f98165672465..79c6fe352b69 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 @@ -160,6 +160,9 @@ public MatchedResource getMatched(String path) // Run servlet spec optimizations on first hit of specific groups if (group != lastGroup) { + // New group, reset skip logic + skipRestOfGroup = false; + // New group in list, so let's look for an optimization switch (group) { From 3d7cb4cbe83b42ab93d3f42d2ce540b504f86759 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 16 May 2022 14:58:12 -0500 Subject: [PATCH 14/17] Issue #7891 - Optimized .put() and .remove() + Implemented recalculations + Added more comments Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/PathMappings.java | 174 ++++++++++-------- 1 file changed, 102 insertions(+), 72 deletions(-) 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 79c6fe352b69..76ffcf8b24b7 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 @@ -292,93 +292,81 @@ public boolean put(String pathSpecString, E resource) public boolean put(PathSpec pathSpec, E resource) { MappedResource entry = new MappedResource<>(pathSpec, resource); - switch (pathSpec.getGroup()) + boolean added = _mappings.add(entry); + if (LOG.isDebugEnabled()) + LOG.debug("{} {} to {}", added ? "Added" : "Ignored", entry, this); + + if (added) { - case EXACT: - if (pathSpec instanceof ServletPathSpec) - { - String exact = pathSpec.getDeclaration(); - while (exact != null && !_exactMap.put(exact, entry)) + switch (pathSpec.getGroup()) + { + case EXACT: + if (pathSpec instanceof ServletPathSpec) { - _exactMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_exactMap, 1.5); + String exact = pathSpec.getDeclaration(); + while (exact != null && !_exactMap.put(exact, entry)) + { + // grow the capacity of the Trie + _exactMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_exactMap, 1.5); + } } - } - else - { - // This is not a Servlet mapping, turn off optimization on Exact - _optimizedExact = false; - } - break; - case PREFIX_GLOB: - if (pathSpec instanceof ServletPathSpec) - { - String prefix = pathSpec.getPrefix(); - while (prefix != null && !_prefixMap.put(prefix, entry)) + else { - _prefixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + // This is not a Servlet mapping, turn off optimization on Exact + // TODO: see if we can optimize all Regex / UriTemplate versions here too. + // Note: Example exact in Regex that can cause problems `^/a\Q/b\E/` (which is only ever matching `/a/b/`) + // Note: UriTemplate can handle exact easily enough + _optimizedExact = false; } - } - else - { - // This is not a Servlet mapping, turn off optimization on Prefix - _optimizedPrefix = false; - } - break; - case SUFFIX_GLOB: - if (pathSpec instanceof ServletPathSpec) - { - String suffix = pathSpec.getSuffix(); - while (suffix != null && !_suffixMap.put(suffix, entry)) + break; + case PREFIX_GLOB: + if (pathSpec instanceof ServletPathSpec) { - _suffixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + String prefix = pathSpec.getPrefix(); + while (prefix != null && !_prefixMap.put(prefix, entry)) + { + // grow the capacity of the Trie + _prefixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + } } - } - else - { - // This is not a Servlet mapping, turn off optimization on Suffix - _optimizedSuffix = false; - } - break; - default: + else + { + // This is not a Servlet mapping, turn off optimization on Prefix + // TODO: see if we can optimize all Regex / UriTemplate versions here too. + // Note: Example Prefix in Regex that can cause problems `^/a/b+` or `^/a/bb*` ('b' one or more times) + // Note: Example Prefix in UriTemplate that might cause problems `/a/{b}/{c}` + _optimizedPrefix = false; + } + break; + case SUFFIX_GLOB: + if (pathSpec instanceof ServletPathSpec) + { + String suffix = pathSpec.getSuffix(); + while (suffix != null && !_suffixMap.put(suffix, entry)) + { + // grow the capacity of the Trie + _suffixMap = new ArrayTernaryTrie<>((ArrayTernaryTrie>)_prefixMap, 1.5); + } + } + else + { + // This is not a Servlet mapping, turn off optimization on Suffix + // TODO: see if we can optimize all Regex / UriTemplate versions here too. + // Note: Example suffix in Regex that can cause problems `^.*/path/name.ext` or `^/a/.*(ending)` + // Note: Example suffix in UriTemplate that can cause problems `/{a}/name.ext` + _optimizedSuffix = false; + } + break; + default: + } } - boolean added = _mappings.add(entry); - if (LOG.isDebugEnabled()) - LOG.debug("{} {} to {}", added ? "Added" : "Ignored", entry, this); return added; } @SuppressWarnings("incomplete-switch") public boolean remove(PathSpec pathSpec) { - switch (pathSpec.getGroup()) - { - case EXACT: - 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; - } - Iterator> iter = _mappings.iterator(); boolean removed = false; while (iter.hasNext()) @@ -390,8 +378,50 @@ public boolean remove(PathSpec pathSpec) break; } } + if (LOG.isDebugEnabled()) LOG.debug("{} {} to {}", removed ? "Removed" : "Ignored", pathSpec, this); + + if (removed) + { + switch (pathSpec.getGroup()) + { + case EXACT: + String exact = pathSpec.getDeclaration(); + if (exact != null) + { + _exactMap.remove(exact); + // Recalculate _optimizeExact + _optimizedExact = _mappings.stream() + .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.EXACT) + .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + } + break; + case PREFIX_GLOB: + String prefix = pathSpec.getPrefix(); + if (prefix != null) + { + _prefixMap.remove(prefix); + // Recalculate _optimizePrefix + _optimizedPrefix = _mappings.stream() + .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.PREFIX_GLOB) + .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + } + break; + case SUFFIX_GLOB: + String suffix = pathSpec.getSuffix(); + if (suffix != null) + { + _suffixMap.remove(suffix); + // Recalculate _optimizeSuffix + _optimizedSuffix = _mappings.stream() + .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.SUFFIX_GLOB) + .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + } + break; + } + } + return removed; } From 0b9f2823286f529e1ae85f74a7d700d939f63013 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 4 Jun 2022 08:44:42 +1000 Subject: [PATCH 15/17] Introduced final preMatchedPath when possible --- .../jetty/http/pathmap/ServletPathSpec.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index f78f26cf345b..d75b97d80e73 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -30,8 +30,10 @@ public class ServletPathSpec extends AbstractPathSpec private final PathSpecGroup _group; private final int _pathDepth; private final int _specLength; + private final int _matchLength; private final String _prefix; private final String _suffix; + private final MatchedPath _preMatchedPath; /** * If a servlet or filter path mapping isn't a suffix mapping, ensure @@ -62,8 +64,10 @@ public ServletPathSpec(String servletPathSpec) _group = PathSpecGroup.ROOT; _pathDepth = -1; // Set pathDepth to -1 to force this to be at the end of the sort order. _specLength = 1; + _matchLength = 0; _prefix = null; _suffix = null; + _preMatchedPath = MatchedPath.from("", "/"); return; } @@ -74,8 +78,10 @@ public ServletPathSpec(String servletPathSpec) _group = PathSpecGroup.DEFAULT; _pathDepth = -1; // Set pathDepth to -1 to force this to be at the end of the sort order. _specLength = 1; + _matchLength = 0; _prefix = null; _suffix = null; + _preMatchedPath = null; return; } @@ -90,6 +96,7 @@ public ServletPathSpec(String servletPathSpec) group = PathSpecGroup.PREFIX_GLOB; prefix = servletPathSpec.substring(0, specLength - 2); suffix = null; + _preMatchedPath = MatchedPath.from(prefix, null); } // suffix based else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) @@ -97,6 +104,7 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) group = PathSpecGroup.SUFFIX_GLOB; prefix = null; suffix = servletPathSpec.substring(2, specLength); + _preMatchedPath = null; } else { @@ -108,6 +116,7 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) LOG.warn("Suspicious URL pattern: '{}'; see sections 12.1 and 12.2 of the Servlet specification", servletPathSpec); } + _preMatchedPath = MatchedPath.from(servletPathSpec, null); } int pathDepth = 0; @@ -126,6 +135,7 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) _group = group; _pathDepth = pathDepth; _specLength = specLength; + _matchLength = prefix == null ? 0 : prefix.length(); _prefix = prefix; _suffix = suffix; @@ -213,9 +223,9 @@ public String getPathInfo(String path) return path; case PREFIX_GLOB: - if (path.length() == (_specLength - 2)) + if (path.length() == _matchLength) return null; - return path.substring(_specLength - 2); + return path.substring(_matchLength); default: return null; @@ -241,7 +251,7 @@ public String getPathMatch(String path) case PREFIX_GLOB: if (isWildcardMatch(path)) - return path.substring(0, _specLength - 2); + return path.substring(0, _matchLength); return null; case SUFFIX_GLOB: @@ -278,7 +288,7 @@ public String getSuffix() private boolean isWildcardMatch(String path) { // For a spec of "/foo/*" match "/foo" , "/foo/..." but not "/foobar" - int cpl = _specLength - 2; + int cpl = _matchLength; if ((_group == PathSpecGroup.PREFIX_GLOB) && (path.regionMatches(0, _declaration, 0, cpl))) return (path.length() == cpl) || ('/' == path.charAt(cpl)); return false; @@ -291,19 +301,14 @@ public MatchedPath matched(String path) { case EXACT: if (_declaration.equals(path)) - return MatchedPath.from(path, null); + return _preMatchedPath; break; case PREFIX_GLOB: if (isWildcardMatch(path)) { - String pathMatch = path; - String pathInfo = null; - if (path.length() != (_specLength - 2)) - { - pathMatch = path.substring(0, _specLength - 2); - pathInfo = path.substring(_specLength - 2); - } - return MatchedPath.from(pathMatch, pathInfo); + if (path.length() == _matchLength) + return _preMatchedPath; + return MatchedPath.from(path.substring(0, _matchLength), path.substring(_matchLength)); } break; case SUFFIX_GLOB: @@ -313,7 +318,7 @@ public MatchedPath matched(String path) case ROOT: // Only "/" matches if ("/".equals(path)) - return MatchedPath.from("", path); + return _preMatchedPath; break; case DEFAULT: // If we reached this point, then everything matches From 00a6fcc7623eb7394e95c69b4399d8fa568bc5e5 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 6 Jun 2022 08:43:36 +1000 Subject: [PATCH 16/17] Improved match --- .../java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index d75b97d80e73..8756c7d6c130 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -288,9 +288,8 @@ public String getSuffix() private boolean isWildcardMatch(String path) { // For a spec of "/foo/*" match "/foo" , "/foo/..." but not "/foobar" - int cpl = _matchLength; - if ((_group == PathSpecGroup.PREFIX_GLOB) && (path.regionMatches(0, _declaration, 0, cpl))) - return (path.length() == cpl) || ('/' == path.charAt(cpl)); + if (_group == PathSpecGroup.PREFIX_GLOB && path.length() >= _matchLength && path.regionMatches(0, _declaration, 0, _matchLength)) + return path.length() == _matchLength || path.charAt(_matchLength) == '/'; return false; } From fc3c54e62ddce4b15f49c58a77ddb8760f886288 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 7 Jun 2022 12:58:14 -0500 Subject: [PATCH 17/17] Changes from review Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/AbstractPathSpec.java | 2 +- .../jetty/http/pathmap/MatchedPath.java | 4 ++-- .../jetty/http/pathmap/MatchedResource.java | 6 +++++ .../jetty/http/pathmap/PathMappings.java | 19 +++++++-------- .../eclipse/jetty/http/pathmap/PathSpec.java | 5 ++-- .../jetty/http/pathmap/RegexPathSpec.java | 4 ++-- .../jetty/http/pathmap/ServletPathSpec.java | 16 +++++++------ .../http/pathmap/UriTemplatePathSpec.java | 4 ++-- .../jetty/http/pathmap/PathMappingsTest.java | 2 +- .../jetty/http/pathmap/RegexPathSpecTest.java | 23 +++++++++++++++++++ 10 files changed, 59 insertions(+), 26 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 06f74f1c4474..cb936e26c37f 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 @@ -60,7 +60,7 @@ public final boolean equals(Object obj) @Override public final int hashCode() { - return Objects.hash(this.getClass().getSimpleName() + ":" + getDeclaration()); + return Objects.hash(getGroup().ordinal(), getSpecLength(), getDeclaration(), getClass().getName()); } @Override diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java index 44f7e57e9b0a..8dc0fe7ecfa9 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MatchedPath.java @@ -37,7 +37,7 @@ public String getPathInfo() @Override public String toString() { - return "MatchedPath.EMPTY"; + return MatchedPath.class.getSimpleName() + ".EMPTY"; } }; @@ -60,7 +60,7 @@ public String getPathInfo() @Override public String toString() { - return "MatchedPath.from[pathMatch=" + pathMatch + ", pathInfo=" + pathInfo + "]"; + return MatchedPath.class.getSimpleName() + "[pathMatch=" + pathMatch + ", pathInfo=" + pathInfo + "]"; } }; } 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 7f0bbff7b825..f18ef2d29830 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 @@ -20,6 +20,12 @@ import java.util.Map; +/** + * The match details when using {@link PathMappings#getMatched(String)}, used to minimize return to the PathSpec or PathMappings for subsequent details + * that are now provided by the {@link MatchedPath} instance. + * + * @param the type of resource (IncludeExclude uses boolean, WebSocket uses endpoint/endpoint config, servlet uses ServletHolder, etc) + */ public class MatchedResource { private final E resource; 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 76ffcf8b24b7..5bb507d42f25 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 @@ -392,9 +392,7 @@ public boolean remove(PathSpec pathSpec) { _exactMap.remove(exact); // Recalculate _optimizeExact - _optimizedExact = _mappings.stream() - .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.EXACT) - .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + _optimizedExact = canBeOptimized(PathSpecGroup.EXACT); } break; case PREFIX_GLOB: @@ -403,9 +401,7 @@ public boolean remove(PathSpec pathSpec) { _prefixMap.remove(prefix); // Recalculate _optimizePrefix - _optimizedPrefix = _mappings.stream() - .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.PREFIX_GLOB) - .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + _optimizedPrefix = canBeOptimized(PathSpecGroup.PREFIX_GLOB); } break; case SUFFIX_GLOB: @@ -414,9 +410,7 @@ public boolean remove(PathSpec pathSpec) { _suffixMap.remove(suffix); // Recalculate _optimizeSuffix - _optimizedSuffix = _mappings.stream() - .filter((mapping) -> mapping.getPathSpec().getGroup() == PathSpecGroup.SUFFIX_GLOB) - .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + _optimizedSuffix = canBeOptimized(PathSpecGroup.SUFFIX_GLOB); } break; } @@ -425,6 +419,13 @@ public boolean remove(PathSpec pathSpec) return removed; } + private boolean canBeOptimized(PathSpecGroup suffixGlob) + { + return _mappings.stream() + .filter((mapping) -> mapping.getPathSpec().getGroup() == suffixGlob) + .allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec); + } + @Override public String toString() { 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 0d8afcd4bc79..2bc12f8879d9 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 @@ -18,6 +18,8 @@ package org.eclipse.jetty.http.pathmap; +import java.util.Objects; + /** * A path specification is a URI path template that can be matched against. *

        @@ -27,8 +29,7 @@ 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 + "]"); + Objects.requireNonNull(pathSpecString, "null PathSpec not supported"); if (pathSpecString.length() == 0) return new ServletPathSpec(""); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index 140a544187eb..1f26475e1b50 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -278,7 +278,7 @@ public MatchedPath matched(String path) return null; } - public class RegexMatchedPath implements MatchedPath + private class RegexMatchedPath implements MatchedPath { private final RegexPathSpec pathSpec; private final String path; @@ -341,7 +341,7 @@ public String getPathInfo() @Override public String toString() { - return "RegexMatchedPath[" + + return getClass().getSimpleName() + "[" + "pathSpec=" + pathSpec + ", path=\"" + path + "\"" + ", matcher=" + matcher + diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java index 8756c7d6c130..11d1cae6204f 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java @@ -89,6 +89,7 @@ public ServletPathSpec(String servletPathSpec) PathSpecGroup group; String prefix; String suffix; + MatchedPath preMatchedPath; // prefix based if (servletPathSpec.charAt(0) == '/' && servletPathSpec.endsWith("/*")) @@ -96,7 +97,7 @@ public ServletPathSpec(String servletPathSpec) group = PathSpecGroup.PREFIX_GLOB; prefix = servletPathSpec.substring(0, specLength - 2); suffix = null; - _preMatchedPath = MatchedPath.from(prefix, null); + preMatchedPath = MatchedPath.from(prefix, null); } // suffix based else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) @@ -104,7 +105,7 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) group = PathSpecGroup.SUFFIX_GLOB; prefix = null; suffix = servletPathSpec.substring(2, specLength); - _preMatchedPath = null; + preMatchedPath = null; } else { @@ -116,16 +117,15 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) LOG.warn("Suspicious URL pattern: '{}'; see sections 12.1 and 12.2 of the Servlet specification", servletPathSpec); } - _preMatchedPath = MatchedPath.from(servletPathSpec, null); + preMatchedPath = MatchedPath.from(servletPathSpec, null); } int pathDepth = 0; for (int i = 0; i < specLength; i++) { - int cp = servletPathSpec.codePointAt(i); - if (cp < 128) + char c = servletPathSpec.charAt(i); + if (c < 128) { - char c = (char)cp; if (c == '/') pathDepth++; } @@ -138,10 +138,12 @@ else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1) _matchLength = prefix == null ? 0 : prefix.length(); _prefix = prefix; _suffix = suffix; + _preMatchedPath = preMatchedPath; if (LOG.isDebugEnabled()) { - LOG.debug("Creating ServletPathSpec[{}] (group: {}, prefix: \"{}\", suffix: \"{}\")", + LOG.debug("Creating {}[{}] (group: {}, prefix: \"{}\", suffix: \"{}\")", + getClass().getSimpleName(), _declaration, _group, _prefix, _suffix); } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java index 05aff17e690e..a832f1a1ad49 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java @@ -441,7 +441,7 @@ public String[] getVariables() return _variables; } - public static class UriTemplateMatchedPath implements MatchedPath + private static class UriTemplateMatchedPath implements MatchedPath { private final UriTemplatePathSpec pathSpec; private final String path; @@ -491,7 +491,7 @@ public String getPathInfo() @Override public String toString() { - return "UriTemplateMatchedPath[" + + return getClass().getSimpleName() + "[" + "pathSpec=" + pathSpec + ", path=\"" + path + "\"" + ", matcher=" + matcher + 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 ef9d25a143f9..8b0cae679eec 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 @@ -29,7 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -// @chxxeckstyle-disable-check : AvoidEscapedUnicodeCharactersCheck +// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck public class PathMappingsTest { private void assertMatch(PathMappings pathmap, String path, String expectedValue) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java index 243e66e619a1..c3a4c42e01fb 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java @@ -22,6 +22,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -188,6 +189,28 @@ public void testSuffixSpecMiddleWithGroupings() assertNotMatches(spec, "/middle"); } + @Test + public void testNamedRegexGroup() + { + RegexPathSpec spec = new RegexPathSpec("^(?(.*)/middle/)(?.*)$"); + assertEquals("^(?(.*)/middle/)(?.*)$", spec.getDeclaration(), "Spec.pathSpec"); + assertEquals("^(?(.*)/middle/)(?.*)$", spec.getPattern().pattern(), "Spec.pattern"); + assertEquals(2, spec.getPathDepth(), "Spec.pathDepth"); + assertEquals(PathSpecGroup.SUFFIX_GLOB, spec.getGroup(), "Spec.group"); + + assertMatches(spec, "/a/middle/c.do"); + assertMatches(spec, "/a/b/c/d/middle/e/f"); + assertMatches(spec, "/middle/"); + + assertNotMatches(spec, "/a.do"); + assertNotMatches(spec, "/a/middle"); + assertNotMatches(spec, "/middle"); + + MatchedPath matchedPath = spec.matched("/a/middle/c.do"); + assertThat(matchedPath.getPathMatch(), is("/a/middle/")); + assertThat(matchedPath.getPathInfo(), is("c.do")); + } + @Test public void testEquals() {