From 77b47a253d4a42aa0c9df30bb86bef143b843b81 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 3 May 2022 12:07:43 -0500 Subject: [PATCH] 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() {