Fix #7891 regex pathInfo (#7892)

Fix 7891 regex pathInfo + Use the pathSpec methods to set servletPath and pathInfo when possible Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · Apr 25, 2022 · efd9f26 · efd9f26
1 parent cef8c9f
commit efd9f26
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 4 deletions.
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ServletPathMapping.java
@@ -90,9 +90,15 @@ public ServletPathMapping(PathSpec pathSpec, String servletName, String pathInCo
                     throw new IllegalStateException();
             }
         }
+        else if (pathSpec != null)
+        {
+            _mappingMatch = null;
+            _servletPath = pathSpec.getPathMatch(pathInContext);
+            _matchValue = _servletPath.startsWith("/") ? _servletPath.substring(1) : _servletPath;
+            _pathInfo = pathSpec.getPathInfo(pathInContext);
+        }
         else
         {
-            // TODO can we do better for RegexPathSpec
             _mappingMatch = null;
             _matchValue = "";
             _servletPath = pathInContext;

diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
@@ -62,6 +62,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.RegexPathSpec;
 import org.eclipse.jetty.http.pathmap.ServletPathSpec;
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
@@ -2043,6 +2044,39 @@ public void testServletPathMapping()
         assertThat(m.getPathInfo(), is(spec.getPathInfo(uri)));
     }
 
+    @Test
+    public void testRegexPathMapping()
+    {
+        RegexPathSpec spec;
+        ServletPathMapping m;
+
+        spec = new RegexPathSpec("^/.*$");
+        m = new ServletPathMapping(spec, "Something", "/some/path");
+        assertThat(m.getMappingMatch(), nullValue());
+        assertThat(m.getPattern(), is(spec.getDeclaration()));
+        assertThat(m.getServletName(), is("Something"));
+        assertThat(m.getServletPath(), is("/some/path"));
+        assertThat(m.getPathInfo(), nullValue());
+        assertThat(m.getMatchValue(), is("some/path"));
+
+        spec = new RegexPathSpec("^/some(/.*)?$");
+        m = new ServletPathMapping(spec, "Something", "/some/path");
+        assertThat(m.getMappingMatch(), nullValue());
+        assertThat(m.getPattern(), is(spec.getDeclaration()));
+        assertThat(m.getServletName(), is("Something"));
+        assertThat(m.getServletPath(), is("/some"));
+        assertThat(m.getPathInfo(), is("/path"));
+        assertThat(m.getMatchValue(), is("some"));
+
+        m = new ServletPathMapping(spec, "Something", "/some");
+        assertThat(m.getMappingMatch(), nullValue());
+        assertThat(m.getPattern(), is(spec.getDeclaration()));
+        assertThat(m.getServletName(), is("Something"));
+        assertThat(m.getServletPath(), is("/some"));
+        assertThat(m.getPathInfo(), nullValue());
+        assertThat(m.getMatchValue(), is("some"));
+    }
+
     private static long getFileCount(Path path)
     {
         try (Stream<Path> s = Files.list(path))

diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RegexServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RegexServletTest.java
@@ -79,7 +79,7 @@ public void testMapping() throws Exception
         assertThat(response, containsString("servletPath='/test/info'"));
         assertThat(response, containsString("pathInfo='null'"));
         assertThat(response, containsString("mapping.mappingMatch='null'"));
-        assertThat(response, containsString("mapping.matchValue=''"));
+        assertThat(response, containsString("mapping.matchValue='test/info'"));
         assertThat(response, containsString("mapping.pattern='^/test/.*$'"));
     }
 
@@ -96,7 +96,7 @@ public void testForward() throws Exception
         assertThat(response, containsString("servletPath='/Test/info'"));
         assertThat(response, containsString("pathInfo='null'"));
         assertThat(response, containsString("mapping.mappingMatch='null'"));
-        assertThat(response, containsString("mapping.matchValue=''"));
+        assertThat(response, containsString("mapping.matchValue='Test/info'"));
         assertThat(response, containsString("mapping.pattern='^/[Tt]est(/.*)?'"));
     }
 
@@ -113,7 +113,7 @@ public void testInclude() throws Exception
         assertThat(response, containsString("servletPath='/include'"));
         assertThat(response, containsString("pathInfo='null'"));
         assertThat(response, containsString("mapping.mappingMatch='null'"));
-        assertThat(response, containsString("mapping.matchValue=''"));
+        assertThat(response, containsString("mapping.matchValue='include'"));
         assertThat(response, containsString("mapping.pattern='^/include$'"));
     }