This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
The following commit(s) were added to refs/heads/master by this push:
new 4651005 SLING-13355 Support a dedicated error handler for non-HTML
requests (#106)
4651005 is described below
commit 4651005f524ac62f84ea0fbe56f5f2dc4bb923ff
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Fri Nov 13 14:29:05 2026 +0100
SLING-13355 Support a dedicated error handler for non-HTML requests (#106)
* SLING-13355 Support a dedicated error handler for non-HTML requests
Error handlers registered with the service property
sling.error.handler=nonhtml are used for requests whose Accept header
neither contains text/html nor */*. The highest ranked one is used.
A default implementation returning just the status with an empty body
is provided.
Co-authored-by: Copilot <[email protected]>
* SLING-13355 Add configuration to disable the Accept header check for
error handling
Co-authored-by: Copilot <[email protected]>
* Use new constants from API PR
* Fix references to new API constants
* Fix test cases and accept all handling
* SLING-13355 Clarify HTML error response selection
Co-authored-by: Copilot <[email protected]>
* SLING-13355 Retrigger PR checks
Co-authored-by: Copilot <[email protected]>
---------
Co-authored-by: Copilot <[email protected]>
---
pom.xml | 2 +-
.../java/org/apache/sling/engine/impl/Config.java | 6 +
.../sling/engine/impl/DefaultErrorHandler.java | 97 ++++-
.../engine/impl/DefaultNonHtmlErrorHandler.java | 53 +++
.../engine/impl/SlingRequestProcessorImpl.java | 24 ++
.../sling/engine/impl/DefaultErrorHandlerTest.java | 395 +++++++++++++++++++++
.../impl/DefaultNonHtmlErrorHandlerTest.java | 70 ++++
.../engine/impl/SlingRequestProcessorImplTest.java | 159 +++++++++
8 files changed, 803 insertions(+), 3 deletions(-)
diff --git a/pom.xml b/pom.xml
index c0a0e04..1a3e01b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -120,7 +120,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>3.0.0</version>
+ <version>3.0.3-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/engine/impl/Config.java
b/src/main/java/org/apache/sling/engine/impl/Config.java
index 91153d0..3963418 100644
--- a/src/main/java/org/apache/sling/engine/impl/Config.java
+++ b/src/main/java/org/apache/sling/engine/impl/Config.java
@@ -126,4 +126,10 @@ public @interface Config {
"When enabled, the getUserPrincipal method of the request
object might even return a principal "
+ "in the non authenticated case. This is not spec
compliant but might be required for some legacy code.")
boolean disable_spec_compliant_getuserprincipal() default true;
+
+ @AttributeDefinition(
+ name = "Enable Accept Header Check for Error Handling",
+ description =
+ "When enabled, the engine will check the Accept header to
determine which error handler to use.")
+ boolean enable_accept_header_check() default true;
}
diff --git
a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
index 3cf96c4..1e9192f 100644
--- a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
+++ b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
@@ -20,6 +20,10 @@ package org.apache.sling.engine.impl;
import java.io.IOException;
import java.io.PrintWriter;
+import java.util.Comparator;
+import java.util.Map;
+import java.util.Optional;
+import java.util.TreeMap;
import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.http.HttpServletRequest;
@@ -29,11 +33,13 @@ import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.SlingJakartaHttpServletRequest;
import org.apache.sling.api.SlingJakartaHttpServletResponse;
import org.apache.sling.api.request.ResponseUtil;
+import org.apache.sling.api.request.header.JakartaMediaRangeList;
import org.apache.sling.api.servlets.ErrorHandler;
import org.apache.sling.api.servlets.JakartaErrorHandler;
import org.apache.sling.api.wrappers.JakartaToJavaxRequestWrapper;
import org.apache.sling.api.wrappers.JakartaToJavaxResponseWrapper;
import org.osgi.framework.ServiceReference;
+import org.osgi.util.converter.Converters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -48,14 +54,25 @@ public class DefaultErrorHandler implements
JakartaErrorHandler {
/** default log */
private final Logger log = LoggerFactory.getLogger(getClass());
+ private static final String ALL_CONTENT_TYPES = "*/*";
+
/** Use this if not null, and if that fails output a report about that
failure */
private volatile JakartaErrorHandler delegate;
+ /** Use this if not null for non html responses */
+ private volatile JakartaErrorHandler nonHtmlDelegate;
+
private volatile JakartaErrorHandler errorHandler;
private volatile JakartaErrorHandler jakartaErrorHandler;
private volatile ServiceReference<?> errorHandlerRef;
private volatile ServiceReference<?> jakartaErrorHandlerRef;
+ private final Map<ServiceReference<JakartaErrorHandler>,
JakartaErrorHandler> jakartaErrorHandlers =
+ new TreeMap<>(Comparator.reverseOrder());
+
+ /** Flag for html accept header check for error handling */
+ private volatile boolean enableAcceptHeaderCheck = true;
+
@SuppressWarnings("deprecation")
public synchronized void setDelegate(final ServiceReference<?> ref, final
ErrorHandler eh) {
if (eh != null) {
@@ -133,6 +150,30 @@ public class DefaultErrorHandler implements
JakartaErrorHandler {
sendError(originalStatus, originalMessage, null, request, response);
}
+ protected JakartaErrorHandler getErrorHandler(final
SlingJakartaHttpServletRequest request) {
+ final JakartaErrorHandler nonHtml = this.nonHtmlDelegate;
+ if (this.enableAcceptHeaderCheck && nonHtml != null &&
!serveHtmlErrorResponse(request)) {
+ return nonHtml;
+ }
+ return this.delegate;
+ }
+
+ /**
+ * Determines whether the request is classified as HTML for error-handler
selection,
+ * rather than merely whether it accepts HTML.
+ * The Accept header must match {@code text/html}, including through
{@code text/*},
+ * and must not contain {@code *}{@code /*}. HTML need not be the
highest-quality match.
+ * A missing or blank Accept header, or any header containing {@code
*}{@code /*},
+ * is treated as non-HTML, even if it also explicitly accepts {@code
text/html}.
+ *
+ * @param request the request to classify
+ * @return {@code true} if the request is classified as HTML, {@code
false} otherwise
+ */
+ private static boolean serveHtmlErrorResponse(final
SlingJakartaHttpServletRequest request) {
+ final JakartaMediaRangeList acceptList =
JakartaMediaRangeList.fromRequest(request);
+ return !acceptList.matchesAll() &&
acceptList.prefer(JakartaMediaRangeList.CONTENT_TYPE_HTML) != null;
+ }
+
// ---------- ErrorHandler interface (default implementation)
--------------
/**
@@ -153,7 +194,7 @@ public class DefaultErrorHandler implements
JakartaErrorHandler {
final SlingJakartaHttpServletResponse response)
throws IOException {
// If we have a delegate let it handle the error
- final JakartaErrorHandler local = this.delegate;
+ final JakartaErrorHandler local = this.getErrorHandler(request);
if (local != null) {
try {
local.handleError(status, message, request, response);
@@ -198,7 +239,7 @@ public class DefaultErrorHandler implements
JakartaErrorHandler {
throws IOException {
final int status = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
// If we have a delegate let it handle the error
- final JakartaErrorHandler local = this.delegate;
+ final JakartaErrorHandler local = this.getErrorHandler(request);
if (local != null) {
try {
local.handleError(throwable, request, response);
@@ -263,4 +304,56 @@ public class DefaultErrorHandler implements
JakartaErrorHandler {
// close the response (SLING-2724)
pw.close();
}
+
+ private boolean matchesName(final ServiceReference<JakartaErrorHandler>
ref, final String name) {
+ final String[] names = Converters.standardConverter()
+ .convert(ref.getProperty(JakartaErrorHandler.PROPERTY_NAME))
+ .to(String[].class);
+
+ for (final String n : names) {
+ if (name.equals(n)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Set the non html error handler
+ */
+ private void setNonHtmlErrorHandler() {
+ final Optional<Map.Entry<ServiceReference<JakartaErrorHandler>,
JakartaErrorHandler>> first =
+ jakartaErrorHandlers.entrySet().stream()
+ .filter(entry -> matchesName(entry.getKey(),
JakartaErrorHandler.NAME_NONHTML))
+ .findFirst();
+ if (first.isPresent()) {
+ this.nonHtmlDelegate = first.get().getValue();
+ } else {
+ this.nonHtmlDelegate = null;
+ }
+ }
+
+ /**
+ * Add a named delegate for handling non-HTML errors.
+ * @param ref the service reference for the delegate
+ * @param handler the error handler instance for the delegate
+ */
+ public synchronized void addNamedDelegate(
+ final ServiceReference<JakartaErrorHandler> ref, final
JakartaErrorHandler handler) {
+ this.jakartaErrorHandlers.put(ref, handler);
+ this.setNonHtmlErrorHandler();
+ }
+
+ /**
+ * Remove a named delegate for handling non-HTML errors.
+ * @param ref the service reference for the delegate
+ */
+ public synchronized void removeNamedDelegate(final
ServiceReference<JakartaErrorHandler> ref) {
+ this.jakartaErrorHandlers.remove(ref);
+ this.setNonHtmlErrorHandler();
+ }
+
+ public void setEnableAcceptHeaderCheck(final boolean enable) {
+ this.enableAcceptHeaderCheck = enable;
+ }
}
diff --git
a/src/main/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandler.java
b/src/main/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandler.java
new file mode 100644
index 0000000..c0633ca
--- /dev/null
+++ b/src/main/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandler.java
@@ -0,0 +1,53 @@
+/*
+ * 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.sling.engine.impl;
+
+import java.io.IOException;
+
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.api.SlingJakartaHttpServletResponse;
+import org.apache.sling.api.servlets.JakartaErrorHandler;
+import org.osgi.service.component.annotations.Component;
+
+@Component(
+ service = JakartaErrorHandler.class,
+ property = {JakartaErrorHandler.PROPERTY_NAME + "=" +
JakartaErrorHandler.NAME_NONHTML})
+public class DefaultNonHtmlErrorHandler implements JakartaErrorHandler {
+
+ @Override
+ public void handleError(
+ int status,
+ String message,
+ SlingJakartaHttpServletRequest request,
+ SlingJakartaHttpServletResponse response)
+ throws IOException {
+ response.setStatus(status);
+ response.setContentLength(0);
+ // commit the response
+ response.flushBuffer();
+ }
+
+ @Override
+ public void handleError(
+ Throwable throwable, SlingJakartaHttpServletRequest request,
SlingJakartaHttpServletResponse response)
+ throws IOException {
+ this.handleError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null,
request, response);
+ }
+}
diff --git
a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
index 1cd2ac5..0c83dba 100644
--- a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
+++ b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
@@ -152,10 +152,12 @@ public class SlingRequestProcessorImpl implements
SlingRequestProcessor {
this.protectHeadersOnInclude = config.sling_includes_protectheaders();
this.checkContentTypeOnInclude =
config.sling_includes_checkcontenttype();
this.disableCheckCompliantGetUserPrincipal =
config.disable_spec_compliant_getuserprincipal();
+
this.errorHandler.setEnableAcceptHeaderCheck(config.enable_accept_header_check());
}
@Reference(
name = "JakartaErrorHandler",
+ target = "(!(" + JakartaErrorHandler.PROPERTY_NAME + "=*))",
cardinality = ReferenceCardinality.OPTIONAL,
policy = ReferencePolicy.DYNAMIC,
policyOption = ReferencePolicyOption.GREEDY)
@@ -167,6 +169,28 @@ public class SlingRequestProcessorImpl implements
SlingRequestProcessor {
this.errorHandler.setDelegate(ref, (JakartaErrorHandler) null);
}
+ /**
+ * Add named error handlers
+ * @param handler The JakartaErrorHandler instance to add as a named
delegate.
+ * @param ref The ServiceReference associated with the JakartaErrorHandler
instance.
+ */
+ @Reference(
+ target = "(" + JakartaErrorHandler.PROPERTY_NAME + "=*)",
+ cardinality = ReferenceCardinality.MULTIPLE,
+ policy = ReferencePolicy.DYNAMIC)
+ void setNamedJakartaErrorHandler(
+ final JakartaErrorHandler handler, final
ServiceReference<JakartaErrorHandler> ref) {
+ this.errorHandler.addNamedDelegate(ref, handler);
+ }
+
+ /**
+ * Remove named error handlers
+ * @param ref The ServiceReference associated with the JakartaErrorHandler
instance to remove.
+ */
+ void unsetNamedJakartaErrorHandler(final
ServiceReference<JakartaErrorHandler> ref) {
+ this.errorHandler.removeNamedDelegate(ref);
+ }
+
@SuppressWarnings("deprecation")
@Reference(
name = "ErrorHandler",
diff --git
a/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java
b/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java
index 7ba3b55..3d36fc3 100644
--- a/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java
+++ b/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java
@@ -18,20 +18,31 @@
*/
package org.apache.sling.engine.impl;
+import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
+import java.util.Hashtable;
import org.apache.sling.api.SlingJakartaHttpServletRequest;
import org.apache.sling.api.SlingJakartaHttpServletResponse;
import org.apache.sling.api.request.RequestProgressTracker;
+import org.apache.sling.api.servlets.JakartaErrorHandler;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceReference;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
/**
@@ -42,6 +53,9 @@ import static org.mockito.Mockito.when;
*/
public class DefaultErrorHandlerTest {
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+
private DefaultErrorHandler handler;
private SlingJakartaHttpServletRequest request;
private SlingJakartaHttpServletResponse response;
@@ -115,4 +129,385 @@ public class DefaultErrorHandlerTest {
responseBody.flush();
assertTrue(responseBody.toString().isEmpty());
}
+
+ // ---------- named / non-html delegate selection ----------
+
+ @SuppressWarnings("unchecked")
+ private ServiceReference<JakartaErrorHandler> registerNamed(
+ final JakartaErrorHandler eh, final int ranking, final Object
names) {
+ final Hashtable<String, Object> props = new Hashtable<>();
+ props.put(JakartaErrorHandler.PROPERTY_NAME, names);
+ props.put(Constants.SERVICE_RANKING, ranking);
+ return (ServiceReference<JakartaErrorHandler>) context.bundleContext()
+ .registerService(JakartaErrorHandler.class, eh, props)
+ .getReference();
+ }
+
+ private void accept(final String accept) {
+ when(request.getHeader("Accept")).thenReturn(accept);
+ }
+
+ @Test
+ public void testNonHtmlRequestUsesNonHtmlDelegateForStatus() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+
+ handler.handleError(404, "not found", request, response);
+
+ verify(nonHtml).handleError(404, "not found", request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testNonHtmlRequestUsesNonHtmlDelegateForThrowable() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+
+ final Exception cause = new IllegalStateException("boom");
+ handler.handleError(cause, request, response);
+
+ verify(nonHtml).handleError(cause, request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testHtmlRequestWithWildcardUsesNonHtmlDelegate() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+
accept("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
+
+ handler.handleError(500, "error", request, response);
+
+ verify(nonHtml).handleError(500, "error", request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testMissingAcceptHeaderIsTreatedAsNonHtml() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testBlankAcceptHeaderIsTreatedAsNonHtml() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept(" ");
+
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void
testMissingAcceptHeaderWithoutNonHtmlDelegateUsesMainDelegate() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+
+ handler.handleError(404, null, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ }
+
+ @Test
+ public void testWildcardAcceptIsTreatedAsNonHtml() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("*/*");
+
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testWildcardWithLowerQualityIsTreatedAsNonHtml() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json, */*;q=0.1");
+
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void testTextWildcardAcceptIsTreatedAsHtml() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("text/*");
+
+ handler.handleError(404, null, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ verifyNoInteractions(nonHtml);
+ }
+
+ @Test
+ public void testHtmlWithLowerQualityIsStillTreatedAsHtml() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json, text/html;q=0.1");
+
+ handler.handleError(404, null, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ verifyNoInteractions(nonHtml);
+ }
+
+ @Test
+ public void testRequestParametersAreNotEvaluated() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+ // parameter parsing might be the cause of the error being handled
+ when(request.getParameter(any())).thenThrow(new
IllegalStateException("parameter parsing failed"));
+
+ handler.handleError(400, null, request, response);
+
+ verify(nonHtml).handleError(400, null, request, response);
+ verifyNoInteractions(main);
+ verify(request, never()).getParameter(any());
+ }
+
+ @Test
+ public void
testNonHtmlRequestWithoutNonHtmlDelegateFallsBackToMainDelegate() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ }
+
+ @Test
+ public void testNonHtmlRequestWithoutAnyDelegateUsesBuiltInResponse()
throws Exception {
+ accept("application/json");
+
+ handler.handleError(404, "not found", request, response);
+
+ verify(response).setStatus(404);
+ responseBody.flush();
+ assertTrue(responseBody.toString().contains("not found"));
+ }
+
+ @Test
+ public void testNonHtmlDelegateWithoutMainDelegate() throws Exception {
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+
+ accept("application/json");
+ handler.handleError(404, null, request, response);
+ verify(nonHtml).handleError(404, null, request, response);
+
+ // html requests must not be routed to the non-html delegate
+ accept("text/html");
+ handler.handleError(403, null, request, response);
+ verify(nonHtml, never()).handleError(403, null, request, response);
+ verify(response).setStatus(403);
+ }
+
+ @Test
+ public void testNamedDelegateWithOtherNameIsIgnored() throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler other = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(other, 100, "something-else"),
other);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ verifyNoInteractions(other);
+ }
+
+ @Test
+ public void testNamedDelegateWithMultipleNamesIncludingNonHtml() throws
Exception {
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.addNamedDelegate(
+ registerNamed(nonHtml, 0, new String[] {"something-else",
JakartaErrorHandler.NAME_NONHTML}), nonHtml);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ }
+
+ @Test
+ public void testHighestRankedNonHtmlDelegateWinsRegardlessOfBindOrder()
throws Exception {
+ final JakartaErrorHandler low = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler high = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler mid = mock(JakartaErrorHandler.class);
+ handler.addNamedDelegate(registerNamed(low, -10,
JakartaErrorHandler.NAME_NONHTML), low);
+ handler.addNamedDelegate(registerNamed(high, 100,
JakartaErrorHandler.NAME_NONHTML), high);
+ handler.addNamedDelegate(registerNamed(mid, 10,
JakartaErrorHandler.NAME_NONHTML), mid);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+
+ verify(high).handleError(404, null, request, response);
+ verifyNoInteractions(low, mid);
+ }
+
+ @Test
+ public void testOlderNonHtmlDelegateWinsOnEqualRanking() throws Exception {
+ final JakartaErrorHandler first = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler second = mock(JakartaErrorHandler.class);
+ final ServiceReference<JakartaErrorHandler> firstRef =
+ registerNamed(first, 0, JakartaErrorHandler.NAME_NONHTML);
+ final ServiceReference<JakartaErrorHandler> secondRef =
+ registerNamed(second, 0, JakartaErrorHandler.NAME_NONHTML);
+ // bind in reverse registration order
+ handler.addNamedDelegate(secondRef, second);
+ handler.addNamedDelegate(firstRef, first);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+
+ verify(first).handleError(404, null, request, response);
+ verifyNoInteractions(second);
+ }
+
+ @Test
+ public void
testRemovingNonHtmlDelegateFallsBackToNextRankedAndThenToMain() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler low = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler high = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ final ServiceReference<JakartaErrorHandler> lowRef =
registerNamed(low, 0, JakartaErrorHandler.NAME_NONHTML);
+ final ServiceReference<JakartaErrorHandler> highRef =
registerNamed(high, 10, JakartaErrorHandler.NAME_NONHTML);
+ handler.addNamedDelegate(lowRef, low);
+ handler.addNamedDelegate(highRef, high);
+ accept("application/json");
+
+ handler.removeNamedDelegate(highRef);
+ handler.handleError(404, null, request, response);
+ verify(low).handleError(404, null, request, response);
+ verifyNoInteractions(high);
+
+ handler.removeNamedDelegate(lowRef);
+ handler.handleError(410, null, request, response);
+ verify(main).handleError(410, null, request, response);
+ verify(low, never()).handleError(410, null, request, response);
+ }
+
+ @Test
+ public void testRemovingUnknownNamedDelegateIsHarmless() throws Exception {
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler unknown = mock(JakartaErrorHandler.class);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+
+ handler.removeNamedDelegate(registerNamed(unknown, 0,
JakartaErrorHandler.NAME_NONHTML));
+ handler.handleError(404, null, request, response);
+
+ verify(nonHtml).handleError(404, null, request, response);
+ }
+
+ @Test
+ public void testFailingNonHtmlDelegateFallsBackToBuiltInResponse() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ doThrow(new IOException("failed"))
+ .when(nonHtml)
+ .handleError(
+ anyInt(),
+ any(),
+ any(SlingJakartaHttpServletRequest.class),
+ any(SlingJakartaHttpServletResponse.class));
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+
+ handler.handleError(404, "not found", request, response);
+
+ verify(response).reset();
+ verify(response).setStatus(404);
+ verifyNoInteractions(main);
+ responseBody.flush();
+ assertTrue(responseBody.toString().contains("not found"));
+ }
+
+ @Test
+ public void testDisabledAcceptHeaderCheckAlwaysUsesMainDelegate() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ handler.setEnableAcceptHeaderCheck(false);
+ accept("application/json");
+
+ handler.handleError(404, null, request, response);
+ final Exception cause = new IllegalStateException("boom");
+ handler.handleError(cause, request, response);
+
+ verify(main).handleError(404, null, request, response);
+ verify(main).handleError(cause, request, response);
+ verifyNoInteractions(nonHtml);
+ verify(request, never()).getHeader(any());
+ }
+
+ @Test
+ public void
testDisabledAcceptHeaderCheckWithoutMainDelegateUsesBuiltInResponse() throws
Exception {
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ handler.setEnableAcceptHeaderCheck(false);
+ accept("application/json");
+
+ handler.handleError(404, "not found", request, response);
+
+ verifyNoInteractions(nonHtml);
+ verify(response).setStatus(404);
+ responseBody.flush();
+ assertTrue(responseBody.toString().contains("not found"));
+ }
+
+ @Test
+ public void testReEnablingAcceptHeaderCheckUsesNonHtmlDelegateAgain()
throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, main);
+ handler.addNamedDelegate(registerNamed(nonHtml, 0,
JakartaErrorHandler.NAME_NONHTML), nonHtml);
+ accept("application/json");
+
+ handler.setEnableAcceptHeaderCheck(false);
+ handler.handleError(404, null, request, response);
+ verify(main).handleError(404, null, request, response);
+
+ handler.setEnableAcceptHeaderCheck(true);
+ handler.handleError(410, null, request, response);
+ verify(nonHtml).handleError(410, null, request, response);
+ verify(main, never()).handleError(410, null, request, response);
+ }
}
diff --git
a/src/test/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandlerTest.java
b/src/test/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandlerTest.java
new file mode 100644
index 0000000..d659255
--- /dev/null
+++
b/src/test/java/org/apache/sling/engine/impl/DefaultNonHtmlErrorHandlerTest.java
@@ -0,0 +1,70 @@
+/*
+ * 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.sling.engine.impl;
+
+import org.apache.sling.api.SlingJakartaHttpServletRequest;
+import org.apache.sling.api.SlingJakartaHttpServletResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InOrder;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+public class DefaultNonHtmlErrorHandlerTest {
+
+ private DefaultNonHtmlErrorHandler handler;
+ private SlingJakartaHttpServletRequest request;
+ private SlingJakartaHttpServletResponse response;
+
+ @Before
+ public void setup() {
+ handler = new DefaultNonHtmlErrorHandler();
+ request = mock(SlingJakartaHttpServletRequest.class);
+ response = mock(SlingJakartaHttpServletResponse.class);
+ }
+
+ @Test
+ public void testStatusIsSentWithEmptyBody() throws Exception {
+ handler.handleError(404, "secret detail", request, response);
+
+ final InOrder order = inOrder(response);
+ order.verify(response).setStatus(404);
+ order.verify(response).setContentLength(0);
+ order.verify(response).flushBuffer();
+ verify(response, never()).getWriter();
+ verify(response, never()).getOutputStream();
+ verify(response, never()).setContentType(anyString());
+ }
+
+ @Test
+ public void testThrowableIsSentAsInternalServerErrorWithEmptyBody() throws
Exception {
+ handler.handleError(new IllegalStateException("secret detail"),
request, response);
+
+ final InOrder order = inOrder(response);
+ order.verify(response).setStatus(500);
+ order.verify(response).setContentLength(0);
+ order.verify(response).flushBuffer();
+ verify(response, never()).getWriter();
+ verify(response, never()).getOutputStream();
+ }
+}
diff --git
a/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java
b/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java
index 8f38a40..68be674 100644
---
a/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java
+++
b/src/test/java/org/apache/sling/engine/impl/SlingRequestProcessorImplTest.java
@@ -21,6 +21,7 @@ package org.apache.sling.engine.impl;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Field;
+import java.util.Collections;
import jakarta.servlet.Servlet;
import jakarta.servlet.ServletRequest;
@@ -33,22 +34,31 @@ import org.apache.sling.api.request.RequestProgressTracker;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceMetadata;
import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.servlets.JakartaErrorHandler;
import org.apache.sling.api.servlets.ServletResolver;
import org.apache.sling.engine.impl.filter.FilterHandle;
import org.apache.sling.engine.impl.filter.ServletFilterManager;
import
org.apache.sling.engine.impl.filter.ServletFilterManager.FilterChainType;
import org.apache.sling.engine.impl.parameters.SlingParameterParseException;
+import org.apache.sling.testing.mock.osgi.MockOsgi;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.jetbrains.annotations.NotNull;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceRegistration;
import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
/**
@@ -59,6 +69,9 @@ import static org.mockito.Mockito.when;
*/
public class SlingRequestProcessorImplTest {
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+
private SlingRequestProcessorImpl processor;
private ServletFilterManager filterManager;
private SlingJakartaHttpServletRequest request;
@@ -173,4 +186,150 @@ public class SlingRequestProcessorImplTest {
when(resourceMetadata.getResolutionPathInfo()).thenReturn(path);
return resource;
}
+
+ // ---------- error handler service wiring (DS reference targets)
----------
+
+ private static SlingJakartaHttpServletRequest anyRequest() {
+ return any(SlingJakartaHttpServletRequest.class);
+ }
+
+ private static SlingJakartaHttpServletResponse anyResponse() {
+ return any(SlingJakartaHttpServletResponse.class);
+ }
+
+ private SlingRequestProcessorImpl activateProcessorWithOsgi(final
Object... config) {
+ context.registerService(ServletResolver.class,
mock(ServletResolver.class));
+ context.registerService(ServletFilterManager.class, filterManager);
+ context.registerService(RequestProcessorMBeanImpl.class,
mock(RequestProcessorMBeanImpl.class));
+ return
context.registerInjectActivateService(SlingRequestProcessorImpl.class, config);
+ }
+
+ @Test
+ public void testNamedErrorHandlerIsNotBoundAsDefaultErrorHandler() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main,
Constants.SERVICE_RANKING, 0);
+ // higher ranking, but must not replace the default handler as it is a
named one
+ context.registerService(
+ JakartaErrorHandler.class,
+ nonHtml,
+ Constants.SERVICE_RANKING,
+ 1000,
+ JakartaErrorHandler.PROPERTY_NAME,
+ JakartaErrorHandler.NAME_NONHTML);
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi();
+
+ when(request.getHeader("Accept")).thenReturn("text/html");
+ osgiProcessor.handleError(404, "not found", request, response);
+
+ verify(main).handleError(eq(404), eq("not found"), anyRequest(),
anyResponse());
+ verifyNoInteractions(nonHtml);
+ }
+
+ @Test
+ public void testNamedNonHtmlErrorHandlerIsUsedForNonHtmlRequests() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main);
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi();
+
+ // registered after activation: dynamic binding
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ context.registerService(
+ JakartaErrorHandler.class,
+ nonHtml,
+ JakartaErrorHandler.PROPERTY_NAME,
+ JakartaErrorHandler.NAME_NONHTML);
+
+ when(request.getHeader("Accept")).thenReturn("application/json");
+ osgiProcessor.handleError(404, "not found", request, response);
+
+ verify(nonHtml).handleError(eq(404), eq("not found"), anyRequest(),
anyResponse());
+ verifyNoInteractions(main);
+ }
+
+ @Test
+ public void
testUnregisteringNonHtmlErrorHandlerFallsBackToDefaultErrorHandler() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main);
+ final ServiceRegistration<JakartaErrorHandler> reg =
context.bundleContext()
+ .registerService(
+ JakartaErrorHandler.class,
+ nonHtml,
+ new java.util.Hashtable<>(Collections.singletonMap(
+ JakartaErrorHandler.PROPERTY_NAME,
JakartaErrorHandler.NAME_NONHTML)));
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi();
+
+ reg.unregister();
+ when(request.getHeader("Accept")).thenReturn("application/json");
+ osgiProcessor.handleError(404, "not found", request, response);
+
+ verify(main).handleError(eq(404), eq("not found"), anyRequest(),
anyResponse());
+ verifyNoInteractions(nonHtml);
+ }
+
+ @Test
+ public void
testDefaultNonHtmlErrorHandlerComponentIsUsedForNonHtmlRequests() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main);
+ // registered with the component properties from its DS metadata
+
context.registerInjectActivateService(DefaultNonHtmlErrorHandler.class);
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi();
+
+ when(request.getHeader("Accept")).thenReturn("application/json");
+ osgiProcessor.handleError(404, "not found", request, response);
+
+ verify(response).setStatus(404);
+ verify(response).setContentLength(0);
+ verifyNoInteractions(main);
+ responseBody.flush();
+ assertTrue(responseBody.toString().isEmpty());
+ }
+
+ @Test
+ public void
testDisabledAcceptHeaderCheckUsesDefaultErrorHandlerForNonHtmlRequests() throws
Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main);
+ context.registerService(
+ JakartaErrorHandler.class,
+ nonHtml,
+ JakartaErrorHandler.PROPERTY_NAME,
+ JakartaErrorHandler.NAME_NONHTML);
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi("enable.accept.header.check", false);
+
+ when(request.getHeader("Accept")).thenReturn("application/json");
+ osgiProcessor.handleError(404, "not found", request, response);
+
+ verify(main).handleError(eq(404), eq("not found"), anyRequest(),
anyResponse());
+ verifyNoInteractions(nonHtml);
+ }
+
+ @Test
+ public void testAcceptHeaderCheckCanBeToggledByModifyingConfiguration()
throws Exception {
+ final JakartaErrorHandler main = mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler nonHtml = mock(JakartaErrorHandler.class);
+ context.registerService(JakartaErrorHandler.class, main);
+ context.registerService(
+ JakartaErrorHandler.class,
+ nonHtml,
+ JakartaErrorHandler.PROPERTY_NAME,
+ JakartaErrorHandler.NAME_NONHTML);
+ final SlingRequestProcessorImpl osgiProcessor =
activateProcessorWithOsgi();
+ when(request.getHeader("Accept")).thenReturn("application/json");
+
+ // enabled by default
+ osgiProcessor.handleError(404, "not found", request, response);
+ verify(nonHtml).handleError(eq(404), eq("not found"), anyRequest(),
anyResponse());
+
+ MockOsgi.modified(osgiProcessor, context.bundleContext(),
"enable.accept.header.check", false);
+ osgiProcessor.handleError(410, "gone", request, response);
+ verify(main).handleError(eq(410), eq("gone"), anyRequest(),
anyResponse());
+ verify(nonHtml, never()).handleError(eq(410), eq("gone"),
anyRequest(), anyResponse());
+
+ MockOsgi.modified(osgiProcessor, context.bundleContext(),
"enable.accept.header.check", true);
+ osgiProcessor.handleError(403, "forbidden", request, response);
+ verify(nonHtml).handleError(eq(403), eq("forbidden"), anyRequest(),
anyResponse());
+ verify(main, never()).handleError(eq(403), eq("forbidden"),
anyRequest(), anyResponse());
+ }
}
