Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3447 adding ability to skip certain paths or error handler service #3448

Merged
merged 4 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)

### Added

- #3448 - Adding support for paths that should not use ErrorHandlerService
- #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
Expand Down
2 changes: 1 addition & 1 deletion all/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
2 changes: 1 addition & 1 deletion bundle-cloud/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
2 changes: 1 addition & 1 deletion bundle-onprem/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
2 changes: 1 addition & 1 deletion bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,10 @@ public interface ErrorPageHandlerService {
* @return true if check is enabled else false
*/
boolean isVanityDispatchCheckEnabled();

/**
* @param request The request
* @return True if the service should be used for this service/ False if it should not
*/
public boolean shouldRequestUseErrorPageHandler(SlingHttpServletRequest request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@
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;
Expand Down Expand Up @@ -93,6 +96,8 @@ 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;

Expand Down Expand Up @@ -239,6 +244,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;

Expand Down Expand Up @@ -832,6 +844,8 @@ 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));
Expand Down Expand Up @@ -1036,4 +1050,14 @@ public boolean isVanityDispatchCheckEnabled() {
return this.vanityDispatchCheckEnabled;
}

@Override
public boolean shouldRequestUseErrorPageHandler(SlingHttpServletRequest request) {
if (this.excludedPaths == null) {
return true;
}

long result = Stream.of(this.excludedPaths).filter((item) -> request.getRequestURI().contains(item)).count();
return (result == 0);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
/**
* Dynamic Error Page Handler.
*/
@org.osgi.annotation.versioning.Version("1.3.1")
@org.osgi.annotation.versioning.Version("1.4.0")
package com.adobe.acs.commons.errorpagehandler;
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,24 @@
*/
package com.adobe.acs.commons.errorpagehandler.impl;

import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.NonExistingResource;
import org.apache.sling.api.resource.ResourceResolver;
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.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;

import java.util.HashSet;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

@RunWith(MockitoJUnitRunner.class)
public class ErrorPageHandlerImplTest {
Expand All @@ -48,43 +51,53 @@ public void setup() {
resourceResolver = context.resourceResolver();
request = context.request();
}

/**
* Test {@link ErrorPageHandlerImpl#findErrorPage(org.apache.sling.api.SlingHttpServletRequest, org.apache.sling.api.resource.Resource)}
* Test
* {@link ErrorPageHandlerImpl#findErrorPage(org.apache.sling.api.SlingHttpServletRequest, org.apache.sling.api.resource.Resource)}
* with a page without {@code jcr:content} node.
*/
@Test
public void testFindErrorPage_withoutContent() {
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request, resourceResolver.getResource("/content/project/test/page-without-content")));
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request,
resourceResolver.getResource("/content/project/test/page-without-content")));
}

/**
* Test {@link ErrorPageHandlerImpl#findErrorPage(org.apache.sling.api.SlingHttpServletRequest, org.apache.sling.api.resource.Resource)}
* Test
* {@link ErrorPageHandlerImpl#findErrorPage(org.apache.sling.api.SlingHttpServletRequest, org.apache.sling.api.resource.Resource)}
* with a page with direct configuration.
*/
@Test
public void testFindErrorPage_withDirectConfig() {
assertEquals("/content/project/test/error-pages2.html", new ErrorPageHandlerImpl().findErrorPage(request, resourceResolver.getResource("/content/project/test/page-with-config")));
assertEquals("/content/project/test/error-pages2.html", new ErrorPageHandlerImpl().findErrorPage(request,
resourceResolver.getResource("/content/project/test/page-with-config")));
}

@Test
public void testFindErrorPage_subResource() {
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver, "/content/project/test/jcr:content/root/non-existing-resource")));
assertEquals("/content/project/test/error-pages.html",
new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver,
"/content/project/test/jcr:content/root/non-existing-resource")));
}

@Test
public void testFindErrorPage_nonExistingPage() {
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver, "/content/project/test/non-existing-page")));
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request,
new NonExistingResource(resourceResolver, "/content/project/test/non-existing-page")));
}

@Test
public void testFindErrorPage_nonExistingPageSubResource() {
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver, "/content/project/test/non-existing-page/jcr:content/test1/test2")));
assertEquals("/content/project/test/error-pages.html",
new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver,
"/content/project/test/non-existing-page/jcr:content/test1/test2")));
}

@Test
public void testFindErrorPage_nonExistingPageWithoutExtension() {
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request, new NonExistingResource(resourceResolver, "/content/project/non-existing-page")));
assertEquals("/content/project/test/error-pages.html", new ErrorPageHandlerImpl().findErrorPage(request,
new NonExistingResource(resourceResolver, "/content/project/non-existing-page")));
}

