FELIX-6873 : Match ERROR filters against the error servlet path

http/base/src/main/java/org/apache/felix/http/base/internal/dispatch/ServletResponseWrapper.java

@@ -17,6 +17,8 @@
 package org.apache.felix.http.base.internal.dispatch;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.jetbrains.annotations.Nullable;
@@ -58,6 +60,38 @@ public ServletResponseWrapper(@NotNull final HttpServletRequest req,
         this.errorRegistry = errorRegistry;
     }
 
+    /**
+     * Get the paths used to match ERROR filters against the URL patterns.
+     * These are the paths of the error servlet, not of the original request.
+     * Extension patterns have no path to match, an error servlet without
+     * patterns is only matched through its name.
+     * @param patterns The patterns of the error servlet, might be {@code null}
+     * @return The paths, might be empty
+     */
+    static String[] getErrorServletPaths(@Nullable final String[] patterns)
+    {
+        final List<String> paths = new ArrayList<>();
+        if ( patterns != null )
+        {
+            for(final String pattern : patterns)
+            {
+                if ( pattern.isEmpty() || pattern.equals("/") )
+                {
+                    paths.add("/");
+                }
+                else if ( pattern.endsWith("/*") )
+                {
+                    paths.add(pattern.substring(0, pattern.length() - 2));
+                }
+                else if ( !pattern.startsWith("*.") )
+                {
+                    paths.add(pattern);
+                }
+            }
+        }
+        return paths.toArray(new String[0]);
+    }
+
     @Override
     public void sendError(int sc) throws IOException
     {
@@ -115,7 +149,8 @@ public void sendError(final int code, final String message) throws IOException
                                 request.getHttpServletMapping().getMappingMatch(),
                                 false);
 
