Fixes #5555 NPE if Filter of named servlet (#5556)

Fixed #5555 NPE if there is a filter with a servlet name mapping, but a request is received for a servlet without a name match.
Added more simple tests for servlet and filter mappings

Author: gregw

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java

@@ -614,10 +614,14 @@ protected FilterChain getFilterChain(Request baseRequest, String pathInContext,
                 for (FilterMapping mapping : _wildFilterNameMappings)
                     chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);
 
-            for (FilterMapping mapping : _filterNameMappings.get(servletHolder.getName()))
+            List<FilterMapping> nameMappings = _filterNameMappings.get(servletHolder.getName());
+            if (nameMappings != null)
             {
-                if (mapping.appliesTo(dispatch))
-                    chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);
+                for (FilterMapping mapping : nameMappings)
+                {
+                    if (mapping.appliesTo(dispatch))
+                        chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain);
+                }
             }
         }
 

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java

@@ -18,18 +18,29 @@
 
 package org.eclipse.jetty.servlet;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
 import javax.servlet.DispatcherType;
+import javax.servlet.Filter;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSessionEvent;
 import javax.servlet.http.HttpSessionListener;
 
 import org.eclipse.jetty.http.pathmap.MappedResource;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.component.Container;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -730,4 +741,111 @@ public void sessionCreated(HttpSessionEvent se)
         assertTrue(removeResults.contains(sh1));
         assertTrue(removeResults.contains(lh1));
     }
+
+    @Test
+    public void testServletMappings() throws Exception
+    {
+        Server server = new Server();
+        ServletHandler handler = new ServletHandler();
+        server.setHandler(handler);
+        for (final String mapping : new String[] {"/", "/foo", "/bar/*", "*.bob"})
+        {
+            handler.addServletWithMapping(new ServletHolder(new HttpServlet()
+            {
+                @Override
+                protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+                {
+                    resp.getOutputStream().println("mapping='" + mapping + "'");
+                }
+            }), mapping);
+        }
+        // add servlet with no mapping
+        handler.addServlet(new ServletHolder(new HttpServlet() {}));
+
+        LocalConnector connector = new LocalConnector(server);
+        server.addConnector(connector);
+
+        server.start();
+
+        assertThat(connector.getResponse("GET /default HTTP/1.0\r\n\r\n"), containsString("mapping='/'"));
+        assertThat(connector.getResponse("GET /foo HTTP/1.0\r\n\r\n"), containsString("mapping='/foo'"));
+        assertThat(connector.getResponse("GET /bar HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
+        assertThat(connector.getResponse("GET /bar/bob HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
+        assertThat(connector.getResponse("GET /bar/foo.bob HTTP/1.0\r\n\r\n"), containsString("mapping='/bar/*'"));
+        assertThat(connector.getResponse("GET /other/foo.bob HTTP/1.0\r\n\r\n"), containsString("mapping='*.bob'"));
+    }
+
+    @Test
+    public void testFilterMappings() throws Exception
+    {
+        Server server = new Server();
+        ServletHandler handler = new ServletHandler();
+        server.setHandler(handler);
+
+        ServletHolder foo = new ServletHolder(new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+            {
+                resp.getOutputStream().println("FOO");
+            }
+        });
+        foo.setName("foo");
+        handler.addServletWithMapping(foo, "/foo/*");
+
+        ServletHolder def = new ServletHolder(new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+            {
+                resp.getOutputStream().println("default");
+            }
+        });
+        def.setName("default");
+        handler.addServletWithMapping(def, "/");
+
+        for (final String mapping : new String[]{"/*", "/foo", "/bar/*", "*.bob"})
+        {
+            handler.addFilterWithMapping(new FilterHolder((TestFilter)(request, response, chain) ->
+            {
+                response.getOutputStream().print("path-" + mapping + "-");
+                chain.doFilter(request, response);
+            }), mapping, EnumSet.of(DispatcherType.REQUEST));
+        }
+
+        FilterHolder fooFilter = new FilterHolder((TestFilter)(request, response, chain) ->
+        {
+            response.getOutputStream().print("name-foo-");
+            chain.doFilter(request, response);
+        });
+        fooFilter.setName("fooFilter");
+        FilterMapping named = new FilterMapping();
+        named.setFilterHolder(fooFilter);
+        named.setServletName("foo");
+        handler.addFilter(fooFilter, named);
+
+        LocalConnector connector = new LocalConnector(server);
+        server.addConnector(connector);
+
+        server.start();
+
+        assertThat(connector.getResponse("GET /default HTTP/1.0\r\n\r\n"), containsString("path-/*-default"));
+        assertThat(connector.getResponse("GET /foo HTTP/1.0\r\n\r\n"), containsString("path-/*-path-/foo-name-foo-FOO"));
+        assertThat(connector.getResponse("GET /foo/bar HTTP/1.0\r\n\r\n"), containsString("path-/*-name-foo-FOO"));
+        assertThat(connector.getResponse("GET /foo/bar.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-name-foo-FOO"));
+        assertThat(connector.getResponse("GET /other.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-default"));
+    }
+
+    private interface TestFilter extends Filter
+    {
+        default void init(FilterConfig filterConfig) throws ServletException
+        {
+        }
+
+        @Override
+        default void destroy()
+        {
+        }
+    }
+
 }