@Test
Expand All @@ -93,20 +106,45 @@ public void testFindErrorPage_JcrContent0() {
new ErrorPageHandlerImpl().findErrorPage(request,
new NonExistingResource(resourceResolver, "/content/project/jcr:content/non-existing")));
}

@Test
public void testResetRequestAndResponse() {
context.response().setStatus(200);

context.request().setAttribute("com.day.cq.widget.HtmlLibraryManager.included", "Some prior clientlibs");
context.request().setAttribute("com.adobe.granite.ui.clientlibs.HtmlLibraryManager.included", "Some prior clientlibs");
context.request().setAttribute("com.adobe.granite.ui.clientlibs.HtmlLibraryManager.included",
"Some prior clientlibs");
context.request().setAttribute("com.day.cq.wcm.componentcontext", "some prior component context");

new ErrorPageHandlerImpl().resetRequestAndResponse(context.request(), context.response(), 500);

assertEquals("true", context.response().getHeader("x-aem-error-pass"));
assertEquals(500, context.response().getStatus());
assertEquals(0, ((HashSet<String>) context.request().getAttribute("com.day.cq.widget.HtmlLibraryManager.included")).size());
assertEquals(0, ((HashSet<String>) context.request().getAttribute("com.adobe.granite.ui.clientlibs.HtmlLibraryManager.included")).size());
assertEquals(0,
((HashSet<String>) context.request().getAttribute("com.day.cq.widget.HtmlLibraryManager.included"))
.size());
assertEquals(0, ((HashSet<String>) context.request()
.getAttribute("com.adobe.granite.ui.clientlibs.HtmlLibraryManager.included")).size());
assertNull(context.request().getAttribute("com.day.cq.wcm.componentcontext"));
}

@Test
public void testPathExclusion_true() {
SlingHttpServletRequest request = Mockito.mock(SlingHttpServletRequest.class);
Mockito.when(request.getRequestURI()).thenReturn("/content");

ErrorPageHandlerImpl errorPageHandlerService = new ErrorPageHandlerImpl();
errorPageHandlerService.excludedPaths = new String[]{"/content/test-page"};
assertTrue(errorPageHandlerService.shouldRequestUseErrorPageHandler(request));
}

@Test
public void testPathExclusion_false() {
SlingHttpServletRequest request = Mockito.mock(SlingHttpServletRequest.class);
Mockito.when(request.getRequestURI()).thenReturn("/content/test-page/test1234");

ErrorPageHandlerImpl errorPageHandlerService = new ErrorPageHandlerImpl();
errorPageHandlerService.excludedPaths = new String[]{"/content/test-page"};
assertFalse(errorPageHandlerService.shouldRequestUseErrorPageHandler(request));
}
}
2 changes: 1 addition & 1 deletion oakpal-checks/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
<packaging>pom</packaging>

<name>ACS AEM Commons - Reactor Project</name>
Expand Down
2 changes: 1 addition & 1 deletion ui.apps/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
%><%@include file="/libs/foundation/global.jsp" %><%
ErrorPageHandlerService errorPageHandlerService = sling.getService(ErrorPageHandlerService.class);

if (errorPageHandlerService != null && errorPageHandlerService.isEnabled()) {
if (errorPageHandlerService != null && errorPageHandlerService.isEnabled() && errorPageHandlerService.shouldRequestUseErrorPageHandler(slingRequest)) {

// Handle ACS AEM Commons vanity logic
if (errorPageHandlerService.isVanityDispatchCheckEnabled()){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

final ErrorPageHandlerService errorPageHandlerService = sling.getService(ErrorPageHandlerService.class);

if (errorPageHandlerService != null && errorPageHandlerService.isEnabled()) {
if (errorPageHandlerService != null && errorPageHandlerService.isEnabled() && && errorPageHandlerService.shouldRequestUseErrorPageHandler(slingRequest)) {
final int status = errorPageHandlerService.getStatusCode(slingRequest);

if (status >= SlingHttpServletResponse.SC_INTERNAL_SERVER_ERROR &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
javax.servlet.http.HttpServletResponse" %><%

final ErrorPageHandlerService errorPageHandlerService = sling.getService(ErrorPageHandlerService.class);
if (errorPageHandlerService == null || !errorPageHandlerService.isEnabled()) {
if (errorPageHandlerService == null || !errorPageHandlerService.isEnabled() || !errorPageHandlerService.shouldRequestUseErrorPageHandler(slingRequest)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion ui.config/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
2 changes: 1 addition & 1 deletion ui.content/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<parent>
<groupId>com.adobe.acs</groupId>
<artifactId>acs-aem-commons</artifactId>
<version>6.7.1-SNAPSHOT</version>
<version>6.8.0-SNAPSHOT</version>
</parent>

<!-- ====================================================================== -->
Expand Down
Loading