-                        final FilterHandler[] filterHandlers = errorRegistry.getFilterHandlers(errorResolution, DispatcherType.ERROR, request.getRequestURI());
+                        final FilterHandler[] filterHandlers = errorRegistry.getFilterHandlers(errorResolution, DispatcherType.ERROR,
+                                getErrorServletPaths(errorResolution.getServletInfo().getPatterns()));
 
                         final ServletRequestWrapper reqWrapper = new ServletRequestWrapper(request,
                                 errorResolution.getContext(),

http/base/src/main/java/org/apache/felix/http/base/internal/registry/FilterRegistry.java

@@ -175,6 +175,21 @@ public synchronized void cleanup()
     public @NotNull FilterHandler[] getFilterHandlers(@Nullable final ServletHandler handler,
             @NotNull final DispatcherType dispatcherType,
             @NotNull final String requestURI)
+    {
+        return getFilterHandlers(handler, dispatcherType, new String[] {requestURI});
+    }
+
+    /**
+     * Get all filters handling any of the paths.
+     * Filters are applied to one of the paths and/or the servlet
+     * @param handler Optional servlet handler
+     * @param dispatcherType The dispatcher type
+     * @param requestURIs The paths to match the URL patterns against, might be empty
+     * @return The array of filter handlers, might be empty.
+     */
+    public @NotNull FilterHandler[] getFilterHandlers(@Nullable final ServletHandler handler,
+            @NotNull final DispatcherType dispatcherType,
+            @NotNull final String[] requestURIs)
     {
         final List<FilterHandler> result = new ArrayList<FilterHandler>();
         final List<FilterRegistrationStatus> allFilters = this.filters;
@@ -191,10 +206,17 @@ public synchronized void cleanup()
                 boolean added = false;
                 for(final PathResolver resolver : status.getResolvers())
                 {
-                    if ( resolver.resolve(requestURI) != null )
+                    for(final String requestURI : requestURIs)
+                    {
+                        if ( resolver.resolve(requestURI) != null )
+                        {
+                            result.add(status.getHandler());
+                            added = true;
+                            break;
+                        }
+                    }
+                    if ( added )
                     {
-                        result.add(status.getHandler());
-                        added = true;
                         break;
                     }
                 }

http/base/src/main/java/org/apache/felix/http/base/internal/registry/PerContextHandlerRegistry.java

@@ -173,6 +173,20 @@ public ServletHandler resolveServletByName(final String name)
         return this.filterRegistry.getFilterHandlers(servletHandler, dispatcherType, requestURI);
     }
 
+    /**
+     * Get filter handlers for any of the request uris
+     * @param servletHandler The servlet handler (might be null)
+     * @param dispatcherType The dispatcher type
+     * @param requestURIs The request uris, might be empty
+     * @return The array of filter handlers, the array might be empty.
+     */
+    public @NotNull FilterHandler[] getFilterHandlers(@Nullable final ServletHandler servletHandler,
+            @NotNull final DispatcherType dispatcherType,
+            @NotNull final String[] requestURIs)
+    {
+        return this.filterRegistry.getFilterHandlers(servletHandler, dispatcherType, requestURIs);
+    }
+
     /**
      * Get the servlet handling the error.
      * @param code The error code

http/base/src/test/java/org/apache/felix/http/base/internal/dispatch/ServletResponseWrapperTest.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.felix.http.base.internal.dispatch;
+
+import static org.junit.Assert.assertArrayEquals;
+
+import org.junit.Test;
+
+public class ServletResponseWrapperTest
+{
+    @Test public void testErrorServletPaths()
+    {
+        assertArrayEquals(new String[0], ServletResponseWrapper.getErrorServletPaths(null));
+        assertArrayEquals(new String[0], ServletResponseWrapper.getErrorServletPaths(new String[0]));
+        assertArrayEquals(new String[] {"/errors/page"}, ServletResponseWrapper.getErrorServletPaths(new String[] {"/errors/page"}));
+        assertArrayEquals(new String[] {"/errors"}, ServletResponseWrapper.getErrorServletPaths(new String[] {"/errors/*"}));
+        assertArrayEquals(new String[] {"/", "/"}, ServletResponseWrapper.getErrorServletPaths(new String[] {"", "/"}));
+        assertArrayEquals(new String[] {"/a", "/b"}, ServletResponseWrapper.getErrorServletPaths(new String[] {"/a", "*.html", "/b"}));
+    }
+}

http/base/src/test/java/org/apache/felix/http/base/internal/registry/FilterRegistryTest.java

@@ -123,7 +123,32 @@ private void clear(final ServletContextDTO dto, final FailedDTOHolder holder)
         reg.removeFilter(h5.getFilterInfo(), true);
     }
 
+    @Test public void testErrorFilterMatchesErrorServletPath() throws InvalidSyntaxException
+    {
+        final FilterHandler h1 = createFilterHandler(1L, 0, new String[] {"ERROR"}, "/errors/*");
+        reg.addFilter(h1);
+
+        // the error servlet is at /errors/page, the original request does not matter
+        assertEquals(1, reg.getFilterHandlers(null, DispatcherType.ERROR, new String[] {"/errors/page"}).length);
+        assertEquals(1, reg.getFilterHandlers(null, DispatcherType.ERROR, new String[] {"/other", "/errors/page"}).length);
+        assertEquals(1, reg.getFilterHandlers(null, DispatcherType.ERROR, "/errors/page").length);
+
+        // error servlet outside of the filter pattern, or without any pattern
+        assertEquals(0, reg.getFilterHandlers(null, DispatcherType.ERROR, new String[] {"/other"}).length);
+        assertEquals(0, reg.getFilterHandlers(null, DispatcherType.ERROR, new String[0]).length);
+
+        // dispatcher type is still checked
+        assertEquals(0, reg.getFilterHandlers(null, DispatcherType.REQUEST, new String[] {"/errors/page"}).length);
+
+        reg.removeFilter(h1.getFilterInfo(), true);
+    }
+
     private static FilterInfo createFilterInfo(final long id, final int ranking, final String... paths) throws InvalidSyntaxException
+    {
+        return createFilterInfo(id, ranking, null, paths);
+    }
+
+    private static FilterInfo createFilterInfo(final long id, final int ranking, final String[] dispatchers, final String... paths) throws InvalidSyntaxException
     {
         final BundleContext bCtx = mock(BundleContext.class);
         when(bCtx.createFilter(ArgumentMatchers.anyString())).thenReturn(null);
@@ -136,6 +161,10 @@ private static FilterInfo createFilterInfo(final long id, final int ranking, fin
         when(ref.getProperty(Constants.SERVICE_ID)).thenReturn(id);
         when(ref.getProperty(Constants.SERVICE_RANKING)).thenReturn(ranking);
         when(ref.getProperty(HttpWhiteboardConstants.HTTP_WHITEBOARD_FILTER_PATTERN)).thenReturn(paths);
+        if ( dispatchers != null )
+        {
+            when(ref.getProperty(HttpWhiteboardConstants.HTTP_WHITEBOARD_FILTER_DISPATCHER)).thenReturn(dispatchers);
+        }
         when(ref.getPropertyKeys()).thenReturn(new String[0]);
         final FilterInfo si = new FilterInfo(ref);
 
@@ -144,7 +173,12 @@ private static FilterInfo createFilterInfo(final long id, final int ranking, fin
 
     private static FilterHandler createFilterHandler(final long id, final int ranking, final String... paths) throws InvalidSyntaxException
     {
-        final FilterInfo si = createFilterInfo(id, ranking, paths);
+        return createFilterHandler(id, ranking, null, paths);
+    }
+
+    private static FilterHandler createFilterHandler(final long id, final int ranking, final String[] dispatchers, final String... paths) throws InvalidSyntaxException
+    {
+        final FilterInfo si = createFilterInfo(id, ranking, dispatchers, paths);
         @SuppressWarnings("unchecked")
         final ServiceObjects<Filter> so = mock(ServiceObjects.class);
         final BundleContext ctx = mock(BundleContext.class);