This is an automated email from the ASF dual-hosted git repository.
radu 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 ece9340 SLING-11722 - The SlingRequestDispatcher doesn't correctly
implement the RequestDispatcher API
ece9340 is described below
commit ece93401392f7fb3b75b02af62d392e75120d245
Author: Radu Cotescu <[email protected]>
AuthorDate: Tue Dec 13 13:08:10 2022 +0100
SLING-11722 - The SlingRequestDispatcher doesn't correctly implement the
RequestDispatcher API
* the IncludeNoContentTypeOverrideResponseWrapper will only report on
Content-Type overrides,
but will not prevent included servlets from setting other headers or the
status code
---
.../java/org/apache/sling/engine/impl/Config.java | 8 +++---
...ncludeNoContentTypeOverrideResponseWrapper.java | 31 +++++++++++++++-------
...deNoContentTypeOverrideResponseWrapperTest.java | 27 ++++++++++++++++---
3 files changed, 48 insertions(+), 18 deletions(-)
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 b47fdfd..16854a4 100644
--- a/src/main/java/org/apache/sling/engine/impl/Config.java
+++ b/src/main/java/org/apache/sling/engine/impl/Config.java
@@ -18,6 +18,8 @@
*/
package org.apache.sling.engine.impl;
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
import org.osgi.service.metatype.annotations.AttributeDefinition;
import org.osgi.service.metatype.annotations.ObjectClassDefinition;
@@ -99,9 +101,7 @@ public @interface Config {
boolean sling_includes_protectheaders() default false;
@AttributeDefinition(name = "Check Content-Type overrides",
- description = "When enabled, in addition to not allowing servlets
included via the RequestDispatcher to " +
- "change the response status code or set headers, it will
also check explicit overrides of the " +
- "Content-Type header and will make the Sling Engine throw
a RuntimeException when such an " +
- "override is detected.")
+ description = "When enabled, it will check explicit overrides of
the Content-Type header and will make the " +
+ "Sling Engine throw a RuntimeException when such an
override is detected.")
boolean sling_includes_checkcontenttype() default false;
}
diff --git
a/src/main/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapper.java
b/src/main/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapper.java
index da1c7d0..ed0aef8 100644
---
a/src/main/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapper.java
+++
b/src/main/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapper.java
@@ -23,10 +23,11 @@ import java.util.Optional;
import org.apache.sling.api.SlingException;
import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.api.wrappers.SlingHttpServletResponseWrapper;
import org.apache.sling.engine.impl.request.RequestData;
import org.jetbrains.annotations.NotNull;
-public class IncludeNoContentTypeOverrideResponseWrapper extends
IncludeResponseWrapper {
+public class IncludeNoContentTypeOverrideResponseWrapper extends
SlingHttpServletResponseWrapper {
private final RequestData requestData;
@@ -47,24 +48,34 @@ public class IncludeNoContentTypeOverrideResponseWrapper
extends IncludeResponse
public void setContentType(String type) {
String contentTypeString = getContentType();
if (contentTypeString != null) {
+ if (type == null) {
+ String message = getMessage(contentTypeString, "null");
+ requestData.getRequestProgressTracker().log("ERROR: " +
message);
+ throw new ContentTypeChangeException(message);
+ }
Optional<String> currentMime =
Arrays.stream(contentTypeString.split(";")).findFirst();
Optional<String> setMime =
Arrays.stream(type.split(";")).findFirst();
if (currentMime.isPresent() && setMime.isPresent() &&
!currentMime.get().equals(setMime.get())) {
- String message = String.format(
- "Servlet %s tried to override the 'Content-Type'
header from '%s' to '%s', however the" +
- " %s forbids this via the %s configuration
property.",
- requestData.getActiveServletName(),
- currentMime.get(),
- setMime.get(),
- Config.PID,
- "sling.includes.checkcontenttype"
- );
+ String message = getMessage(contentTypeString, type);
requestData.getRequestProgressTracker().log("ERROR: " +
message);
throw new ContentTypeChangeException(message);
}
+ getResponse().setContentType(type);
}
}
+ private String getMessage(String currentContentType, String
setContentType) {
+ return String.format(
+ "Servlet %s tried to override the 'Content-Type' header from
'%s' to '%s', however the" +
+ " %s forbids this via the %s configuration property.",
+ requestData.getActiveServletName(),
+ currentContentType,
+ setContentType,
+ Config.PID,
+ "sling.includes.checkcontenttype"
+ );
+ }
+
private static class ContentTypeChangeException extends SlingException {
protected ContentTypeChangeException(String text) {
super(text);
diff --git
a/src/test/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapperTest.java
b/src/test/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapperTest.java
index 89367ba..2bd063b 100644
---
a/src/test/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapperTest.java
+++
b/src/test/java/org/apache/sling/engine/impl/IncludeNoContentTypeOverrideResponseWrapperTest.java
@@ -26,7 +26,8 @@ import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -61,13 +62,18 @@ public class
IncludeNoContentTypeOverrideResponseWrapperTest {
}
@Test
- public void testContentTypeOverrideLenient() {
+ public void testContentTypeOverrideLenient() throws Throwable {
RequestData requestData = mock(RequestData.class);
when(requestData.getActiveServletName()).thenReturn(ACTIVE_SERVLET_NAME);
RequestProgressTracker requestProgressTracker =
mock(RequestProgressTracker.class);
when(requestData.getRequestProgressTracker()).thenReturn(requestProgressTracker);
SlingHttpServletResponse response =
mock(SlingHttpServletResponse.class);
when(response.getContentType()).thenReturn("text/html");
+ doAnswer(invocationOnMock -> {
+ String setContentType = invocationOnMock.getArgument(0);
+ when(response.getContentType()).thenReturn(setContentType);
+ return null;
+ }).when(response).setContentType(anyString());
IncludeNoContentTypeOverrideResponseWrapper wrapper = new
IncludeNoContentTypeOverrideResponseWrapper(requestData,
@@ -80,8 +86,21 @@ public class IncludeNoContentTypeOverrideResponseWrapperTest
{
} catch (Throwable t) {
throwable = t;
}
- assertNull(throwable);
- assertEquals("text/html", wrapper.getContentType());
+ if (throwable != null) {
+ throw throwable;
+ }
+ assertEquals("text/html;utf-8", wrapper.getContentType());
+
+ try {
+ wrapper.setContentType(null);
+ } catch (Throwable t) {
+ throwable = t;
+ }
+ assertNotNull(throwable);
+ assertEquals("Servlet activeServlet tried to override the
'Content-Type' header from 'text/html;utf-8' to " +
+ "'null', however the
org.apache.sling.engine.impl.SlingMainServlet forbids this via the sling" +
+ ".includes.checkcontenttype configuration property.",
throwable.getMessage());
+ assertEquals("text/html;utf-8", wrapper.getContentType());
}
}
