Adjusted exclusions to use regex, and force Cloud Manager-required pa… (

#3450)

* Adjusted exclusions to use regex, and force Cloud Manager-required paths into exclusion list

CHANGELOG.md

@@ -12,14 +12,15 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
 
 ### Added
 
-- #3448 - Adding support for paths that should not use ErrorHandlerService
+- #3448 - Adding support for URIs that should not use ErrorHandlerService using regex
 - #3415 - Allow Robots.txt generation to serve different file by requested resource path
 - #3426 - Content Sync: view history of completed jobs
 - #3417 - Configurable recursion in Content Sync
 
 ### Changed
 
-- #3420 - Redirect Map Manager - enable Redirect Map Manager in AEM CS (would require a specific - not public yet - AEM CS release version, TBA)
+- #3420 - Redirect Map Manager - enable Redirect Map Manager in AEM CS (would require a specific - not public yet - AEM
+  CS release version, TBA)
 - #3429 - UI Widgets - add uniq function to embedded lodash library to resolve issue with composite multifield widget
 - #3423 - Redirect Manager - status code is not retaining its value in the dialog after authoring
 - #3417 - Configurable recursion in Content Sync
@@ -33,16 +34,19 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
 ### Fixed
 
 - #3380 - Remove forced red theme from system notification text body
-- #3398 - CreateRedirectConfigurationServlet throws PersistenceException when ancestor node types are different than expected
+- #3398 - CreateRedirectConfigurationServlet throws PersistenceException when ancestor node types are different than
+  expected
 - #3402 - EnsureOakIndexManagerImpl does not pick up changes in EnsureOakIndex configurations.
 - #3357 - Added debugging and null checking to ReferencesModel to prevent NPE
-- #3398 - CreateRedirectConfigurationServlet throws PersistenceException when ancestor node types are different than expected
+- #3398 - CreateRedirectConfigurationServlet throws PersistenceException when ancestor node types are different than
+  expected
 - #3275 - CCVAR: Fixed Same Attribute not updating correctly.
 - #3402 - EnsureOakIndexManagerImpl does not pick up changes in EnsureOakIndex configurations.
 
 ### Changed
 
-- #3403 - Replace deprecated com.day.cq.contentsync.handler.util.RequestResponseFactory by SlingHttpServletRequestBuilder 
+- #3403 - Replace deprecated com.day.cq.contentsync.handler.util.RequestResponseFactory by
+  SlingHttpServletRequestBuilder
 - #3376 - Redirect Manager: refactor code to not require service user
 - #3408 - Reduce usage of Apache Commons Lang 2
 - #3401 - Move SyslogAppender into separate bundle for onprem only. SyslogAppender does not work in Cloud Service.

bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java

@@ -26,22 +26,20 @@
 import java.util.Dictionary;
 import java.util.HashSet;
 import java.util.Hashtable;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Stream;
 
 import javax.management.DynamicMBean;
 import javax.management.NotCompliantMBeanException;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.felix.scr.annotations.Activate;
