diff --git a/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java index 3ce71e9c9a8..6aa755f20b2 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java @@ -31,6 +31,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; import org.springframework.http.HttpRequest; +import org.springframework.http.HttpStatus; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedCaseInsensitiveMap; @@ -78,6 +79,8 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private boolean removeOnly; + private boolean relativeRedirects; + public ForwardedHeaderFilter() { this.pathHelper = new UrlPathHelper(); @@ -89,13 +92,28 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { /** * Enables mode in which any "Forwarded" or "X-Forwarded-*" headers are * removed only and the information in them ignored. - * @param removeOnly whether to discard and ingore forwarded headers + * @param removeOnly whether to discard and ignore forwarded headers * @since 4.3.9 */ public void setRemoveOnly(boolean removeOnly) { this.removeOnly = removeOnly; } + /** + * Use this property to enable relative redirects as explained in and also + * using the same response wrapper as {@link RelativeRedirectFilter} does. + * Or if both filters are used, only one will wrap the response. + *

By default, if this property is set to false, in which case calls to + * {@link HttpServletResponse#sendRedirect(String)} are overridden in order + * to turn relative into absolute URLs since (which Servlet containers are + * also required to do) also taking forwarded headers into consideration. + * @param relativeRedirects whether to use relative redirects + * @since 4.3.10 + */ + public void setRelativeRedirects(boolean relativeRedirects) { + this.relativeRedirects = relativeRedirects; + } + @Override protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException { @@ -129,7 +147,9 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } else { HttpServletRequest theRequest = new ForwardedHeaderExtractingRequest(request, this.pathHelper); - HttpServletResponse theResponse = new ForwardedHeaderExtractingResponse(response, theRequest); + HttpServletResponse theResponse = (this.relativeRedirects ? + RelativeRedirectResponseWrapper.wrapIfNecessary(response, HttpStatus.SEE_OTHER) : + new ForwardedHeaderExtractingResponse(response, theRequest)); filterChain.doFilter(theRequest, theResponse); } } @@ -142,7 +162,6 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private final Map> headers; - public ForwardedHeaderRemovingRequest(HttpServletRequest request) { super(request); this.headers = initHeaders(request); @@ -180,6 +199,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } } + /** * Extract and use "Forwarded" or "X-Forwarded-*" headers. */ @@ -199,7 +219,6 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private final String requestUrl; - public ForwardedHeaderExtractingRequest(HttpServletRequest request, UrlPathHelper pathHelper) { super(request); @@ -276,10 +295,8 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private static final String FOLDER_SEPARATOR = "/"; - private final HttpServletRequest request; - public ForwardedHeaderExtractingResponse(HttpServletResponse response, HttpServletRequest request) { super(response); this.request = request; diff --git a/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectFilter.java b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectFilter.java new file mode 100644 index 00000000000..b9055b1ffd5 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectFilter.java @@ -0,0 +1,74 @@ +/* + * Copyright 2002-2017 the original author or authors. + * + * Licensed 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.springframework.web.filter; + +import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.http.HttpStatus; +import org.springframework.util.Assert; + +/** + * Overrides {@link HttpServletResponse#sendRedirect(String)} and handles it by + * setting the HTTP status and "Location" headers. This keeps the Servlet + * container from re-writing relative redirect URLs and instead follows the + * recommendation in + * RFC 7231 Section 7.1.2. + * + *

Note: While relative redirects are more efficient they + * may not work with reverse proxies under some configurations. + * + * @author Rob Winch + * @author Rossen Stoyanchev + * @since 4.3.10 + */ +public class RelativeRedirectFilter extends OncePerRequestFilter { + + private HttpStatus redirectStatus = HttpStatus.SEE_OTHER; + + + /** + * Set the default HTTP Status to use for redirects. + *

By default this is {@link HttpStatus#SEE_OTHER}. + * @param status the 3xx redirect status to use + */ + public void setRedirectStatus(HttpStatus status) { + Assert.notNull(status, "Property 'redirectStatus' is required"); + Assert.isTrue(status.is3xxRedirection(), "Not a redirect status code"); + this.redirectStatus = status; + } + + /** + * Return the configured redirect status. + */ + public HttpStatus getRedirectStatus() { + return this.redirectStatus; + } + + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + + response = RelativeRedirectResponseWrapper.wrapIfNecessary(response, this.redirectStatus); + filterChain.doFilter(request, response); + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectResponseWrapper.java b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectResponseWrapper.java new file mode 100644 index 00000000000..af95f1c4188 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectResponseWrapper.java @@ -0,0 +1,72 @@ +/* + * Copyright 2002-2017 the original author or authors. + * + * Licensed 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.springframework.web.filter; + +import java.io.IOException; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.util.Assert; + +/** + * A response wrapper used for the implementation of + * {@link RelativeRedirectFilter} also shared with {@link ForwardedHeaderFilter}. + * + * @author Rossen Stoyanchev + * @since 4.3.10 + */ +class RelativeRedirectResponseWrapper extends HttpServletResponseWrapper { + + private final HttpStatus redirectStatus; + + + private RelativeRedirectResponseWrapper(HttpServletResponse response, HttpStatus redirectStatus) { + super(response); + Assert.notNull(redirectStatus, "'redirectStatus' is required"); + this.redirectStatus = redirectStatus; + } + + + @Override + public void sendRedirect(String location) throws IOException { + setStatus(this.redirectStatus.value()); + setHeader(HttpHeaders.LOCATION, location); + } + + + public static HttpServletResponse wrapIfNecessary(HttpServletResponse response, + HttpStatus redirectStatus) { + + return (hasWrapper(response) ? response : new RelativeRedirectResponseWrapper(response, redirectStatus)); + } + + private static boolean hasWrapper(ServletResponse response) { + if (response instanceof RelativeRedirectResponseWrapper) { + return true; + } + while (response instanceof HttpServletResponseWrapper) { + response = ((HttpServletResponseWrapper) response).getResponse(); + if (response instanceof RelativeRedirectResponseWrapper) { + return true; + } + } + return false; + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java index 15aff6ffc12..4e20defb0e6 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,11 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.web.filter; import java.io.IOException; import java.util.Enumeration; - import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -32,20 +32,18 @@ import org.springframework.mock.web.test.MockFilterChain; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * Unit tests for {@link ForwardedHeaderFilter}. + * * @author Rossen Stoyanchev * @author Eddú Meléndez * @author Rob Winch */ public class ForwardedHeaderFilterTests { - private static final String X_FORWARDED_PROTO = "x-forwarded-proto"; // SPR-14372 (case insensitive) + private static final String X_FORWARDED_PROTO = "x-forwarded-proto"; // SPR-14372 (case insensitive) private static final String X_FORWARDED_HOST = "x-forwarded-host"; private static final String X_FORWARDED_PORT = "x-forwarded-port"; private static final String X_FORWARDED_PREFIX = "x-forwarded-prefix"; @@ -60,7 +58,7 @@ public class ForwardedHeaderFilterTests { @Before @SuppressWarnings("serial") - public void setUp() throws Exception { + public void setup() throws Exception { this.request = new MockHttpServletRequest(); this.request.setScheme("http"); this.request.setServerName("localhost"); @@ -185,9 +183,7 @@ public class ForwardedHeaderFilterTests { @Test public void caseInsensitiveForwardedPrefix() throws Exception { this.request = new MockHttpServletRequest() { - // Make it case-sensitive (SPR-14372) - @Override public String getHeader(String header) { Enumeration names = getHeaderNames(); @@ -404,6 +400,27 @@ public class ForwardedHeaderFilterTests { assertEquals("../foo/bar", redirectedUrl); } + @Test + public void sendRedirectWhenRequestOnlyAndXForwardedThenUsesRelativeRedirects() throws Exception { + this.request.addHeader(X_FORWARDED_PROTO, "https"); + this.request.addHeader(X_FORWARDED_HOST, "example.com"); + this.request.addHeader(X_FORWARDED_PORT, "443"); + this.filter.setRelativeRedirects(true); + + String location = sendRedirect("/a"); + + assertEquals("/a", location); + } + + @Test + public void sendRedirectWhenRequestOnlyAndNoXForwardedThenUsesRelativeRedirects() throws Exception { + this.filter.setRelativeRedirects(true); + + String location = sendRedirect("/a"); + + assertEquals("/a", location); + } + private String sendRedirect(final String location) throws ServletException, IOException { MockHttpServletResponse response = doWithFiltersAndGetResponse(this.filter, new OncePerRequestFilter() { @Override @@ -412,10 +429,10 @@ public class ForwardedHeaderFilterTests { response.sendRedirect(location); } }); - return response.getRedirectedUrl(); } + @SuppressWarnings("serial") private MockHttpServletResponse doWithFiltersAndGetResponse(Filter... filters) throws ServletException, IOException { MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = new MockFilterChain(new HttpServlet() {}, filters); diff --git a/spring-web/src/test/java/org/springframework/web/filter/RelativeRedirectFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/RelativeRedirectFilterTests.java new file mode 100644 index 00000000000..46b807346c7 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/filter/RelativeRedirectFilterTests.java @@ -0,0 +1,110 @@ +/* + * Copyright 2002-2017 the original author or authors. + * + * Licensed 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.springframework.web.filter; + +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; + +import org.junit.Test; +import org.mockito.InOrder; +import org.mockito.Mockito; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.mock.web.test.MockFilterChain; +import org.springframework.mock.web.test.MockHttpServletRequest; +import org.springframework.mock.web.test.MockHttpServletResponse; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link RelativeRedirectFilter}. + * + * @author Rob Winch + * @author Juergen Hoeller + */ +public class RelativeRedirectFilterTests { + + private RelativeRedirectFilter filter = new RelativeRedirectFilter(); + + private HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + + + @Test(expected = IllegalArgumentException.class) + public void sendRedirectHttpStatusWhenNullThenIllegalArgumentException() { + this.filter.setRedirectStatus(null); + } + + @Test(expected = IllegalArgumentException.class) + public void sendRedirectHttpStatusWhenNot3xxThenIllegalArgumentException() { + this.filter.setRedirectStatus(HttpStatus.OK); + } + + @Test + public void doFilterSendRedirectWhenDefaultsThenLocationAnd303() throws Exception { + String location = "/foo"; + sendRedirect(location); + + InOrder inOrder = Mockito.inOrder(this.response); + inOrder.verify(this.response).setStatus(HttpStatus.SEE_OTHER.value()); + inOrder.verify(this.response).setHeader(HttpHeaders.LOCATION, location); + } + + @Test + public void doFilterSendRedirectWhenCustomSendRedirectHttpStatusThenLocationAnd301() throws Exception { + String location = "/foo"; + HttpStatus status = HttpStatus.MOVED_PERMANENTLY; + this.filter.setRedirectStatus(status); + sendRedirect(location); + + InOrder inOrder = Mockito.inOrder(this.response); + inOrder.verify(this.response).setStatus(status.value()); + inOrder.verify(this.response).setHeader(HttpHeaders.LOCATION, location); + } + + @Test + public void wrapOnceOnly() throws Exception { + HttpServletResponse original = new MockHttpServletResponse(); + + MockFilterChain chain = new MockFilterChain(); + this.filter.doFilterInternal(new MockHttpServletRequest(), original, chain); + + HttpServletResponse wrapped1 = (HttpServletResponse) chain.getResponse(); + assertNotSame(original, wrapped1); + + chain.reset(); + this.filter.doFilterInternal(new MockHttpServletRequest(), wrapped1, chain); + HttpServletResponse current = (HttpServletResponse) chain.getResponse(); + assertSame(wrapped1, current); + + chain.reset(); + HttpServletResponse wrapped2 = new HttpServletResponseWrapper(wrapped1); + this.filter.doFilterInternal(new MockHttpServletRequest(), wrapped2, chain); + current = (HttpServletResponse) chain.getResponse(); + assertSame(wrapped2, current); + } + + + private void sendRedirect(String location) throws Exception { + MockFilterChain chain = new MockFilterChain(); + this.filter.doFilterInternal(new MockHttpServletRequest(), this.response, chain); + + HttpServletResponse wrappedResponse = (HttpServletResponse) chain.getResponse(); + wrappedResponse.sendRedirect(location); + } + +}