Issue #5810 - Support bespoke PathSpec for servlets in embedded-jetty

examples/embedded/src/main/java/org/eclipse/jetty/embedded/SecuredHelloHandler.java

@@ -79,7 +79,7 @@ public static Server createServer(int port) throws FileNotFoundException
         // for this constraint mapping are mined from the Constraint itself
         // although methods exist to declare and bind roles separately as well.
         ConstraintMapping mapping = new ConstraintMapping();
-        mapping.setPathSpec("/*");
+        mapping.setServletPathSpec("/*");
         mapping.setConstraint(constraint);
 
         // First you see the constraint mapping being applied to the handler as

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/ServletSecurityAnnotationHandler.java

@@ -26,6 +26,7 @@
 import javax.servlet.annotation.ServletSecurity.TransportGuarantee;
 
 import org.eclipse.jetty.annotations.AnnotationIntrospector.AbstractIntrospectableAnnotationHandler;
+import org.eclipse.jetty.http.pathmap.PathSpec;
 import org.eclipse.jetty.security.ConstraintAware;
 import org.eclipse.jetty.security.ConstraintMapping;
 import org.eclipse.jetty.security.ConstraintSecurityHandler;
@@ -99,10 +100,10 @@ public void doHandle(Class clazz)
         ServletSecurityElement securityElement = new ServletSecurityElement(servletSecurity);
         for (ServletMapping sm : servletMappings)
         {
-            for (String url : sm.getPathSpecs())
+            for (PathSpec pathSpec : sm.toPathSpecs())
             {
-                _context.getMetaData().setOrigin("constraint.url." + url, servletSecurity, clazz);
-                constraintMappings.addAll(ConstraintSecurityHandler.createConstraintsWithMappingsForPath(clazz.getName(), url, securityElement));
+                _context.getMetaData().setOrigin("constraint.url." + pathSpec.getDeclaration(), servletSecurity, clazz);
+                constraintMappings.addAll(ConstraintSecurityHandler.createConstraintsWithMappingsForPath(clazz.getName(), pathSpec, securityElement));
             }
         }
 
@@ -163,33 +164,25 @@ protected List<ServletMapping> getServletMappings(String className)
      */
     protected boolean constraintsExist(List<ServletMapping> servletMappings, List<ConstraintMapping> constraintMappings)
     {
-        boolean exists = false;
+        if (servletMappings == null || servletMappings.isEmpty() || constraintMappings == null || constraintMappings.isEmpty())
+            return false;
 
         //Check to see if the path spec on each constraint mapping matches a pathSpec in the servlet mappings.
         //If it does, then we should ignore the security annotations.
         for (ServletMapping mapping : servletMappings)
         {
-            //Get its url mappings
-            String[] pathSpecs = mapping.getPathSpecs();
-            if (pathSpecs == null)
-                continue;
-
             //Check through the constraints to see if there are any whose pathSpecs (url mappings)
             //match the servlet. If so, then we already have constraints defined for this servlet,
             //and we will not be processing the annotation (ie web.xml or programmatic override).
-            for (int i = 0; constraintMappings != null && i < constraintMappings.size() && !exists; i++)
+            for (ConstraintMapping constraint : constraintMappings)
             {
-                for (int j = 0; j < pathSpecs.length; j++)
+                if (mapping.containsPathSpec(constraint.toPathSpec()))
                 {
                     //TODO decide if we need to check the origin
-                    if (pathSpecs[j].equals(constraintMappings.get(i).getPathSpec()))
-                    {
-                        exists = true;
-                        break;
-                    }
+                    return true;
                 }
             }
         }
-        return exists;
+        return false;
     }
 }

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebFilterAnnotation.java

@@ -85,6 +85,7 @@ public void apply()
             return;
         }
 
+        Source source = new Source(Source.Origin.ANNOTATION, clazz.getName());
         String name = (filterAnnotation.filterName().equals("") ? clazz.getName() : filterAnnotation.filterName());
         String[] urlPatterns = filterAnnotation.value();
         if (urlPatterns.length == 0)
@@ -109,7 +110,7 @@ public void apply()
                 metaData.setOrigin(name + ".filter.init-param." + ip.name(), ip, clazz);
             }
 
-            FilterMapping mapping = new FilterMapping();
+            FilterMapping mapping = new FilterMapping(source);
             mapping.setFilterName(holder.getName());
 
             if (urlPatterns.length > 0)
@@ -119,7 +120,7 @@ public void apply()
                 {
                     paths.add(ServletPathSpec.normalize(s));
                 }
-                mapping.setPathSpecs(paths.toArray(new String[paths.size()]));
+                mapping.setServletPathSpecs(paths.toArray(new String[paths.size()]));
             }
 
             if (filterAnnotation.servletNames().length > 0)
