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 @@ 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 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 IncludeNoContentTypeOverrideResponseWrapper(@NotNull RequestData requestD 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 currentMime = Arrays.stream(contentTypeString.split(";")).findFirst(); Optional 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 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 void testContentTypeOverrideThrows() { } @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 void testContentTypeOverrideLenient() { } 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()); } }