This is an automated email from the ASF dual-hosted git repository.
joerghoh 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 8f60137 SLING-13370 improve error handling (#100)
8f60137 is described below
commit 8f60137148a94a5d79379e8b26a5ab9860be4e93
Author: Jörg Hoh <[email protected]>
AuthorDate: Thu Oct 1 09:36:50 2026 +0200
SLING-13370 improve error handling (#100)
* set statically configured headers also for error pages
* in the error handling the DispatcherType.ERROR is never set
---
.../sling/engine/impl/filter/ErrorFilterChain.java | 36 +++++++++-
.../engine/impl/filter/ErrorFilterChainTest.java | 79 ++++++++++++++++------
2 files changed, 94 insertions(+), 21 deletions(-)
diff --git
a/src/main/java/org/apache/sling/engine/impl/filter/ErrorFilterChain.java
b/src/main/java/org/apache/sling/engine/impl/filter/ErrorFilterChain.java
index e9f5340..9e75109 100644
--- a/src/main/java/org/apache/sling/engine/impl/filter/ErrorFilterChain.java
+++ b/src/main/java/org/apache/sling/engine/impl/filter/ErrorFilterChain.java
@@ -24,10 +24,12 @@ import jakarta.servlet.DispatcherType;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
+import jakarta.servlet.ServletResponseWrapper;
import org.apache.sling.api.SlingJakartaHttpServletRequest;
import org.apache.sling.api.SlingJakartaHttpServletResponse;
import org.apache.sling.api.servlets.JakartaErrorHandler;
import org.apache.sling.engine.impl.SlingJakartaHttpServletResponseImpl;
+import org.apache.sling.engine.impl.StaticResponseHeader;
import org.apache.sling.engine.impl.request.DispatchingInfo;
public class ErrorFilterChain extends AbstractSlingFilterChain {
@@ -124,8 +126,8 @@ public class ErrorFilterChain extends
AbstractSlingFilterChain {
}
// reset the response to clear headers and body
- if (response instanceof SlingJakartaHttpServletResponseImpl) {
- SlingJakartaHttpServletResponseImpl slingResponse =
(SlingJakartaHttpServletResponseImpl) response;
+ final SlingJakartaHttpServletResponseImpl slingResponse =
unwrap(response);
+ if (slingResponse != null) {
/*
* Below section stores the original dispatching info for
later restoration.
* This is necessary to ensure that the dispatching info is
set to ERROR
@@ -140,6 +142,14 @@ public class ErrorFilterChain extends
AbstractSlingFilterChain {
final DispatchingInfo dispatchInfo = new
DispatchingInfo(DispatcherType.ERROR);
slingResponse.getRequestData().setDispatchingInfo(dispatchInfo);
response.reset();
+ // reset() clears any operator-configured static response
headers;
+ // re-apply them so error pages are not served without
these security headers
+ for (final StaticResponseHeader mapping : slingResponse
+ .getRequestData()
+ .getSlingRequestProcessor()
+ .getAdditionalResponseHeaders()) {
+
slingResponse.addHeader(mapping.getResponseHeaderName(),
mapping.getResponseHeaderValue());
+ }
super.doFilter(request, response);
} finally {
slingResponse.getRequestData().setDispatchingInfo(originalInfo);
@@ -153,6 +163,28 @@ public class ErrorFilterChain extends
AbstractSlingFilterChain {
}
}
+ /**
+ * Unwraps the given response, following the chain of
+ * {@link ServletResponseWrapper#getResponse()} calls, to find the
+ * underlying {@link SlingJakartaHttpServletResponseImpl}.
+ *
+ * @return the underlying {@code SlingJakartaHttpServletResponseImpl}, or
+ * {@code null} if none is found in the wrapper chain
+ */
+ private static SlingJakartaHttpServletResponseImpl unwrap(ServletResponse
response) {
+ while (response != null) {
+ if (response instanceof SlingJakartaHttpServletResponseImpl) {
+ return (SlingJakartaHttpServletResponseImpl) response;
+ }
+ if (response instanceof ServletResponseWrapper) {
+ response = ((ServletResponseWrapper) response).getResponse();
+ } else {
+ return null;
+ }
+ }
+ return null;
+ }
+
protected void render(final SlingJakartaHttpServletRequest request, final
SlingJakartaHttpServletResponse response)
throws IOException, ServletException {
if (this.mode == Mode.STATUS) {
diff --git
a/src/test/java/org/apache/sling/engine/impl/filter/ErrorFilterChainTest.java
b/src/test/java/org/apache/sling/engine/impl/filter/ErrorFilterChainTest.java
index 94f94a3..e4d77b9 100644
---
a/src/test/java/org/apache/sling/engine/impl/filter/ErrorFilterChainTest.java
+++
b/src/test/java/org/apache/sling/engine/impl/filter/ErrorFilterChainTest.java
@@ -19,6 +19,7 @@
package org.apache.sling.engine.impl.filter;
import java.io.IOException;
+import java.util.Collections;
import java.util.Objects;
import jakarta.servlet.DispatcherType;
@@ -27,14 +28,17 @@ import org.apache.sling.api.SlingJakartaHttpServletResponse;
import org.apache.sling.api.servlets.JakartaErrorHandler;
import org.apache.sling.engine.impl.DefaultErrorHandler;
import org.apache.sling.engine.impl.SlingJakartaHttpServletResponseImpl;
+import org.apache.sling.engine.impl.SlingRequestProcessorImpl;
+import org.apache.sling.engine.impl.StaticResponseHeader;
import org.apache.sling.engine.impl.request.RequestData;
import org.junit.Test;
-import org.mockito.Mockito;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.argThat;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -49,12 +53,12 @@ public class ErrorFilterChainTest {
@Test
public void testResponseCommitted() throws IOException,
jakarta.servlet.ServletException {
final DefaultErrorHandler handler = new DefaultErrorHandler();
- final JakartaErrorHandler errorHandler =
Mockito.mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler errorHandler =
mock(JakartaErrorHandler.class);
handler.setDelegate(null, errorHandler);
- final SlingJakartaHttpServletRequest request =
Mockito.mock(SlingJakartaHttpServletRequest.class);
- final SlingJakartaHttpServletResponse response =
Mockito.mock(SlingJakartaHttpServletResponse.class);
- Mockito.when(response.isCommitted()).thenReturn(true);
+ final SlingJakartaHttpServletRequest request =
mock(SlingJakartaHttpServletRequest.class);
+ final SlingJakartaHttpServletResponse response =
mock(SlingJakartaHttpServletResponse.class);
+ when(response.isCommitted()).thenReturn(true);
final ErrorFilterChain chain1 = new ErrorFilterChain(new
FilterHandle[0], handler, new Exception());
chain1.doFilter(request, response);
@@ -62,27 +66,27 @@ public class ErrorFilterChainTest {
final ErrorFilterChain chain2 = new ErrorFilterChain(new
FilterHandle[0], handler, 500, "message");
chain2.doFilter(request, response);
- Mockito.verify(errorHandler,
never()).handleError(any(Throwable.class), eq(null), eq(response));
- Mockito.verify(errorHandler, never()).handleError(anyInt(),
anyString(), eq(null), eq(response));
+ verify(errorHandler, never()).handleError(any(Throwable.class),
eq(null), eq(response));
+ verify(errorHandler, never()).handleError(anyInt(), anyString(),
eq(null), eq(response));
}
@Test
public void testResponseNotCommitted() throws IOException,
jakarta.servlet.ServletException {
final DefaultErrorHandler handler = new DefaultErrorHandler();
- final JakartaErrorHandler errorHandler =
Mockito.mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler errorHandler =
mock(JakartaErrorHandler.class);
handler.setDelegate(null, errorHandler);
- final SlingJakartaHttpServletRequest request =
Mockito.mock(SlingJakartaHttpServletRequest.class);
- final SlingJakartaHttpServletResponse response =
Mockito.mock(SlingJakartaHttpServletResponse.class);
- Mockito.when(response.isCommitted()).thenReturn(false);
+ final SlingJakartaHttpServletRequest request =
mock(SlingJakartaHttpServletRequest.class);
+ final SlingJakartaHttpServletResponse response =
mock(SlingJakartaHttpServletResponse.class);
+ when(response.isCommitted()).thenReturn(false);
final ErrorFilterChain chain1 = new ErrorFilterChain(new
FilterHandle[0], handler, new Exception());
chain1.doFilter(request, response);
- Mockito.verify(errorHandler,
times(1)).handleError(any(Throwable.class), eq(request), eq(response));
+ verify(errorHandler, times(1)).handleError(any(Throwable.class),
eq(request), eq(response));
final ErrorFilterChain chain2 = new ErrorFilterChain(new
FilterHandle[0], handler, 500, "message");
chain2.doFilter(request, response);
- Mockito.verify(errorHandler, times(1)).handleError(anyInt(),
anyString(), eq(request), eq(response));
+ verify(errorHandler, times(1)).handleError(anyInt(), anyString(),
eq(request), eq(response));
}
@Test
@@ -90,13 +94,16 @@ public class ErrorFilterChainTest {
// mocks a final method in SlingJakartaHttpServletResponseImpl, needs
// mockito-inline
final DefaultErrorHandler handler = new DefaultErrorHandler();
- final JakartaErrorHandler errorHandler =
Mockito.mock(JakartaErrorHandler.class);
+ final JakartaErrorHandler errorHandler =
mock(JakartaErrorHandler.class);
handler.setDelegate(null, errorHandler);
- final SlingJakartaHttpServletRequest request =
Mockito.mock(SlingJakartaHttpServletRequest.class);
- final SlingJakartaHttpServletResponseImpl response =
Mockito.mock(SlingJakartaHttpServletResponseImpl.class);
- RequestData requestData = Mockito.mock(RequestData.class);
+ final SlingJakartaHttpServletRequest request =
mock(SlingJakartaHttpServletRequest.class);
+ final SlingJakartaHttpServletResponseImpl response =
mock(SlingJakartaHttpServletResponseImpl.class);
+ RequestData requestData = mock(RequestData.class);
when(response.getRequestData()).thenReturn(requestData);
+ final SlingRequestProcessorImpl requestProcessor =
mock(SlingRequestProcessorImpl.class);
+
when(requestProcessor.getAdditionalResponseHeaders()).thenReturn(Collections.emptyList());
+
when(requestData.getSlingRequestProcessor()).thenReturn(requestProcessor);
final ErrorFilterChain chain2 = new ErrorFilterChain(new
FilterHandle[0], handler, 404, "not found");
chain2.doFilter(request, response);
@@ -104,10 +111,44 @@ public class ErrorFilterChainTest {
// ensure that the dispatching info of type ERROR is set on the
request data
verify(requestData, times(1))
- .setDispatchingInfo(Mockito.argThat(info -> info != null &&
info.getType() == DispatcherType.ERROR));
+ .setDispatchingInfo(argThat(info -> info != null &&
info.getType() == DispatcherType.ERROR));
// ensure that the original request dispatcher info that is restored
after the
// error handling was performed, in this case null
- verify(requestData,
times(1)).setDispatchingInfo(Mockito.argThat(Objects::isNull));
+ verify(requestData,
times(1)).setDispatchingInfo(argThat(Objects::isNull));
+ }
+
+ @Test
+ public void testAdditionalResponseHeadersReappliedAfterReset()
+ throws IOException, jakarta.servlet.ServletException {
+ // mocks a final method in SlingJakartaHttpServletResponseImpl, needs
+ // mockito-inline
+ final DefaultErrorHandler handler = new DefaultErrorHandler();
+ final JakartaErrorHandler errorHandler =
mock(JakartaErrorHandler.class);
+ handler.setDelegate(null, errorHandler);
+
+ final SlingJakartaHttpServletRequest request =
mock(SlingJakartaHttpServletRequest.class);
+ final SlingJakartaHttpServletResponseImpl response =
mock(SlingJakartaHttpServletResponseImpl.class);
+ final RequestData requestData = mock(RequestData.class);
+ when(response.getRequestData()).thenReturn(requestData);
+ final SlingRequestProcessorImpl requestProcessor =
mock(SlingRequestProcessorImpl.class);
+ final StaticResponseHeader nosniff = mock(StaticResponseHeader.class);
+
when(nosniff.getResponseHeaderName()).thenReturn("X-Content-Type-Options");
+ when(nosniff.getResponseHeaderValue()).thenReturn("nosniff");
+ final StaticResponseHeader frameOptions =
mock(StaticResponseHeader.class);
+
when(frameOptions.getResponseHeaderName()).thenReturn("X-Frame-Options");
+ when(frameOptions.getResponseHeaderValue()).thenReturn("SAMEORIGIN");
+ when(requestProcessor.getAdditionalResponseHeaders())
+ .thenReturn(java.util.Arrays.asList(nosniff, frameOptions));
+
when(requestData.getSlingRequestProcessor()).thenReturn(requestProcessor);
+
+ final ErrorFilterChain chain = new ErrorFilterChain(new
FilterHandle[0], handler, 404, "not found");
+ chain.doFilter(request, response);
+
+ // response.reset() clears headers set at response-wrapper construction
+ // time; the configured static headers must be re-applied afterwards
+ verify(response, times(1)).reset();
+ verify(response, times(1)).addHeader("X-Content-Type-Options",
"nosniff");
+ verify(response, times(1)).addHeader("X-Frame-Options", "SAMEORIGIN");
}
}