@@ -180,7 +181,7 @@ public void apply()
             //from the annotation
             if (!mappingExists)
             {
-                FilterMapping mapping = new FilterMapping();
+                FilterMapping mapping = new FilterMapping(source);
                 mapping.setFilterName(holder.getName());
 
                 if (urlPatterns.length > 0)
@@ -190,7 +191,7 @@ public void apply()
                     {
                         paths.add(ServletPathSpec.normalize(s));
                     }
-                    mapping.setPathSpecs(paths.toArray(new String[paths.size()]));
+                    mapping.setServletPathSpecs(paths.toArray(new String[paths.size()]));
                 }
                 if (filterAnnotation.servletNames().length > 0)
                 {

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebServletAnnotation.java

@@ -26,6 +26,7 @@
 import javax.servlet.annotation.WebServlet;
 import javax.servlet.http.HttpServlet;
 
+import org.eclipse.jetty.http.pathmap.PathSpec;
 import org.eclipse.jetty.http.pathmap.ServletPathSpec;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.servlet.ServletMapping;
@@ -156,7 +157,7 @@ public void apply()
 
             mapping = new ServletMapping(source);
             mapping.setServletName(holder.getName());
-            mapping.setPathSpecs(LazyList.toStringArray(urlPatternList));
+            mapping.setServletPathSpecs(LazyList.toStringArray(urlPatternList));
         }
         else
         {
@@ -192,7 +193,7 @@ public void apply()
             {
                 mapping = new ServletMapping(new Source(Source.Origin.ANNOTATION, clazz.getName()));
                 mapping.setServletName(servletName);
-                mapping.setPathSpecs(LazyList.toStringArray(urlPatternList));
+                mapping.setServletPathSpecs(LazyList.toStringArray(urlPatternList));
             }
         }
 
@@ -213,10 +214,11 @@ public void apply()
             //  guard against duplicate path mapping here: that is the job of the ServletHandler
             for (String p : urlPatternList)
             {
+                PathSpec pathSpec = new ServletPathSpec(p);
                 ServletMapping existingMapping = _context.getServletHandler().getServletMapping(p);
                 if (existingMapping != null && existingMapping.isDefault())
                 {
-                    String[] updatedPaths = ArrayUtil.removeFromArray(existingMapping.getPathSpecs(), p);
+                    PathSpec[] updatedPaths = ArrayUtil.removeFromArray(existingMapping.toPathSpecs(), pathSpec);
                     //if we removed the last path from a servletmapping, delete the servletmapping
                     if (updatedPaths == null || updatedPaths.length == 0)
                     {
@@ -234,7 +236,7 @@ public void apply()
                 _context.getMetaData().setOrigin(servletName + ".servlet.mapping." + p, annotation, clazz);
             }
             allMappings.add(mapping);
-            _context.getServletHandler().setServletMappings(allMappings.toArray(new ServletMapping[allMappings.size()]));
+            _context.getServletHandler().setServletMappings(allMappings.toArray(new ServletMapping[0]));
         }
     }
 

jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestSecurityAnnotationConversions.java

@@ -107,11 +107,11 @@ public void testDenyAllOnClass() throws Exception
 
         expectedMappings[0] = new ConstraintMapping();
         expectedMappings[0].setConstraint(expectedConstraint);
-        expectedMappings[0].setPathSpec("/foo/*");
+        expectedMappings[0].setServletPathSpec("/foo/*");
 
         expectedMappings[1] = new ConstraintMapping();
         expectedMappings[1].setConstraint(expectedConstraint);
-        expectedMappings[1].setPathSpec("*.foo");
+        expectedMappings[1].setServletPathSpec("*.foo");
 
         introspector.introspect(DenyServlet.class);
 
@@ -164,11 +164,11 @@ public void testRolesAllowedWithTransportGuarantee() throws Exception
         ConstraintMapping[] expectedMappings = new ConstraintMapping[2];
         expectedMappings[0] = new ConstraintMapping();
         expectedMappings[0].setConstraint(expectedConstraint);
-        expectedMappings[0].setPathSpec("/foo/*");
+        expectedMappings[0].setServletPathSpec("/foo/*");
 
         expectedMappings[1] = new ConstraintMapping();
         expectedMappings[1].setConstraint(expectedConstraint);
-        expectedMappings[1].setPathSpec("*.foo");
+        expectedMappings[1].setServletPathSpec("*.foo");
 
         introspector.introspect(RolesServlet.class);
         compareResults(expectedMappings, ((ConstraintAware)wac.getSecurityHandler()).getConstraintMappings());
@@ -200,20 +200,20 @@ public void testMethodAnnotation() throws Exception
         ConstraintMapping[] expectedMappings = new ConstraintMapping[4];
         expectedMappings[0] = new ConstraintMapping();
         expectedMappings[0].setConstraint(expectedConstraint1);
