This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13370 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
commit cad7911abcb594b8d6a94873c88481e2a71d4a86 Author: Joerg Hoh <[email protected]> AuthorDate: Tue Sep 29 18:10:21 2026 +0200 SLING-13370 improve error handling * 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 | 40 ++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 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..1005116 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..884d9a7 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,6 +28,8 @@ 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; @@ -97,6 +100,9 @@ public class ErrorFilterChainTest { final SlingJakartaHttpServletResponseImpl response = Mockito.mock(SlingJakartaHttpServletResponseImpl.class); RequestData requestData = Mockito.mock(RequestData.class); when(response.getRequestData()).thenReturn(requestData); + final SlingRequestProcessorImpl requestProcessor = Mockito.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); @@ -110,4 +116,38 @@ public class ErrorFilterChainTest { // error handling was performed, in this case null verify(requestData, times(1)).setDispatchingInfo(Mockito.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 = Mockito.mock(JakartaErrorHandler.class); + handler.setDelegate(null, errorHandler); + + final SlingJakartaHttpServletRequest request = Mockito.mock(SlingJakartaHttpServletRequest.class); + final SlingJakartaHttpServletResponseImpl response = Mockito.mock(SlingJakartaHttpServletResponseImpl.class); + final RequestData requestData = Mockito.mock(RequestData.class); + when(response.getRequestData()).thenReturn(requestData); + final SlingRequestProcessorImpl requestProcessor = Mockito.mock(SlingRequestProcessorImpl.class); + final StaticResponseHeader nosniff = Mockito.mock(StaticResponseHeader.class); + when(nosniff.getResponseHeaderName()).thenReturn("X-Content-Type-Options"); + when(nosniff.getResponseHeaderValue()).thenReturn("nosniff"); + final StaticResponseHeader frameOptions = Mockito.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"); + } }
