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 f550921 SLING-13350 subsequent call of header-modifying operations
must be logged/fail as well (#85)
f550921 is described below
commit f550921fc9afe16278aa9854685ee1388518cebc
Author: Jörg Hoh <[email protected]>
AuthorDate: Mon Sep 21 13:32:12 2026 +0200
SLING-13350 subsequent call of header-modifying operations must be
logged/fail as well (#85)
---
.../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);