-        expectedMappings[0].setPathSpec("/foo/*");
+        expectedMappings[0].setServletPathSpec("/foo/*");
         expectedMappings[0].setMethodOmissions(new String[]{"GET"});
         expectedMappings[1] = new ConstraintMapping();
         expectedMappings[1].setConstraint(expectedConstraint1);
-        expectedMappings[1].setPathSpec("*.foo");
+        expectedMappings[1].setServletPathSpec("*.foo");
         expectedMappings[1].setMethodOmissions(new String[]{"GET"});
 
         expectedMappings[2] = new ConstraintMapping();
         expectedMappings[2].setConstraint(expectedConstraint2);
-        expectedMappings[2].setPathSpec("/foo/*");
+        expectedMappings[2].setServletPathSpec("/foo/*");
         expectedMappings[2].setMethod("GET");
         expectedMappings[3] = new ConstraintMapping();
         expectedMappings[3].setConstraint(expectedConstraint2);
-        expectedMappings[3].setPathSpec("*.foo");
+        expectedMappings[3].setServletPathSpec("*.foo");
         expectedMappings[3].setMethod("GET");
 
         AnnotationIntrospector introspector = new AnnotationIntrospector();
@@ -252,20 +252,20 @@ public void testMethodAnnotation2() throws Exception
         ConstraintMapping[] expectedMappings = new ConstraintMapping[4];
         expectedMappings[0] = new ConstraintMapping();
         expectedMappings[0].setConstraint(expectedConstraint1);
-        expectedMappings[0].setPathSpec("/foo/*");
+        expectedMappings[0].setServletPathSpec("/foo/*");
         expectedMappings[0].setMethodOmissions(new String[]{"GET"});
         expectedMappings[1] = new ConstraintMapping();
         expectedMappings[1].setConstraint(expectedConstraint1);
-        expectedMappings[1].setPathSpec("*.foo");
+        expectedMappings[1].setServletPathSpec("*.foo");
         expectedMappings[1].setMethodOmissions(new String[]{"GET"});
 
         expectedMappings[2] = new ConstraintMapping();
         expectedMappings[2].setConstraint(expectedConstraint2);
-        expectedMappings[2].setPathSpec("/foo/*");
+        expectedMappings[2].setServletPathSpec("/foo/*");
         expectedMappings[2].setMethod("GET");
         expectedMappings[3] = new ConstraintMapping();
         expectedMappings[3].setConstraint(expectedConstraint2);
-        expectedMappings[3].setPathSpec("*.foo");
+        expectedMappings[3].setServletPathSpec("*.foo");
         expectedMappings[3].setMethod("GET");
 
         introspector.introspect(Method2Servlet.class);