@@ -96,8 +94,6 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService {
     private static final String REDIRECT_TO_LOGIN = "redirect-to-login";
     private static final String RESPOND_WITH_404 = "respond-with-404";
 
-    public String[] excludedPaths = null;
-
     /* Enable/Disable */
     private static final boolean DEFAULT_ENABLED = true;
 
@@ -107,6 +103,16 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService {
             boolValue = DEFAULT_ENABLED)
     private static final String PROP_ENABLED = "enabled";
 
+    /* Excluded URI patterns */
+    protected ArrayList<Pattern> excludedUriPatterns = new ArrayList<>();
+
+    @Property(
+            label = "Excluded URI patterns from handler",
+            description = "Regex URI patterns that are excluded from the error page handler. [Optional] [Default: None] [Forces: ^/content/test-site(/.*)?, ^/content/dam/test(/.*)?] ",
+            cardinality = Integer.MAX_VALUE)
+    private static final String EXCLUDED_URIS_FROM_HANDLER = "error-page.uri-exclusions";
+
+
     /* Enable/Disable Vanity Dispatch check*/
     private static final boolean DEFAULT_VANITY_DISPATCH_ENABLED = false;
 
@@ -179,7 +185,7 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService {
 
     /* Not Found Path Patterns */
     private static final String[] DEFAULT_NOT_FOUND_EXCLUSION_PATH_PATTERNS = {};
-    private ArrayList<Pattern> notFoundExclusionPatterns = new ArrayList<Pattern>();
+    private ArrayList<Pattern> notFoundExclusionPatterns = new ArrayList<>();
 
     @Property(
             label = "Not Found Exclusions",
@@ -244,19 +250,13 @@ public final class ErrorPageHandlerImpl implements ErrorPageHandlerService {
             value = {"png", "jpeg", "jpg", "gif"})
     private static final String PROP_ERROR_IMAGE_EXTENSIONS = "error-images.extensions";
 
-    @Property(
-        label = "Excluded pages from handler",
-        description = "A list of pages that should not be considered as applicable to this service",
-        cardinality = Integer.MAX_VALUE,
-        value = { "/content/test-site", "/content/dam/test"})
-    private static final String EXCLUDED_PATHS_FROM_HANDLER = "error-page.exclusion-list";
 
     @Reference
     private ResourceResolverFactory resourceResolverFactory;
 
     @Reference
     private Authenticator authenticator;
-    
+
     @Reference
     private VanityURLService vanityUrlService;
 
@@ -844,8 +844,6 @@ private void configure(ComponentContext componentContext) {
         Dictionary<?, ?> config = componentContext.getProperties();
         final String legacyPrefix = "prop.";
 
-        this.excludedPaths = PropertiesUtil.toStringArray(config.get(EXCLUDED_PATHS_FROM_HANDLER));
-
         this.enabled = PropertiesUtil.toBoolean(config.get(PROP_ENABLED),
                 PropertiesUtil.toBoolean(config.get(legacyPrefix + PROP_ENABLED),
                         DEFAULT_ENABLED));
@@ -879,11 +877,17 @@ private void configure(ComponentContext componentContext) {
         String[] tmpNotFoundExclusionPatterns = PropertiesUtil.toStringArray(
                 config.get(PROP_NOT_FOUND_EXCLUSION_PATH_PATTERNS), DEFAULT_NOT_FOUND_EXCLUSION_PATH_PATTERNS);
 
-        this.notFoundExclusionPatterns = new ArrayList<Pattern>();
+        this.notFoundExclusionPatterns = new ArrayList<>();
         for (final String tmpPattern : tmpNotFoundExclusionPatterns) {
             this.notFoundExclusionPatterns.add(Pattern.compile(tmpPattern));
         }
 
+        String[] tmpExcludedUriPatterns = ArrayUtils.addAll(PropertiesUtil.toStringArray(config.get(EXCLUDED_URIS_FROM_HANDLER)),
+                "^/content/test-site(/.*)?", "^/content/dam/test(/.*)?");
+        // The above patterns are the default patterns that are always excluded from the error page handler due to a requirement by AEM Cloud Manager
+        for (final String tmpPattern : tmpExcludedUriPatterns) {
+            this.excludedUriPatterns.add(Pattern.compile(tmpPattern));
+        }
 
         /** Error Page Cache **/
 
@@ -949,6 +953,7 @@ private void configure(ComponentContext componentContext) {
         pw.printf("System Error Page Path: %s", this.systemErrorPagePath).println();
         pw.printf("Error Page Extension: %s", this.errorPageExtension).println();
         pw.printf("Fallback Error Page Name: %s", fallbackErrorName).println();
+        pw.printf("Excluded URI Patterns: %s", Arrays.toString(excludedUriPatterns.toArray())).println();
 
         pw.printf("Resource Not Found - Behavior: %s", this.notFoundBehavior).println();
         pw.printf("Resource Not Found - Exclusion Path Patterns %s", Arrays.toString(tmpNotFoundExclusionPatterns)).println();
@@ -1052,12 +1057,18 @@ public boolean isVanityDispatchCheckEnabled() {
 
     @Override
     public boolean shouldRequestUseErrorPageHandler(SlingHttpServletRequest request) {
-        if (this.excludedPaths == null) {
+        if (CollectionUtils.isEmpty(this.excludedUriPatterns)) {
             return true;
         }
 
-        long result = Stream.of(this.excludedPaths).filter((item) -> request.getRequestURI().contains(item)).count();
-        return (result == 0);
-    }
+        String requestURI = request.getRequestURI();
+
+        for (Pattern excludedPattern : this.excludedUriPatterns) {
+            if (excludedPattern.matcher(requestURI).matches()) {
+                return false;
+            }
+        }
 
+        return true;
+    }
 }

bundle/src/test/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImplTest.java

@@ -17,16 +17,19 @@
  */
 package com.adobe.acs.commons.errorpagehandler.impl;
 
-import org.apache.sling.api.SlingHttpServletRequest;
+import com.adobe.acs.commons.errorpagehandler.ErrorPageHandlerService;
+import com.adobe.acs.commons.wcm.vanity.VanityURLService;
+import com.adobe.acs.commons.wcm.vanity.impl.VanityURLServiceImpl;
 import org.apache.sling.api.resource.NonExistingResource;
 import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.auth.Authenticator;
 import org.apache.sling.testing.mock.sling.junit.SlingContext;
 import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mockito;
+import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.HashSet;
@@ -42,6 +45,9 @@ public class ErrorPageHandlerImplTest {
     @Rule
     public final SlingContext context = new SlingContext();
 
+    @Mock
+    Authenticator authenticator;
+
     private MockSlingHttpServletRequest request;
     private ResourceResolver resourceResolver;
 
@@ -50,6 +56,10 @@ public void setup() {
         context.load().json(getClass().getResourceAsStream("ErrorPageHandlerImplTest.json"), "/content/project");
         resourceResolver = context.resourceResolver();
         request = context.request();
+
+
+        context.registerService(VanityURLService.class, new VanityURLServiceImpl());
+        context.registerService(Authenticator.class, authenticator);
     }
 
     /**
@@ -129,22 +139,53 @@ public void testResetRequestAndResponse() {
     }
 
     @Test
-    public void testPathExclusion_true() {
-        SlingHttpServletRequest request = Mockito.mock(SlingHttpServletRequest.class);
-        Mockito.when(request.getRequestURI()).thenReturn("/content");
+    public void testUriExclusion_defaults_true() {
+        context.registerInjectActivateService(new ErrorPageHandlerImpl());
+        ErrorPageHandlerImpl errorPageHandlerService = (ErrorPageHandlerImpl) context.getService(ErrorPageHandlerService.class);
+
+        context.request().setPathInfo("/content");
+
+        assertTrue(errorPageHandlerService.shouldRequestUseErrorPageHandler(context.request()));
+    }
+
+    @Test
+    public void testUriExclusion_defaults_site_false() {
+        context.registerInjectActivateService(new ErrorPageHandlerImpl());
+        ErrorPageHandlerImpl errorPageHandlerService = (ErrorPageHandlerImpl) context.getService(ErrorPageHandlerService.class);
+
+        context.request().setPathInfo("/content/test-site/home.html");
+
+        assertFalse(errorPageHandlerService.shouldRequestUseErrorPageHandler(context.request()));
+    }
+
+    @Test
+    public void testUriExclusion_defaults_dam_false() {
+        context.registerInjectActivateService(new ErrorPageHandlerImpl(), "error-page.uri-exclusions", new String[]{"^/whatever(/.*)?"});
+        ;
+        ErrorPageHandlerImpl errorPageHandlerService = (ErrorPageHandlerImpl) context.getService(ErrorPageHandlerService.class);
+
+        context.request().setPathInfo("/content/dam/test/file.png");
+
+        assertFalse(errorPageHandlerService.shouldRequestUseErrorPageHandler(context.request()));
+
+    }
+
+    @Test
+    public void testUrihExclusion_true() {
+        context.registerInjectActivateService(new ErrorPageHandlerImpl(), "error-page.uri-exclusions", new String[]{"^/whatever(/.*)?"});
+        ErrorPageHandlerImpl errorPageHandlerService = (ErrorPageHandlerImpl) context.getService(ErrorPageHandlerService.class);
+
+        context.request().setPathInfo("/content");
 
-        ErrorPageHandlerImpl errorPageHandlerService = new ErrorPageHandlerImpl();
-        errorPageHandlerService.excludedPaths = new String[]{"/content/test-page"};
-        assertTrue(errorPageHandlerService.shouldRequestUseErrorPageHandler(request));
+        assertTrue(errorPageHandlerService.shouldRequestUseErrorPageHandler(context.request()));
     }
 
     @Test
-    public void testPathExclusion_false() {
-        SlingHttpServletRequest request = Mockito.mock(SlingHttpServletRequest.class);
-        Mockito.when(request.getRequestURI()).thenReturn("/content/test-page/test1234");
+    public void testUriExclusion_false() {
+        context.registerInjectActivateService(new ErrorPageHandlerImpl(), "error-page.uri-exclusions", new String[]{"^/whatever(/.*)?", "^/content/(.*)/foo/(.*)?"});
+        ErrorPageHandlerImpl errorPageHandlerService = (ErrorPageHandlerImpl) context.getService(ErrorPageHandlerService.class);
 
-        ErrorPageHandlerImpl errorPageHandlerService = new ErrorPageHandlerImpl();
-        errorPageHandlerService.excludedPaths = new String[]{"/content/test-page"};
-        assertFalse(errorPageHandlerService.shouldRequestUseErrorPageHandler(request));
+        context.request().setPathInfo("/content/whatever/foo/bar.html");
+        assertFalse(errorPageHandlerService.shouldRequestUseErrorPageHandler(context.request()));
     }
 }

pom.xml

@@ -265,7 +265,7 @@ Bundle-DocURL: https://adobe-consulting-services.github.io/acs-aem-commons/
                             <artifactId>org.apache.sling.caconfig.bnd-plugin</artifactId>
                             <version>1.0.2</version>
                         </dependency>
-                        <!-- enforce that provider types are not implemented/extended, https://github.com/apache/sling-org-apache-sling-providertype-bnd-plugin --> 
+                        <!-- enforce that provider types are not implemented/extended, https://github.com/apache/sling-org-apache-sling-providertype-bnd-plugin -->
                         <dependency>
                             <groupId>org.apache.sling</groupId>
                             <artifactId>org.apache.sling.providertype.bnd-plugin</artifactId>