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 6aa755f20b..f78ecc37c3 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -100,13 +100,13 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } /** - * 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. + * Use this property to enable relative redirects as explained in + * {@link RelativeRedirectFilter}, and also using the same response wrapper + * as that filter does, or if both are configured, only one will wrap. *

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. + * to turn relative into absolute URLs, also taking into account forwarded + * headers. * @param relativeRedirects whether to use relative redirects * @since 4.3.10 */ @@ -304,10 +304,12 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { @Override public void sendRedirect(String location) throws IOException { + UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(location); + UriComponents uriComponents = builder.build(); // Absolute location - if (builder.build().getScheme() != null) { + if (uriComponents.getScheme() != null) { super.sendRedirect(location); return; } @@ -319,13 +321,18 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { return; } - // Relative to Servlet container root or to current request - String path = (location.startsWith(FOLDER_SEPARATOR) ? location : - StringUtils.applyRelativePath(this.request.getRequestURI(), location)); + String path = uriComponents.getPath(); + if (path != null) { + // Relative to Servlet container root or to current request + path = (path.startsWith(FOLDER_SEPARATOR) ? path : + StringUtils.applyRelativePath(this.request.getRequestURI(), path)); + } String result = UriComponentsBuilder .fromHttpRequest(new ServletServerHttpRequest(this.request)) .replacePath(path) + .replaceQuery(uriComponents.getQuery()) + .fragment(uriComponents.getFragment()) .build().normalize().toUriString(); super.sendRedirect(result); 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 index b9055b1ffd..5c5a5232d3 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -27,13 +27,14 @@ 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. + * setting the HTTP status and "Location" headers, which keeps the Servlet + * container from re-writing relative redirect URLs into absolute ones. + * Servlet containers are required to do that but against the recommendation of + * RFC 7231 Section 7.1.2, + * and furthermore not necessarily taking into account "X-Forwarded" headers. * - *

Note: While relative redirects are more efficient they - * may not work with reverse proxies under some configurations. + *

Note: While relative redirects are recommended in the + * RFC, under some configurations with reverse proxies they may not work. * * @author Rob Winch * @author Rossen Stoyanchev 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 index af95f1c418..9055f675ae 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectResponseWrapper.java +++ b/spring-web/src/main/java/org/springframework/web/filter/RelativeRedirectResponseWrapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -15,14 +15,13 @@ */ 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; +import org.springframework.web.util.WebUtils; /** * A response wrapper used for the implementation of @@ -44,7 +43,7 @@ class RelativeRedirectResponseWrapper extends HttpServletResponseWrapper { @Override - public void sendRedirect(String location) throws IOException { + public void sendRedirect(String location) { setStatus(this.redirectStatus.value()); setHeader(HttpHeaders.LOCATION, location); } @@ -53,20 +52,11 @@ class RelativeRedirectResponseWrapper extends HttpServletResponseWrapper { public static HttpServletResponse wrapIfNecessary(HttpServletResponse response, HttpStatus redirectStatus) { - return (hasWrapper(response) ? response : new RelativeRedirectResponseWrapper(response, redirectStatus)); - } + RelativeRedirectResponseWrapper wrapper = + WebUtils.getNativeResponse(response, RelativeRedirectResponseWrapper.class); - 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; + return (wrapper != null ? response : + new RelativeRedirectResponseWrapper(response, redirectStatus)); } } 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 4e20defb0e..18b2a1ace4 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-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -301,6 +301,17 @@ public class ForwardedHeaderFilterTests { assertEquals("https://example.com/foo/bar", redirectedUrl); } + @Test // SPR-16506 + public void sendRedirectWithAbsolutePathQueryParamAndFragment() 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.request.setQueryString("oldqp=1"); + + String redirectedUrl = sendRedirect("/foo/bar?newqp=2#fragment"); + assertEquals("https://example.com/foo/bar?newqp=2#fragment", redirectedUrl); + } + @Test public void sendRedirectWithContextPath() throws Exception { this.request.addHeader(X_FORWARDED_PROTO, "https");