@@ -285,7 +285,7 @@ private void compareResults(ConstraintMapping[] expectedMappings, List<Constrain
             for (int i = 0; i < expectedMappings.length && !matched; i++)
             {
                 ConstraintMapping em = expectedMappings[i];
-                if (em.getPathSpec().equals(am.getPathSpec()))
+                if (em.toPathSpec().equals(am.toPathSpec()))
                 {
                     if ((em.getMethod() == null && am.getMethod() == null) || em.getMethod() != null && em.getMethod().equals(am.getMethod()))
                     {
@@ -315,7 +315,7 @@ private void compareResults(ConstraintMapping[] expectedMappings, List<Constrain
             }
 
             if (!matched)
-                fail("No expected ConstraintMapping matching method:" + am.getMethod() + " pathSpec: " + am.getPathSpec());
+                fail("No expected ConstraintMapping matching method:" + am.getMethod() + " pathSpec: " + am.toPathSpec());
         }
     }
 
@@ -334,7 +334,7 @@ private WebAppContext makeWebAppContext(String className, String servletName, St
         ServletMapping[] servletMappings = new ServletMapping[1];
         servletMappings[0] = new ServletMapping();
 
-        servletMappings[0].setPathSpecs(paths);
+        servletMappings[0].setServletPathSpecs(paths);
         servletMappings[0].setServletName(servletName);
         wac.getServletHandler().setServletMappings(servletMappings);
         return wac;

jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestServletAnnotations.java

@@ -97,7 +97,7 @@ public void testServletAnnotation() throws Exception
         ServletMapping[] mappings = wac.getServletHandler().getServletMappings();
         assertNotNull(mappings);
         assertEquals(1, mappings.length);
-        String[] paths = mappings[0].getPathSpecs();
+        String[] paths = mappings[0].getServletPathSpecs();
         assertNotNull(paths);
         assertEquals(2, paths.length);
     }
@@ -128,9 +128,9 @@ public void testWebServletAnnotationOverrideDefault() throws Exception
         ServletMapping[] resultMappings = wac.getServletHandler().getServletMappings();
         assertNotNull(resultMappings);
         assertEquals(1, resultMappings.length);
-        assertEquals(2, resultMappings[0].getPathSpecs().length);
+        assertEquals(2, resultMappings[0].getServletPathSpecs().length);
         resultMappings[0].getServletName().equals("DServlet");
-        for (String s : resultMappings[0].getPathSpecs())
+        for (String s : resultMappings[0].getServletPathSpecs())
         {
             assertThat(s, anyOf(is("/"), is("/bah/*")));
         }
@@ -170,13 +170,13 @@ public void testWebServletAnnotationReplaceDefault() throws Exception
         {
             if (r.getServletName().equals("default"))
             {
-                assertEquals(1, r.getPathSpecs().length);
-                assertEquals("/other", r.getPathSpecs()[0]);
+                assertEquals(1, r.getServletPathSpecs().length);
+                assertEquals("/other", r.getServletPathSpecs()[0]);
             }
             else if (r.getServletName().equals("DServlet"))
             {
-                assertEquals(2, r.getPathSpecs().length);
-                for (String p : r.getPathSpecs())
+                assertEquals(2, r.getServletPathSpecs().length);
+                for (String p : r.getServletPathSpecs())
                 {
                     if (!p.equals("/") && !p.equals("/bah/*"))
                         fail("Unexpected path");
@@ -211,11 +211,11 @@ public void testWebServletAnnotationNotOverride() throws Exception
         {
             if (r.getServletName().equals("DServlet"))
             {
-                assertEquals(2, r.getPathSpecs().length);
+                assertEquals(2, r.getServletPathSpecs().length);
             }
             else if (r.getServletName().equals("foo"))
             {
-                assertEquals(1, r.getPathSpecs().length);
+                assertEquals(1, r.getServletPathSpecs().length);
             }
             else
                 fail("Unexpected servlet name: " + r);
@@ -252,8 +252,8 @@ public void testWebServletAnnotationIgnore() throws Exception
 
         for (ServletMapping r : resultMappings)
         {
-            assertEquals(1, r.getPathSpecs().length);
-            if (!r.getPathSpecs()[0].equals("/default") && !r.getPathSpecs()[0].equals("/other"))
+            assertEquals(1, r.getServletPathSpecs().length);
+            if (!r.getServletPathSpecs()[0].equals("/default") && !r.getServletPathSpecs()[0].equals("/other"))
                 fail("Unexpected path in mapping: " + r);
         }
     }
@@ -273,8 +273,8 @@ public void testWebServletAnnotationNoMappings() throws Exception
 
         ServletMapping[] resultMappings = wac.getServletHandler().getServletMappings();
         assertEquals(1, resultMappings.length);
-        assertEquals(2, resultMappings[0].getPathSpecs().length);
-        for (String s : resultMappings[0].getPathSpecs())
+        assertEquals(2, resultMappings[0].getServletPathSpecs().length);
+        for (String s : resultMappings[0].getServletPathSpecs())
         {
             assertThat(s, anyOf(is("/"), is("/bah/*")));
         }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java

@@ -112,7 +112,7 @@ private void start(Scenario scenario, Authenticator authenticator, Handler handl
         constraint.setAuthenticate(true);
         constraint.setRoles(new String[]{"**"}); //allow any authenticated user
         ConstraintMapping mapping = new ConstraintMapping();
-        mapping.setPathSpec("/secure");
+        mapping.setServletPathSpec("/secure");
         mapping.setConstraint(constraint);
 
         securityHandler.addConstraintMapping(mapping);

jetty-client/src/test/java/org/eclipse/jetty/client/util/SPNEGOAuthenticationTest.java

@@ -149,7 +149,7 @@ private void startSPNEGO(Scenario scenario, Handler handler) throws Exception
         constraint.setAuthenticate(true);
         constraint.setRoles(new String[]{"**"}); //allow any authenticated user
         ConstraintMapping mapping = new ConstraintMapping();
-        mapping.setPathSpec("/secure");
+        mapping.setServletPathSpec("/secure");
         mapping.setConstraint(constraint);
         securityHandler.addConstraintMapping(mapping);
         authenticator = new ConfigurableSpnegoAuthenticator();

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java

@@ -22,6 +22,20 @@
 
 public abstract class AbstractPathSpec implements PathSpec
 {
+    private final String _declaration;
+
+    protected AbstractPathSpec(String declaration)
+    {
+        Objects.requireNonNull(declaration);
+        _declaration = declaration;
+    }
+
+    @Override
+    public String getDeclaration()
+    {
+        return _declaration;
+    }
+
     @Override
     public int compareTo(PathSpec other)
     {