You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/08/06 15:16:32 UTC
[sling-org-apache-sling-engine] branch master updated: SLING-11514 : Provide dispatcher option to ignore header changes on include
This is an automated email from the ASF dual-hosted git repository.
cziegeler 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 5b8354a SLING-11514 : Provide dispatcher option to ignore header changes on include
5b8354a is described below
commit 5b8354ac62b712aa162222c1fd301a351dfb9bac
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Sat Aug 6 17:16:27 2022 +0200
SLING-11514 : Provide dispatcher option to ignore header changes on include
---
pom.xml | 8 +-
.../sling/engine/impl/IncludeResponseWrapper.java | 130 +++++++++++++++++++++
.../engine/impl/SlingRequestProcessorImpl.java | 21 ++--
.../impl/request/SlingRequestDispatcher.java | 16 ++-
.../engine/impl/IncludeResponseWrapperTest.java | 105 +++++++++++++++++
5 files changed, 267 insertions(+), 13 deletions(-)
diff --git a/pom.xml b/pom.xml
index 68a3590..3b3bdac 100644
--- a/pom.xml
+++ b/pom.xml
@@ -116,7 +116,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>2.25.0</version>
+ <version>2.26.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -161,6 +161,12 @@
<groupId>junit</groupId>
<artifactId>junit</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>4.6.1</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.jmock</groupId>
<artifactId>jmock-junit4</artifactId>
diff --git a/src/main/java/org/apache/sling/engine/impl/IncludeResponseWrapper.java b/src/main/java/org/apache/sling/engine/impl/IncludeResponseWrapper.java
new file mode 100644
index 0000000..0da6b0b
--- /dev/null
+++ b/src/main/java/org/apache/sling/engine/impl/IncludeResponseWrapper.java
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.engine.impl;
+
+import java.io.IOException;
+import java.util.Locale;
+
+import javax.servlet.http.Cookie;
+
+import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.api.wrappers.SlingHttpServletResponseWrapper;
+
+/**
+ * Wrapper to handle dispatching of includes where all changes to headers
+ * are simply silently ignored (see section 9.3 of the servlet specification)
+ */
+public class IncludeResponseWrapper extends SlingHttpServletResponseWrapper {
+
+ public IncludeResponseWrapper(final SlingHttpServletResponse response) {
+ super(response);
+ }
+
+ @Override
+ public void reset() {
+ // ignore if not committed
+ if (getResponse().isCommitted()) {
+ getResponse().reset();
+ }
+ }
+
+ @Override
+ public void setContentLength(final int len) {
+ // ignore
+ }
+
+ @Override
+ public void setContentLengthLong(final long len) {
+ // ignore
+ }
+
+ @Override
+ public void setContentType(final String type) {
+ // ignore
+ }
+
+ @Override
+ public void setLocale(final Locale loc) {
+ // ignore
+ }
+
+ @Override
+ public void setBufferSize(final int size) {
+ // ignore
+ }
+
+ @Override
+ public void addCookie(final Cookie cookie) {
+ // ignore
+ }
+
+
+ @Override
+ public void addDateHeader(final String name, final long value) {
+ // ignore
+ }
+
+ @Override
+ public void addHeader(final String name, final String value) {
+ // ignore
+ }
+
+ @Override
+ public void addIntHeader(final String name, final int value) {
+ // ignore
+ }
+
+ @Override
+ public void sendError(final int sc) throws IOException {
+ // ignore
+ }
+
+ @Override
+ public void sendError(final int sc, final String msg) throws IOException {
+ // ignore
+ }
+
+ @Override
+ public void sendRedirect(final String location) throws IOException {
+ // ignore
+ }
+
+ @Override
+ public void setDateHeader(final String name, final long value) {
+ // ignore
+ }
+
+ @Override
+ public void setHeader(final String name, final String value) {
+ // ignore
+ }
+
+ @Override
+ public void setIntHeader(final String name, final int value) {
+ // ignore
+ }
+
+ @Override
+ public void setStatus(final int sc) {
+ // ignore
+ }
+
+ @Override
+ public void setStatus(final int sc, final String msg) {
+ // ignore
+ }
+}
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 d70283b..63b1458 100644
--- a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
+++ b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
@@ -295,16 +295,23 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor {
/**
* Dispatches the request on behalf of the
* {@link org.apache.sling.engine.impl.request.SlingRequestDispatcher}.
+ * @param request The request
+ * @param response The response
+ * @param resource The resource
+ * @param resolvedURL Request path info
+ * @param include Is this an include (or forward) ?
+ * @param protectHeadersOnInclude Should the headers be protected on include?
*/
- public void dispatchRequest(ServletRequest request,
- ServletResponse response, Resource resource,
- RequestPathInfo resolvedURL, boolean include) throws IOException,
- ServletException {
+ public void dispatchRequest(final ServletRequest request,
+ final ServletResponse response, final Resource resource,
+ final RequestPathInfo resolvedURL,
+ final boolean include,
+ final boolean protectHeadersOnInclude) throws IOException, ServletException {
// we need a SlingHttpServletRequest/SlingHttpServletResponse tupel
// to continue
- SlingHttpServletRequest cRequest = RequestData.toSlingHttpServletRequest(request);
- SlingHttpServletResponse cResponse = RequestData.toSlingHttpServletResponse(response);
+ final SlingHttpServletRequest cRequest = RequestData.toSlingHttpServletRequest(request);
+ final SlingHttpServletResponse cResponse = RequestData.toSlingHttpServletResponse(response);
// get the request data (and btw check the correct type)
final RequestData requestData = RequestData.getRequestData(cRequest);
@@ -320,7 +327,7 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor {
? FilterChainType.INCLUDE
: FilterChainType.FORWARD;
- processComponent(cRequest, cResponse, type);
+ processComponent(cRequest, include && protectHeadersOnInclude ? new IncludeResponseWrapper(cResponse) : cResponse, type);
} finally {
requestData.resetContent(oldContentData);
}
diff --git a/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java b/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java
index 9f7924e..79cc37f 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/SlingRequestDispatcher.java
@@ -168,8 +168,14 @@ public class SlingRequestDispatcher implements RequestDispatcher {
return uri + '/' + path;
}
- private void dispatch(ServletRequest request, ServletResponse sResponse,
- boolean include) throws ServletException, IOException {
+ /**
+ * Dispatch the request
+ * @param request The request
+ * @param response The response
+ * @param include Is this an include (or forward)
+ */
+ private void dispatch(final ServletRequest request, final ServletResponse response,
+ final boolean include) throws ServletException, IOException {
SlingHttpServletRequest cRequest = RequestData.unwrap(request);
RequestData rd = RequestData.getRequestData(cRequest);
String absPath = getAbsolutePath(cRequest, path);
@@ -177,7 +183,7 @@ public class SlingRequestDispatcher implements RequestDispatcher {
// if the response is not an HttpServletResponse, fail gracefully not
// doing anything
- if (!(sResponse instanceof HttpServletResponse)) {
+ if (!(response instanceof HttpServletResponse)) {
log.error("include: Failed to include {}, response has wrong type",
absPath);
return;
@@ -208,8 +214,8 @@ public class SlingRequestDispatcher implements RequestDispatcher {
SlingRequestPathInfo info = getMergedRequestPathInfo(cRequest);
requestProgressTracker.log(
"Including resource {0} ({1})", resource, info);
- rd.getSlingRequestProcessor().dispatchRequest(request, sResponse, resource,
- info, include);
+ rd.getSlingRequestProcessor().dispatchRequest(request, response, resource,
+ info, include, this.options.isProtectHeadersOnInclude());
}
/**
diff --git a/src/test/java/org/apache/sling/engine/impl/IncludeResponseWrapperTest.java b/src/test/java/org/apache/sling/engine/impl/IncludeResponseWrapperTest.java
new file mode 100644
index 0000000..39d39a7
--- /dev/null
+++ b/src/test/java/org/apache/sling/engine/impl/IncludeResponseWrapperTest.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.engine.impl;
+
+import static org.mockito.Mockito.times;
+
+import java.io.IOException;
+
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.engine.impl.IncludeResponseWrapper;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class IncludeResponseWrapperTest {
+
+ @Test public void testReset() {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ Mockito.when(orig.isCommitted()).thenReturn(false);
+ include.reset();
+ Mockito.verify(orig, times(1)).isCommitted();
+ Mockito.verifyNoMoreInteractions(orig);
+
+ Mockito.when(orig.isCommitted()).thenReturn(true);
+ include.reset();
+ Mockito.verify(orig, times(2)).isCommitted();
+ Mockito.verify(orig, times(1)).reset();
+ Mockito.verifyNoMoreInteractions(orig);
+ }
+
+ @Test public void testContentMethods() {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ include.setContentLength(54);
+ include.setContentLengthLong(33L);
+ include.setContentType("text/plain");
+ include.setLocale(null);
+ include.setBufferSize(4500);
+
+ Mockito.verifyNoInteractions(orig);
+ }
+
+ @Test public void testCookies() {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ include.addCookie(new Cookie("foo", "bar"));
+
+ Mockito.verifyNoInteractions(orig);
+ }
+
+ @Test public void testSendError() throws IOException {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ include.sendError(500);
+ include.sendError(500, "Error");
+
+ Mockito.verifyNoInteractions(orig);
+ }
+
+ @Deprecated
+ @Test public void testSetStatus() {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ include.setStatus(500);
+ include.setStatus(500, "Error");
+
+ Mockito.verifyNoInteractions(orig);
+ }
+
+ @Test public void testHeaders() {
+ final SlingHttpServletResponse orig = Mockito.mock(SlingHttpServletResponse.class);
+ final HttpServletResponse include = new IncludeResponseWrapper(orig);
+
+ include.setDateHeader("foo-d", 2000L);
+ include.addDateHeader("bar-d", 3000L);
+ include.setIntHeader("foo-i", 1);
+ include.addIntHeader("bar-i", 2);
+ include.setHeader("foo", "value");
+ include.addHeader("bar", "another");
+
+ Mockito.verifyNoInteractions(orig);
+ }
+}