From 25dd6d0ef7ba22f96a7c39213c5b6be011f603f4 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 8 Jun 2022 12:36:30 -0500 Subject: [PATCH] Cherry-pick of Improvements to PathSpec for Jetty 10.0.x (#8136) * Cherry-pick of Improvements to PathSpec. * From commit: 5b4d1dd1c64482d00919029e0a2ba4ac1f4d8e6b * Fixing ConstraintSecurityHandler usage of PathMappings * Fixing bad INCLUDE logic from cherry-pick in ServletHandler.doScope() * Cleanup of non ServletPathSpec behaviors in ServletPathMapping class * Skip optional group name/info lookup if regex fails. * Prevent NPE on static servletPathMappings * Update WebSocketMappings to use new PathMappings.getMatched(String) Signed-off-by: Joakim Erdfelt --- .../jetty/http/pathmap/PathMappings.java | 17 ++- .../jetty/http/pathmap/RegexPathSpec.java | 119 +++++++++++++-- .../jetty/http/pathmap/RegexPathSpecTest.java | 7 +- .../test/resources/jetty-logging.properties | 1 + .../ee10/servlet/ServletPathMapping.java | 138 +++++++++++------- .../eclipse/jetty/ee9/nested/RequestTest.java | 1 + 6 files changed, 209 insertions(+), 74 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index 8664fbf3896c..f094b5080dd0 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -178,13 +178,14 @@ public MatchedResource getMatched(String path) { MappedResource candidate = _exactMap.getBest(path, 0, i--); if (candidate == null) - continue; + break; matchedPath = candidate.getPathSpec().matched(path); if (matchedPath != null) { return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); } + i--; } // If we reached here, there's NO optimized EXACT Match possible, skip simple match below skipRestOfGroup = true; @@ -201,11 +202,12 @@ public MatchedResource getMatched(String path) { MappedResource candidate = _prefixMap.getBest(path, 0, i--); if (candidate == null) - continue; + break; matchedPath = candidate.getPathSpec().matched(path); if (matchedPath != null) return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath); + i--; } // If we reached here, there's NO optimized PREFIX Match possible, skip simple match below skipRestOfGroup = true; @@ -227,7 +229,7 @@ public MatchedResource getMatched(String path) { MappedResource candidate = _suffixMap.get(path, i + 1, path.length() - i - 1); if (candidate == null) - continue; + break; matchedPath = candidate.getPathSpec().matched(path); if (matchedPath != null) @@ -259,6 +261,15 @@ public Iterator> iterator() return _mappings.iterator(); } + /** + * @deprecated use {@link PathSpec#from(String)} instead + */ + @Deprecated + public static PathSpec asPathSpec(String pathSpecString) + { + return PathSpec.from(pathSpecString); + } + public E get(PathSpec spec) { return _mappings.stream() diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java index f58645e212c6..12a0b90a7df7 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java @@ -125,9 +125,8 @@ public RegexPathSpec(String regex) declaration = regex; int specLength = declaration.length(); // build up a simple signature we can use to identify the grouping - boolean inCharacterClass = false; + boolean inTextList = false; boolean inQuantifier = false; - boolean inCaptureGroup = false; StringBuilder signature = new StringBuilder(); int pathDepth = 0; @@ -140,6 +139,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 @@ -148,32 +149,25 @@ public RegexPathSpec(String regex) case '.': // any char token signature.append('g'); // glob break; - case '(': // in regex capture group - inCaptureGroup = true; - break; - case ')': - inCaptureGroup = false; - signature.append('g'); - break; - case '{': // in regex quantifier + case '{': inQuantifier = true; break; case '}': inQuantifier = false; break; - case '[': // in regex character class - inCharacterClass = true; + case '[': + inTextList = true; break; case ']': - inCharacterClass = false; + inTextList = false; signature.append('g'); // glob break; case '/': - if (!inCharacterClass && !inQuantifier && !inCaptureGroup) + if (!inTextList && !inQuantifier) pathDepth++; break; default: - if (!inCharacterClass && !inQuantifier && !inCaptureGroup && Character.isLetterOrDigit(c)) + if (!inTextList && !inQuantifier && Character.isLetterOrDigit(c)) { if (last == '\\') // escaped { @@ -325,6 +319,101 @@ public MatchedPath matched(String path) return null; } + private class RegexMatchedPath implements MatchedPath + { + private final RegexPathSpec pathSpec; + private final String path; + private final Matcher matcher; + + public RegexMatchedPath(RegexPathSpec regexPathSpec, String path, Matcher matcher) + { + this.pathSpec = regexPathSpec; + this.path = path; + this.matcher = matcher; + } + + @Override + public String getPathMatch() + { + try + { + String p = matcher.group("name"); + if (p != null) + { + return p; + } + } + catch (IllegalArgumentException ignore) + { + // ignore if group name not found. + } + + 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() + { + try + { + String p = matcher.group("info"); + if (p != null) + { + return p; + } + } + catch (IllegalArgumentException ignore) + { + // ignore if group info not found. + } + + // 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; + } + + @Override + public String toString() + { + return getClass().getSimpleName() + "[" + + "pathSpec=" + pathSpec + + ", path=\"" + path + "\"" + + ", matcher=" + matcher + + ']'; + } + + @Override + public MatchedPath matched(String path) + { + Matcher matcher = getMatcher(path); + if (matcher.matches()) + { + return new RegexMatchedPath(this, path, matcher); + } + return null; + } + private class RegexMatchedPath implements MatchedPath { private final RegexPathSpec pathSpec; diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java index 474b909acd2b..878672033be5 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java @@ -28,6 +28,9 @@ 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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; public class RegexPathSpecTest { @@ -201,8 +204,8 @@ public void testSuffixSpecTraditional() assertNotMatches(spec, "/aa/bb"); assertNotMatches(spec, "/aa/bb.do/more"); - assertThat(spec.getPathMatch("/a/b/c.do"), equalTo("")); - assertThat(spec.getPathInfo("/a/b/c.do"), equalTo("/a/b/c.do")); + assertThat(spec.getPathMatch("/a/b/c.do"), equalTo("/a/b/c.do")); + assertThat(spec.getPathInfo("/a/b/c.do"), nullValue()); } /** diff --git a/jetty-ee10/jetty-ee10-maven-plugin/src/test/resources/jetty-logging.properties b/jetty-ee10/jetty-ee10-maven-plugin/src/test/resources/jetty-logging.properties index c1e6a540a536..1ab206d22d5a 100644 --- a/jetty-ee10/jetty-ee10-maven-plugin/src/test/resources/jetty-logging.properties +++ b/jetty-ee10/jetty-ee10-maven-plugin/src/test/resources/jetty-logging.properties @@ -3,3 +3,4 @@ #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG +#org.eclipse.jetty.http.pathmap.LEVEL=DEBUG diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletPathMapping.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletPathMapping.java index 8eb7af4cfeb2..e918ba543845 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletPathMapping.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletPathMapping.java @@ -16,6 +16,7 @@ import jakarta.servlet.http.HttpServletMapping; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.MappingMatch; +import org.eclipse.jetty.http.pathmap.MatchedPath; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.http.pathmap.ServletPathSpec; @@ -38,71 +39,100 @@ public class ServletPathMapping implements HttpServletMapping private final String _servletPath; private final String _pathInfo; - public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext) + public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext, MatchedPath matchedPath) { _servletName = (servletName == null ? "" : servletName); - _pattern = pathSpec == null ? null : pathSpec.getDeclaration(); - if (pathSpec instanceof ServletPathSpec && pathInContext != null) - { - switch (pathSpec.getGroup()) - { - case ROOT: - _mappingMatch = MappingMatch.CONTEXT_ROOT; - _matchValue = ""; - _servletPath = ""; - _pathInfo = "/"; - break; - - case DEFAULT: - _mappingMatch = MappingMatch.DEFAULT; - _matchValue = ""; - _servletPath = pathInContext; - _pathInfo = null; - break; - - case EXACT: - _mappingMatch = MappingMatch.EXACT; - _matchValue = _pattern.startsWith("/") ? _pattern.substring(1) : _pattern; - _servletPath = _pattern; - _pathInfo = null; - break; - - case PREFIX_GLOB: - _mappingMatch = MappingMatch.PATH; - _servletPath = pathSpec.getPrefix(); - // TODO avoid the substring on the known servletPath! - _matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath; - _pathInfo = pathSpec.getPathInfo(pathInContext); - break; - - case SUFFIX_GLOB: - _mappingMatch = MappingMatch.EXTENSION; - int dot = pathInContext.lastIndexOf('.'); - _matchValue = pathInContext.substring(pathInContext.startsWith("/") ? 1 : 0, dot); - _servletPath = pathInContext; - _pathInfo = null; - break; - - case MIDDLE_GLOB: - default: - throw new IllegalStateException(); - } - } - else if (pathSpec != null) + if (pathSpec == null) { + _pattern = null; _mappingMatch = null; - _servletPath = pathSpec.getPathMatch(pathInContext); - _matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath; - _pathInfo = pathSpec.getPathInfo(pathInContext); + _matchValue = ""; + _servletPath = pathInContext; + _pathInfo = null; + return; } - else + + if (pathInContext == null) { + _pattern = pathSpec.getDeclaration(); _mappingMatch = null; _matchValue = ""; - _servletPath = pathInContext; + _servletPath = ""; _pathInfo = null; + return; + } + + // Path Spec types that are not ServletPathSpec + if (!(pathSpec instanceof ServletPathSpec)) + { + _pattern = pathSpec.getDeclaration(); + _mappingMatch = null; + if (matchedPath != null) + { + _servletPath = matchedPath.getPathMatch(); + _pathInfo = matchedPath.getPathInfo(); + } + else + { + _servletPath = pathInContext; + _pathInfo = null; + } + _matchValue = _servletPath.substring(_servletPath.charAt(0) == '/' ? 1 : 0); + return; } + + // from here down is ServletPathSpec behavior + _pattern = pathSpec.getDeclaration(); + + switch (pathSpec.getGroup()) + { + case ROOT: + _mappingMatch = MappingMatch.CONTEXT_ROOT; + _matchValue = ""; + _servletPath = ""; + _pathInfo = "/"; + break; + + case DEFAULT: + _mappingMatch = MappingMatch.DEFAULT; + _matchValue = ""; + _servletPath = pathInContext; + _pathInfo = null; + break; + + case EXACT: + _mappingMatch = MappingMatch.EXACT; + _matchValue = _pattern.startsWith("/") ? _pattern.substring(1) : _pattern; + _servletPath = _pattern; + _pathInfo = null; + break; + + case PREFIX_GLOB: + _mappingMatch = MappingMatch.PATH; + _servletPath = pathSpec.getPrefix(); + // TODO avoid the substring on the known servletPath! + _matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath; + _pathInfo = matchedPath != null ? matchedPath.getPathInfo() : null; + break; + + case SUFFIX_GLOB: + _mappingMatch = MappingMatch.EXTENSION; + int dot = pathInContext.lastIndexOf('.'); + _matchValue = pathInContext.substring(pathInContext.startsWith("/") ? 1 : 0, dot); + _servletPath = pathInContext; + _pathInfo = null; + break; + + case MIDDLE_GLOB: + default: + throw new IllegalStateException("ServletPathSpec of type MIDDLE_GLOB"); + } + } + + public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInContext) + { + this(pathSpec, servletName, pathInContext, null); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java index 0ee58ab9dbd8..6ce12c1518f2 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java @@ -64,6 +64,7 @@ import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.UriCompliance; +import org.eclipse.jetty.http.pathmap.MatchedPath; import org.eclipse.jetty.http.pathmap.RegexPathSpec; import org.eclipse.jetty.http.pathmap.MatchedPath; import org.eclipse.jetty.http.pathmap.RegexPathSpec;