Skip to content

Commit 5e774e0

Browse files
Rob Winchrstoyanchev
Rob Winch
authored andcommitted
Add Support for ForwardedHeaderFilter sendRedirect
Previously ForwrdedHeaderFilter did not ensure that HttpServletResponse.sendRedirect worked properly based on X-Forwarded-* headers. This commit updates ForwardedHeaderFilter to overrided the HttpServletResponse.sendRedirect method to ensure X-Forwarded-* headers are honored. Issue SPR-15020
1 parent e16d753 commit 5e774e0

File tree

2 files changed

+217
-6
lines changed

2 files changed

+217
-6
lines changed

spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java

+61-6
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,31 @@
2828
import javax.servlet.http.HttpServletRequest;
2929
import javax.servlet.http.HttpServletRequestWrapper;
3030
import javax.servlet.http.HttpServletResponse;
31+
import javax.servlet.http.HttpServletResponseWrapper;
3132

3233
import org.springframework.http.HttpRequest;
3334
import org.springframework.http.server.ServletServerHttpRequest;
3435
import org.springframework.util.CollectionUtils;
3536
import org.springframework.util.LinkedCaseInsensitiveMap;
37+
import org.springframework.util.StringUtils;
3638
import org.springframework.web.util.UriComponents;
3739
import org.springframework.web.util.UriComponentsBuilder;
3840
import org.springframework.web.util.UrlPathHelper;
3941

4042
/**
41-
* Filter that wraps the request in order to override its
43+
* Filter that wraps the request and response in order to override its
4244
* {@link HttpServletRequest#getServerName() getServerName()},
4345
* {@link HttpServletRequest#getServerPort() getServerPort()},
44-
* {@link HttpServletRequest#getScheme() getScheme()}, and
45-
* {@link HttpServletRequest#isSecure() isSecure()} methods with values derived
46-
* from "Forwarded" or "X-Forwarded-*" headers. In effect the wrapped request
47-
* reflects the client-originated protocol and address.
46+
* {@link HttpServletRequest#getScheme() getScheme()},
47+
* {@link HttpServletRequest#isSecure() isSecure()},
48+
* {@link HttpServletResponse#sendRedirect(String) sendRedirect(String)},
49+
* methods with values derived from "Forwarded" or "X-Forwarded-*"
50+
* headers. In effect the wrapped request and response reflects the
51+
* client-originated protocol and address.
4852
*
4953
* @author Rossen Stoyanchev
5054
* @author Eddú Meléndez
55+
* @author Rob Winch
5156
* @since 4.3
5257
*/
5358
public class ForwardedHeaderFilter extends OncePerRequestFilter {
@@ -93,7 +98,9 @@ protected boolean shouldNotFilterErrorDispatch() {
9398
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
9499
FilterChain filterChain) throws ServletException, IOException {
95100

96-
filterChain.doFilter(new ForwardedHeaderRequestWrapper(request, this.pathHelper), response);
101+
ForwardedHeaderRequestWrapper wrappedRequest = new ForwardedHeaderRequestWrapper(request, this.pathHelper);
102+
ForwardedHeaderResponseWrapper wrappedResponse = new ForwardedHeaderResponseWrapper(response, wrappedRequest);
103+
filterChain.doFilter(wrappedRequest, wrappedResponse);
97104
}
98105

99106

@@ -222,4 +229,52 @@ public Enumeration<String> getHeaderNames() {
222229
}
223230
}
224231

232+
private static class ForwardedHeaderResponseWrapper extends HttpServletResponseWrapper {
233+
234+
private static final String FOLDER_SEPARATOR = "/";
235+
236+
private final HttpServletRequest request;
237+
238+
239+
public ForwardedHeaderResponseWrapper(HttpServletResponse response, HttpServletRequest request) {
240+
super(response);
241+
this.request = request;
242+
}
243+
244+
@Override
245+
public void sendRedirect(String location) throws IOException {
246+
247+
UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(location);
248+
249+
// Absolute location
250+
if (builder.build().getScheme() != null) {
251+
super.sendRedirect(location);
252+
return;
253+
}
254+
255+
// Network-path reference
256+
if(location.startsWith("//")) {
257+
String scheme = this.request.getScheme();
258+
super.sendRedirect(builder.scheme(scheme).toUriString());
259+
return;
260+
}
261+
262+
// Relative to Servlet container root or to current request
263+
String path;
264+
if (location.startsWith(FOLDER_SEPARATOR)) {
265+
path = this.request.getContextPath() + location;
266+
}
267+
else {
268+
path = StringUtils.applyRelativePath(this.request.getRequestURI(), location);
269+
}
270+
271+
String result = UriComponentsBuilder
272+
.fromHttpRequest(new ServletServerHttpRequest(this.request))
273+
.replacePath(path)
274+
.build().normalize().toUriString();
275+
276+
super.sendRedirect(result);
277+
}
278+
}
279+
225280
}

spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java

+156
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@
1717

1818
import java.io.IOException;
1919
import java.util.Enumeration;
20+
21+
import javax.servlet.Filter;
22+
import javax.servlet.FilterChain;
2023
import javax.servlet.ServletException;
2124
import javax.servlet.http.HttpServlet;
2225
import javax.servlet.http.HttpServletRequest;
26+
import javax.servlet.http.HttpServletResponse;
2327

2428
import org.junit.Before;
2529
import org.junit.Test;
@@ -37,6 +41,7 @@
3741
* Unit tests for {@link ForwardedHeaderFilter}.
3842
* @author Rossen Stoyanchev
3943
* @author Eddú Meléndez
44+
* @author Rob Winch
4045
*/
4146
public class ForwardedHeaderFilterTests {
4247

@@ -222,6 +227,157 @@ public void contextPathWithForwardedPrefixTrailingSlash() throws Exception {
222227
assertEquals("/prefix", actual);
223228
}
224229

230+
@Test
231+
public void sendRedirectWithAbsolutePath() throws Exception {
232+
this.request.addHeader(X_FORWARDED_PROTO, "https");
233+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
234+
this.request.addHeader(X_FORWARDED_PORT, "443");
235+
236+
String redirectedUrl = sendRedirect("/foo/bar");
237+
assertEquals("https://example.com/foo/bar", redirectedUrl);
238+
}
239+
240+
@Test
241+
public void sendRedirectWithContextPath() throws Exception {
242+
this.request.addHeader(X_FORWARDED_PROTO, "https");
243+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
244+
this.request.addHeader(X_FORWARDED_PORT, "443");
245+
this.request.setContextPath("/context");
246+
247+
String redirectedUrl = sendRedirect("/foo/bar");
248+
assertEquals("https://example.com/context/foo/bar", redirectedUrl);
249+
}
250+
251+
@Test
252+
public void sendRedirectWithXForwardedPrefix() throws Exception {
253+
this.request.addHeader(X_FORWARDED_PROTO, "https");
254+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
255+
this.request.addHeader(X_FORWARDED_PORT, "443");
256+
this.request.addHeader(X_FORWARDED_PREFIX, "/prefix");
257+
258+
String redirectedUrl = sendRedirect("/foo/bar");
259+
assertEquals("https://example.com/prefix/foo/bar", redirectedUrl);
260+
}
261+
262+
@Test
263+
public void sendRedirectWithXForwardedPrefixAndContextPath() throws Exception {
264+
this.request.addHeader(X_FORWARDED_PROTO, "https");
265+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
266+
this.request.addHeader(X_FORWARDED_PORT, "443");
267+
this.request.addHeader(X_FORWARDED_PREFIX, "/prefix");
268+
this.request.setContextPath("/context");
269+
270+
String redirectedUrl = sendRedirect("/foo/bar");
271+
assertEquals("https://example.com/prefix/foo/bar", redirectedUrl);
272+
}
273+
274+
@Test
275+
public void sendRedirectWithRelativePath() throws Exception {
276+
this.request.addHeader(X_FORWARDED_PROTO, "https");
277+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
278+
this.request.addHeader(X_FORWARDED_PORT, "443");
279+
this.request.setRequestURI("/parent/");
280+
281+
String redirectedUrl = sendRedirect("foo/bar");
282+
assertEquals("https://example.com/parent/foo/bar", redirectedUrl);
283+
}
284+
285+
@Test
286+
public void sendRedirectWithFileInPathAndRelativeRedirect() throws Exception {
287+
this.request.addHeader(X_FORWARDED_PROTO, "https");
288+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
289+
this.request.addHeader(X_FORWARDED_PORT, "443");
290+
this.request.setRequestURI("/context/a");
291+
292+
String redirectedUrl = sendRedirect("foo/bar");
293+
assertEquals("https://example.com/context/foo/bar", redirectedUrl);
294+
}
295+
296+
@Test
297+
public void sendRedirectWithRelativePathIgnoresFile() throws Exception {
298+
this.request.addHeader(X_FORWARDED_PROTO, "https");
299+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
300+
this.request.addHeader(X_FORWARDED_PORT, "443");
301+
this.request.setRequestURI("/parent");
302+
303+
String redirectedUrl = sendRedirect("foo/bar");
304+
assertEquals("https://example.com/foo/bar", redirectedUrl);
305+
}
306+
307+
@Test
308+
public void sendRedirectWithLocationDotDotPath() throws Exception {
309+
this.request.addHeader(X_FORWARDED_PROTO, "https");
310+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
311+
this.request.addHeader(X_FORWARDED_PORT, "443");
312+
313+
String redirectedUrl = sendRedirect("parent/../foo/bar");
314+
assertEquals("https://example.com/foo/bar", redirectedUrl);
315+
}
316+
317+
@Test
318+
public void sendRedirectWithLocationHasScheme() throws Exception {
319+
this.request.addHeader(X_FORWARDED_PROTO, "https");
320+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
321+
this.request.addHeader(X_FORWARDED_PORT, "443");
322+
323+
String location = "http://other.info/foo/bar";
324+
String redirectedUrl = sendRedirect(location);
325+
assertEquals(location, redirectedUrl);
326+
}
327+
328+
@Test
329+
public void sendRedirectWithLocationSlashSlash() throws Exception {
330+
this.request.addHeader(X_FORWARDED_PROTO, "https");
331+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
332+
this.request.addHeader(X_FORWARDED_PORT, "443");
333+
334+
String location = "//other.info/foo/bar";
335+
String redirectedUrl = sendRedirect(location);
336+
assertEquals("https:" + location, redirectedUrl);
337+
}
338+
339+
@Test
340+
public void sendRedirectWithLocationSlashSlashParentDotDot() throws Exception {
341+
this.request.addHeader(X_FORWARDED_PROTO, "https");
342+
this.request.addHeader(X_FORWARDED_HOST, "example.com");
343+
this.request.addHeader(X_FORWARDED_PORT, "443");
344+
345+
String location = "//other.info/parent/../foo/bar";
346+
String redirectedUrl = sendRedirect(location);
347+
assertEquals("https:" + location, redirectedUrl);
348+
}
349+
350+
@Test
351+
public void sendRedirectWithNoXForwardedAndAbsolutePath() throws Exception {
352+
String redirectedUrl = sendRedirect("/foo/bar");
353+
assertEquals("/foo/bar", redirectedUrl);
354+
}
355+
356+
@Test
357+
public void sendRedirectWithNoXForwardedAndDotDotPath() throws Exception {
358+
String redirectedUrl = sendRedirect("../foo/bar");
359+
assertEquals("../foo/bar", redirectedUrl);
360+
}
361+
362+
private String sendRedirect(final String location) throws ServletException, IOException {
363+
MockHttpServletResponse response = doWithFiltersAndGetResponse(this.filter, new OncePerRequestFilter() {
364+
@Override
365+
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
366+
throws ServletException, IOException {
367+
response.sendRedirect(location);
368+
}
369+
});
370+
371+
return response.getRedirectedUrl();
372+
}
373+
374+
private MockHttpServletResponse doWithFiltersAndGetResponse(Filter... filters) throws ServletException, IOException {
375+
MockHttpServletResponse response = new MockHttpServletResponse();
376+
FilterChain filterChain = new MockFilterChain(new HttpServlet() {}, filters);
377+
filterChain.doFilter(request, response);
378+
return response;
379+
}
380+
225381
private String filterAndGetContextPath() throws ServletException, IOException {
226382
return filterAndGetWrappedRequest().getContextPath();
227383
}

0 commit comments

Comments
 (0)