This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13350 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
commit 21d45e9a23a7b2e4f1a87d5dbec31a4648962dae Author: Joerg Hoh <[email protected]> AuthorDate: Wed Sep 16 10:19:43 2026 +0200 SLING-13350 subsequent call of header-modifying operations must be logged/fail as well --- .../impl/SlingJakartaHttpServletResponseImpl.java | 39 +++++++++++++--- .../engine/impl/SlingRequestProcessorImpl.java | 13 +++--- .../impl/SlingHttpServletResponseImplTest.java | 53 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/apache/sling/engine/impl/SlingJakartaHttpServletResponseImpl.java b/src/main/java/org/apache/sling/engine/impl/SlingJakartaHttpServletResponseImpl.java index 5ed609b..e8a690c 100644 --- a/src/main/java/org/apache/sling/engine/impl/SlingJakartaHttpServletResponseImpl.java +++ b/src/main/java/org/apache/sling/engine/impl/SlingJakartaHttpServletResponseImpl.java @@ -369,15 +369,20 @@ public class SlingJakartaHttpServletResponseImpl extends HttpServletResponseWrap * @return an optional message to log */ protected Optional<String> checkContentTypeOverride(@Nullable String contentType) { - if (requestData.getSlingRequestProcessor().getContentTypeHeaderState() == ContentTypeHeaderState.VIOLATED) { - // return immediatly as the content type header has already been violated - // prevoiously, no more checks needed - return Optional.empty(); - } + // A previously detected violation must not disable + // the check itself - otherwise the second and any later override attempt + // within the same request would pass unchecked even though the first one + // was blocked. + // Return a shorter message in any subsequent case (without the stack) + final boolean isFirstViolation = + requestData.getSlingRequestProcessor().getContentTypeHeaderState() != ContentTypeHeaderState.VIOLATED; String currentContentType = getContentType(); if (contentType == null) { requestData.getSlingRequestProcessor().setContentTypeHeaderState(ContentTypeHeaderState.VIOLATED); - return Optional.of(getMessage(currentContentType, null)); + return Optional.of( + isFirstViolation + ? getMessage(currentContentType, null) + : getShortMessage(currentContentType, null)); } else { Optional<String> currentMime = currentContentType == null ? Optional.of("null") @@ -387,12 +392,32 @@ public class SlingJakartaHttpServletResponseImpl extends HttpServletResponseWrap && setMime.isPresent() && !currentMime.get().equals(setMime.get())) { requestData.getSlingRequestProcessor().setContentTypeHeaderState(ContentTypeHeaderState.VIOLATED); - return Optional.of(getMessage(currentContentType, contentType)); + return Optional.of( + isFirstViolation + ? getMessage(currentContentType, contentType) + : getShortMessage(currentContentType, contentType)); } } return Optional.empty(); } + /** + * Short variant of {@link #getMessage(String, String)} used for repeated + * violations within the same request: it omits the include stack and the + * progress tracker messages which have already been reported with the first + * violation. + * + * @param currentContentType the current 'Content-Type' header + * @param setContentType the 'Content-Type' header that is being set + */ + private String getShortMessage(@Nullable String currentContentType, @Nullable String setContentType) { + return String.format( + "Servlet %s tried to override the 'Content-Type' header from '%s' to '%s'. This is a violation of " + + "the RequestDispatcher.include() contract. See the previously reported violation for the " + + "include stack and the RequestProgressTracker messages.", + requestData.getActiveServletName(), currentContentType, setContentType); + } + private List<String> getLastMessagesOfProgressTracker() { // Collect the last MAX_NR_OF_MESSAGES messages from the RequestProgressTracker // to prevent excessive memory 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 0a9f58e..27451b6 100644 --- a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java +++ b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java @@ -281,12 +281,13 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor { final SlingJakartaHttpServletRequest request = requestData.getSlingRequest(); final SlingJakartaHttpServletResponse response = requestData.getSlingResponse(); + // remember the state of the outer request (if any) - nested processRequest + // calls must restore it when done instead of resetting it to UNSET, + // otherwise a nested call would clear a violation already detected for + // the outer request + final ContentTypeHeaderState outerContentTypeHeaderState = getContentTypeHeaderState(); + try { - if (getContentTypeHeaderState() != ContentTypeHeaderState.UNSET) { - log.debug( - "Content Type Header state has not been cleared properly, is set to {}", - getContentTypeHeaderState()); - } setContentTypeHeaderState(ContentTypeHeaderState.NOT_VIOLATED); // initialize the request data - resolve resource and servlet @@ -361,7 +362,7 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor { localBean.addRequestData(requestData); } - setContentTypeHeaderState(ContentTypeHeaderState.UNSET); + setContentTypeHeaderState(outerContentTypeHeaderState); } } diff --git a/src/test/java/org/apache/sling/engine/impl/SlingHttpServletResponseImplTest.java b/src/test/java/org/apache/sling/engine/impl/SlingHttpServletResponseImplTest.java index fd658f5..51bd2b7 100644 --- a/src/test/java/org/apache/sling/engine/impl/SlingHttpServletResponseImplTest.java +++ b/src/test/java/org/apache/sling/engine/impl/SlingHttpServletResponseImplTest.java @@ -409,6 +409,59 @@ public class SlingHttpServletResponseImplTest { logMessage); } + @Test + public void testContentTypeOverrideStillEnforcedAfterPreviousViolation() { + final SlingJakartaHttpServletResponse orig = Mockito.mock(SlingJakartaHttpServletResponse.class); + final RequestData requestData = mock(RequestData.class); + final DispatchingInfo info = new DispatchingInfo(DispatcherType.INCLUDE); + final RequestProgressTracker requestProgressTracker = mock(RequestProgressTracker.class); + when(requestData.getDispatchingInfo()).thenReturn(info); + when(orig.getContentType()).thenReturn("text/plain"); + when(requestData.getRequestProgressTracker()).thenReturn(requestProgressTracker); + info.setCheckContentTypeOnInclude(true); + + final SlingRequestProcessorImpl requestProcessor = mock(SlingRequestProcessorImpl.class); + // a violation has already been detected earlier within this request + when(requestProcessor.getContentTypeHeaderState()).thenReturn(ContentTypeHeaderState.VIOLATED); + when(requestData.getSlingRequestProcessor()).thenReturn(requestProcessor); + when(requestData.getActiveServletName()).thenReturn(ACTIVE_SERVLET_NAME); + + final HttpServletResponse include = new SlingJakartaHttpServletResponseImpl(requestData, orig); + + Throwable throwable = null; + try { + include.setContentType("text/html"); + } catch (RuntimeException e) { + throwable = e; + } + assertNotNull("Expected the repeated override attempt to still be blocked.", throwable); + Mockito.verify(orig, never()).setContentType("text/html"); + } + + @Test + public void testContentTypeOverrideStillIgnoredAfterPreviousViolationWithProtectHeaders() { + final SlingJakartaHttpServletResponse orig = Mockito.mock(SlingJakartaHttpServletResponse.class); + final RequestData requestData = mock(RequestData.class); + final DispatchingInfo info = new DispatchingInfo(DispatcherType.INCLUDE); + final RequestProgressTracker requestProgressTracker = mock(RequestProgressTracker.class); + when(requestData.getDispatchingInfo()).thenReturn(info); + when(orig.getContentType()).thenReturn("text/plain"); + when(requestData.getRequestProgressTracker()).thenReturn(requestProgressTracker); + info.setProtectHeadersOnInclude(true); + + final SlingRequestProcessorImpl requestProcessor = mock(SlingRequestProcessorImpl.class); + // a violation has already been detected earlier within this request + when(requestProcessor.getContentTypeHeaderState()).thenReturn(ContentTypeHeaderState.VIOLATED); + when(requestData.getSlingRequestProcessor()).thenReturn(requestProcessor); + when(requestData.getActiveServletName()).thenReturn(ACTIVE_SERVLET_NAME); + + final HttpServletResponse include = new SlingJakartaHttpServletResponseImpl(requestData, orig); + + include.setContentType("text/html"); + + Mockito.verify(orig, never()).setContentType("text/html"); + } + @Test public void testNoOverrideProtectHeadersContentTypeOverride() { final SlingJakartaHttpServletResponse orig = Mockito.mock(SlingJakartaHttpServletResponse.class);
