From 919f6c96f9887746d14dbad2b6cd7af906484aea Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 17 Jun 2016 14:20:42 -0400 Subject: [PATCH] ForwardedHeaderFilter is case-insensitive Issue: SPR-14372 --- .../web/filter/ForwardedHeaderFilter.java | 46 ++++++------ .../filter/ForwardedHeaderFilterTests.java | 72 +++++++++++++------ .../support/ServletUriComponentsBuilder.java | 12 +++- 3 files changed, 88 insertions(+), 42 deletions(-) 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 82ebb0a130..a1196ec2fd 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 @@ -19,9 +19,8 @@ package org.springframework.web.filter; import java.io.IOException; import java.util.Collections; import java.util.Enumeration; -import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import javax.servlet.FilterChain; @@ -33,6 +32,7 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.http.HttpRequest; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.util.CollectionUtils; +import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UrlPathHelper; @@ -52,10 +52,10 @@ import org.springframework.web.util.UrlPathHelper; */ public class ForwardedHeaderFilter extends OncePerRequestFilter { - private static final Set FORWARDED_HEADER_NAMES; + private static final Set FORWARDED_HEADER_NAMES = + Collections.newSetFromMap(new LinkedCaseInsensitiveMap(5, Locale.ENGLISH)); static { - FORWARDED_HEADER_NAMES = new HashSet(5); FORWARDED_HEADER_NAMES.add("Forwarded"); FORWARDED_HEADER_NAMES.add("X-Forwarded-Host"); FORWARDED_HEADER_NAMES.add("X-Forwarded-Port"); @@ -69,9 +69,9 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { @Override protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException { - Enumeration headerNames = request.getHeaderNames(); - while (headerNames.hasMoreElements()) { - String name = headerNames.nextElement(); + Enumeration names = request.getHeaderNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); if (FORWARDED_HEADER_NAMES.contains(name)) { return false; } @@ -136,27 +136,33 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } private static String getForwardedPrefix(HttpServletRequest request) { - String header = request.getHeader("X-Forwarded-Prefix"); - if (header != null) { - while (header.endsWith("/")) { - header = header.substring(0, header.length() - 1); + String prefix = null; + Enumeration names = request.getHeaderNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + if ("X-Forwarded-Prefix".equalsIgnoreCase(name)) { + prefix = request.getHeader(name); } } - return header; + if (prefix != null) { + while (prefix.endsWith("/")) { + prefix = prefix.substring(0, prefix.length() - 1); + } + } + return prefix; } /** * Copy the headers excluding any {@link #FORWARDED_HEADER_NAMES}. */ private static Map> initHeaders(HttpServletRequest request) { - Map> headers = new LinkedHashMap>(); - Enumeration headerNames = request.getHeaderNames(); - while (headerNames.hasMoreElements()) { - String name = headerNames.nextElement(); - headers.put(name, Collections.list(request.getHeaders(name))); - } - for (String name : FORWARDED_HEADER_NAMES) { - headers.remove(name); + Map> headers = new LinkedCaseInsensitiveMap>(Locale.ENGLISH); + Enumeration names = request.getHeaderNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + if (!FORWARDED_HEADER_NAMES.contains(name)) { + headers.put(name, Collections.list(request.getHeaders(name))); + } } return headers; } 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 08bf91bc5a..56d93f8c40 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 @@ -16,6 +16,7 @@ package org.springframework.web.filter; import java.io.IOException; +import java.util.Enumeration; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -39,6 +40,12 @@ import static org.junit.Assert.assertTrue; */ public class ForwardedHeaderFilterTests { + 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"; + + private final ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); private MockHttpServletRequest request; @@ -59,25 +66,25 @@ public class ForwardedHeaderFilterTests { @Test public void contextPathEmpty() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", ""); + this.request.addHeader(X_FORWARDED_PREFIX, ""); assertEquals("", filterAndGetContextPath()); } @Test public void contextPathWithTrailingSlash() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/foo/bar/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/foo/bar/"); assertEquals("/foo/bar", filterAndGetContextPath()); } @Test public void contextPathWithTrailingSlashes() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/foo/bar/baz///"); + this.request.addHeader(X_FORWARDED_PREFIX, "/foo/bar/baz///"); assertEquals("/foo/bar/baz", filterAndGetContextPath()); } @Test public void requestUri() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/"); this.request.setContextPath("/app"); this.request.setRequestURI("/app/path"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -88,7 +95,7 @@ public class ForwardedHeaderFilterTests { @Test public void requestUriWithTrailingSlash() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/"); this.request.setContextPath("/app"); this.request.setRequestURI("/app/path/"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -98,7 +105,7 @@ public class ForwardedHeaderFilterTests { } @Test public void requestUriEqualsContextPath() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/"); this.request.setContextPath("/app"); this.request.setRequestURI("/app"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -109,7 +116,7 @@ public class ForwardedHeaderFilterTests { @Test public void requestUriRootUrl() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/"); this.request.setContextPath("/app"); this.request.setRequestURI("/app/"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -118,12 +125,37 @@ public class ForwardedHeaderFilterTests { assertEquals("/", actual.getRequestURI()); } + @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(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + if (name.equals(header)) { + return super.getHeader(header); + } + } + return null; + } + }; + this.request.addHeader(X_FORWARDED_PREFIX, "/prefix"); + this.request.setRequestURI("/path"); + HttpServletRequest actual = filterAndGetWrappedRequest(); + + assertEquals("/prefix/path", actual.getRequestURI()); + } + @Test public void shouldFilter() throws Exception { testShouldFilter("Forwarded"); - testShouldFilter("X-Forwarded-Host"); - testShouldFilter("X-Forwarded-Port"); - testShouldFilter("X-Forwarded-Proto"); + testShouldFilter(X_FORWARDED_HOST); + testShouldFilter(X_FORWARDED_PORT); + testShouldFilter(X_FORWARDED_PROTO); } @Test @@ -134,9 +166,9 @@ public class ForwardedHeaderFilterTests { @Test public void forwardedRequest() throws Exception { this.request.setRequestURI("/mvc-showcase"); - this.request.addHeader("X-Forwarded-Proto", "https"); - this.request.addHeader("X-Forwarded-Host", "84.198.58.199"); - this.request.addHeader("X-Forwarded-Port", "443"); + this.request.addHeader(X_FORWARDED_PROTO, "https"); + this.request.addHeader(X_FORWARDED_HOST, "84.198.58.199"); + this.request.addHeader(X_FORWARDED_PORT, "443"); this.request.addHeader("foo", "bar"); this.filter.doFilter(this.request, new MockHttpServletResponse(), this.filterChain); @@ -148,15 +180,15 @@ public class ForwardedHeaderFilterTests { assertEquals(443, actual.getServerPort()); assertTrue(actual.isSecure()); - assertNull(actual.getHeader("X-Forwarded-Proto")); - assertNull(actual.getHeader("X-Forwarded-Host")); - assertNull(actual.getHeader("X-Forwarded-Port")); + assertNull(actual.getHeader(X_FORWARDED_PROTO)); + assertNull(actual.getHeader(X_FORWARDED_HOST)); + assertNull(actual.getHeader(X_FORWARDED_PORT)); assertEquals("bar", actual.getHeader("foo")); } @Test public void requestUriWithForwardedPrefix() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.request.addHeader(X_FORWARDED_PREFIX, "/prefix"); this.request.setRequestURI("/mvc-showcase"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -165,7 +197,7 @@ public class ForwardedHeaderFilterTests { @Test public void requestUriWithForwardedPrefixTrailingSlash() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/prefix/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/prefix/"); this.request.setRequestURI("/mvc-showcase"); HttpServletRequest actual = filterAndGetWrappedRequest(); @@ -174,7 +206,7 @@ public class ForwardedHeaderFilterTests { @Test public void contextPathWithForwardedPrefix() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.request.addHeader(X_FORWARDED_PREFIX, "/prefix"); this.request.setContextPath("/mvc-showcase"); String actual = filterAndGetContextPath(); @@ -183,7 +215,7 @@ public class ForwardedHeaderFilterTests { @Test public void contextPathWithForwardedPrefixTrailingSlash() throws Exception { - this.request.addHeader("X-Forwarded-Prefix", "/prefix/"); + this.request.addHeader(X_FORWARDED_PREFIX, "/prefix/"); this.request.setContextPath("/mvc-showcase"); String actual = filterAndGetContextPath(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java index 07abc64b45..852f5cd2dc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java @@ -16,6 +16,7 @@ package org.springframework.web.servlet.support; +import java.util.Enumeration; import javax.servlet.http.HttpServletRequest; import org.springframework.http.HttpRequest; @@ -133,8 +134,15 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder { } private static String prependForwardedPrefix(HttpServletRequest request, String path) { - String prefix = request.getHeader("X-Forwarded-Prefix"); - if (StringUtils.hasText(prefix)) { + String prefix = null; + Enumeration names = request.getHeaderNames(); + while (names.hasMoreElements()) { + String name = names.nextElement(); + if ("X-Forwarded-Prefix".equalsIgnoreCase(name)) { + prefix = request.getHeader(name); + } + } + if (prefix != null) { path = prefix + path; } return path; -- GitLab