From 0d1bab0f228500cdda746221a1869e0a90f88006 Mon Sep 17 00:00:00 2001 From: "david g." Date: Thu, 17 Oct 2024 17:33:39 -0400 Subject: [PATCH] =?UTF-8?q?Adjusted=20exclusions=20to=20use=20regex,=20and?= =?UTF-8?q?=20force=20Cloud=20Manager-required=20pa=E2=80=A6=20(#3450)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adjusted exclusions to use regex, and force Cloud Manager-required paths into exclusion list --- CHANGELOG.md | 14 ++-- .../impl/ErrorPageHandlerImpl.java | 51 ++++++++------ .../impl/ErrorPageHandlerImplTest.java | 69 +++++++++++++++---- pom.xml | 2 +- 4 files changed, 96 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93c68e2f15..41dd383f42 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java index 0336e200c8..471d84cd86 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImpl.java @@ -26,15 +26,12 @@ 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; @@ -42,6 +39,7 @@ 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 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 notFoundExclusionPatterns = new ArrayList(); + private ArrayList 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(); + 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; + } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImplTest.java index df7a3332bd..03fc0409fd 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/errorpagehandler/impl/ErrorPageHandlerImplTest.java +++ b/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())); } } \ No newline at end of file diff --git a/pom.xml b/pom.xml index d70afd7788..e68025610c 100644 --- a/pom.xml +++ b/pom.xml @@ -265,7 +265,7 @@ Bundle-DocURL: https://adobe-consulting-services.github.io/acs-aem-commons/ org.apache.sling.caconfig.bnd-plugin 1.0.2 - + org.apache.sling org.apache.sling.providertype.bnd-plugin