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 be68a201cff..82fa047e427 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 @@ -101,13 +101,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 */ @@ -307,54 +307,41 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { this.request = request; } - @Override - public void sendRedirect(String location) throws IOException { - UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(location); - - // Absolute location - if (builder.build().getScheme() != null) { - super.sendRedirect(location); - return; - } +@Override +public void sendRedirect(String location) throws IOException { - // Network-path reference - if (location.startsWith("//")) { - String scheme = this.request.getScheme(); - super.sendRedirect(builder.scheme(scheme).toUriString()); - return; - } + UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(location); + UriComponents uriComponents = builder.build(); - String fragment = null; - int fragmentIndex = location.indexOf('#'); - if (fragmentIndex != -1) { - if (location.length() > fragmentIndex) { - fragment = location.substring(fragmentIndex + 1); - } - location = location.substring(0, fragmentIndex); - } + // Absolute location + if (uriComponents.getScheme() != null) { + super.sendRedirect(location); + return; + } - String query = null; - int queryIndex = location.indexOf('?'); - if (queryIndex != -1) { - if (location.length() > queryIndex) { - query = location.substring(queryIndex + 1); - } - location = location.substring(0, queryIndex); - } + // Network-path reference + if (location.startsWith("//")) { + String scheme = this.request.getScheme(); + super.sendRedirect(builder.scheme(scheme).toUriString()); + 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(query) - .fragment(fragment) - .build().normalize().toUriString(); + String result = UriComponentsBuilder + .fromHttpRequest(new ServletServerHttpRequest(this.request)) + .replacePath(path) + .replaceQuery(uriComponents.getQuery()) + .fragment(uriComponents.getFragment()) + .build().normalize().toUriString(); - super.sendRedirect(result); - } + 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 b9055b1ffd5..5c5a5232d3f 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 af95f1c4188..a3f5e938f52 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 @@ -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 18b2a1ace4f..ef14dbefbbc 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 @@ -433,18 +433,22 @@ public class ForwardedHeaderFilterTests { } private String sendRedirect(final String location) throws ServletException, IOException { - MockHttpServletResponse response = doWithFiltersAndGetResponse(this.filter, new OncePerRequestFilter() { + Filter filter = new OncePerRequestFilter() { @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) - throws ServletException, IOException { - response.sendRedirect(location); + protected void doFilterInternal(HttpServletRequest req, HttpServletResponse res, + FilterChain chain) throws IOException { + + res.sendRedirect(location); } - }); + }; + MockHttpServletResponse response = doWithFiltersAndGetResponse(this.filter, filter); return response.getRedirectedUrl(); } @SuppressWarnings("serial") - private MockHttpServletResponse doWithFiltersAndGetResponse(Filter... filters) throws ServletException, IOException { + private MockHttpServletResponse doWithFiltersAndGetResponse(Filter... filters) + throws ServletException, IOException { + MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = new MockFilterChain(new HttpServlet() {}, filters); filterChain.doFilter(request, response);