From ea5ff87f8ec8d34871aaf3bcc49a165e2db076e5 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 6 Oct 2016 15:36:48 +0200 Subject: [PATCH] Fix NumberFormatException with X-Forwarded-Host This commit fixes `NumberFormatException`s that were thrown when parsing IPv6 host values in `X-Forwarded-Host` request headers. Issue: SPR-14761 --- .../web/util/UriComponentsBuilder.java | 9 +- .../web/util/UriComponentsBuilderTests.java | 145 +++++++++++------- 2 files changed, 92 insertions(+), 62 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index 36392537615..98596580adf 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -51,6 +51,7 @@ import org.springframework.web.util.HierarchicalUriComponents.PathComponent; * @author Rossen Stoyanchev * @author Phillip Webb * @author Oliver Gierke + * @author Brian Clozel * @since 3.1 * @see #newInstance() * @see #fromPath(String) @@ -687,10 +688,10 @@ public class UriComponentsBuilder implements Cloneable { String hostHeader = headers.getFirst("X-Forwarded-Host"); if (StringUtils.hasText(hostHeader)) { String hostToUse = StringUtils.tokenizeToStringArray(hostHeader, ",")[0]; - String[] hostAndPort = StringUtils.split(hostToUse, ":"); - if (hostAndPort != null) { - host(hostAndPort[0]); - port(Integer.parseInt(hostAndPort[1])); + int portSeparatorIdx = hostToUse.lastIndexOf(":"); + if (portSeparatorIdx > hostToUse.lastIndexOf("]")) { + host(hostToUse.substring(0, portSeparatorIdx)); + port(Integer.parseInt(hostToUse.substring(portSeparatorIdx + 1))); } else { host(hostToUse); diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index 3ed08765cb0..5c6731df1ee 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -125,9 +125,7 @@ public class UriComponentsBuilderTests { assertEquals("Invalid result URI", uri, result.toUri()); } - // SPR-9317 - - @Test + @Test // SPR-9317 public void fromUriEncodedQuery() throws URISyntaxException { URI uri = new URI("http://www.example.org/?param=aGVsbG9Xb3JsZA%3D%3D"); String fromUri = UriComponentsBuilder.fromUri(uri).build().getQueryParams().get("param").get(0); @@ -182,9 +180,7 @@ public class UriComponentsBuilderTests { assertEquals("28", result.getFragment()); } - // SPR-9832 - - @Test + @Test // SPR-9832 public void fromUriStringQueryParamWithReservedCharInValue() throws URISyntaxException { String uri = "http://www.google.com/ig/calculator?q=1USD=?EUR"; UriComponents result = UriComponentsBuilder.fromUriString(uri).build(); @@ -193,41 +189,35 @@ public class UriComponentsBuilderTests { assertEquals("1USD=?EUR", result.getQueryParams().getFirst("q")); } - // SPR-10779 - - @Test + @Test // SPR-10779 public void fromHttpUrlStringCaseInsesitiveScheme() { assertEquals("http", UriComponentsBuilder.fromHttpUrl("HTTP://www.google.com").build().getScheme()); assertEquals("https", UriComponentsBuilder.fromHttpUrl("HTTPS://www.google.com").build().getScheme()); } - // SPR-10539 - @Test(expected = IllegalArgumentException.class) + + @Test(expected = IllegalArgumentException.class) // SPR-10539 public void fromHttpUrlStringInvalidIPv6Host() throws URISyntaxException { UriComponentsBuilder.fromHttpUrl("http://[1abc:2abc:3abc::5ABC:6abc:8080/resource").build().encode(); } - // SPR-10539 - - @Test + @Test // SPR-10539 public void fromUriStringIPv6Host() throws URISyntaxException { - UriComponents result = UriComponentsBuilder - .fromUriString("http://[1abc:2abc:3abc::5ABC:6abc]:8080/resource").build().encode(); - assertEquals("[1abc:2abc:3abc::5ABC:6abc]", result.getHost()); + UriComponents result = UriComponentsBuilder + .fromUriString("http://[1abc:2abc:3abc::5ABC:6abc]:8080/resource").build().encode(); + assertEquals("[1abc:2abc:3abc::5ABC:6abc]", result.getHost()); - UriComponents resultWithScopeId = UriComponentsBuilder - .fromUriString("http://[1abc:2abc:3abc::5ABC:6abc%eth0]:8080/resource").build().encode(); + UriComponents resultWithScopeId = UriComponentsBuilder + .fromUriString("http://[1abc:2abc:3abc::5ABC:6abc%eth0]:8080/resource").build().encode(); assertEquals("[1abc:2abc:3abc::5ABC:6abc%25eth0]", resultWithScopeId.getHost()); - UriComponents resultIPv4compatible = UriComponentsBuilder - .fromUriString("http://[::192.168.1.1]:8080/resource").build().encode(); + UriComponents resultIPv4compatible = UriComponentsBuilder + .fromUriString("http://[::192.168.1.1]:8080/resource").build().encode(); assertEquals("[::192.168.1.1]", resultIPv4compatible.getHost()); } - // SPR-11970 - - @Test + @Test // SPR-11970 public void fromUriStringNoPathWithReservedCharInQuery() { UriComponents result = UriComponentsBuilder.fromUriString("http://example.com?foo=bar@baz").build(); assertTrue(StringUtils.isEmpty(result.getUserInfo())); @@ -253,9 +243,7 @@ public class UriComponentsBuilderTests { assertEquals("a=1", result.getQuery()); } - // SPR-12771 - - @Test + @Test // SPR-12771 public void fromHttpRequestResetsPortBeforeSettingIt() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("X-Forwarded-Proto", "https"); @@ -275,6 +263,67 @@ public class UriComponentsBuilderTests { assertEquals("/rest/mobile/users/1", result.getPath()); } + @Test //SPR-14761 + public void fromHttpRequestWithForwardedIPv4Host() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("http"); + request.setServerName("localhost"); + request.setServerPort(-1); + request.setRequestURI("/mvc-showcase"); + request.addHeader("Forwarded", "host=192.168.0.1"); + + HttpRequest httpRequest = new ServletServerHttpRequest(request); + UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build(); + + assertEquals("http://192.168.0.1/mvc-showcase", result.toString()); + } + + @Test //SPR-14761 + public void fromHttpRequestWithForwardedIPv6() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("http"); + request.setServerName("localhost"); + request.setServerPort(-1); + request.setRequestURI("/mvc-showcase"); + request.addHeader("Forwarded", "host=[1abc:2abc:3abc::5ABC:6abc]"); + + HttpRequest httpRequest = new ServletServerHttpRequest(request); + UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build(); + + assertEquals("http://[1abc:2abc:3abc::5ABC:6abc]/mvc-showcase", result.toString()); + } + + @Test //SPR-14761 + public void fromHttpRequestWithForwardedIPv6Host() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("http"); + request.setServerName("localhost"); + request.setServerPort(-1); + request.setRequestURI("/mvc-showcase"); + request.addHeader("X-Forwarded-Host", "[1abc:2abc:3abc::5ABC:6abc]"); + + HttpRequest httpRequest = new ServletServerHttpRequest(request); + UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build(); + + assertEquals("http://[1abc:2abc:3abc::5ABC:6abc]/mvc-showcase", result.toString()); + } + + @Test //SPR-14761 + public void fromHttpRequestWithForwardedIPv6HostAndPort() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("http"); + request.setServerName("localhost"); + request.setServerPort(-1); + request.setRequestURI("/mvc-showcase"); + request.addHeader("X-Forwarded-Host", "[1abc:2abc:3abc::5ABC:6abc]:8080"); + + HttpRequest httpRequest = new ServletServerHttpRequest(request); + UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build(); + + assertEquals("http://[1abc:2abc:3abc::5ABC:6abc]:8080/mvc-showcase", result.toString()); + } + + @Test public void fromHttpRequestWithForwardedHost() { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -290,9 +339,7 @@ public class UriComponentsBuilderTests { assertEquals("http://anotherHost/mvc-showcase", result.toString()); } - // SPR-10701 - - @Test + @Test // SPR-10701 public void fromHttpRequestWithForwardedHostIncludingPort() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -308,9 +355,7 @@ public class UriComponentsBuilderTests { assertEquals(443, result.getPort()); } - // SPR-11140 - - @Test + @Test // SPR-11140 public void fromHttpRequestWithForwardedHostMultiValuedHeader() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -325,9 +370,7 @@ public class UriComponentsBuilderTests { assertEquals(-1, result.getPort()); } - // SPR-11855 - - @Test + @Test // SPR-11855 public void fromHttpRequestWithForwardedHostAndPort() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -343,9 +386,7 @@ public class UriComponentsBuilderTests { assertEquals(9090, result.getPort()); } - // SPR-11872 - - @Test + @Test // SPR-11872 public void fromHttpRequestWithForwardedHostWithDefaultPort() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -378,9 +419,7 @@ public class UriComponentsBuilderTests { assertEquals(-1, result.getPort()); } - // SPR-12771 - - @Test + @Test // SPR-12771 public void fromHttpRequestWithForwardedProtoAndDefaultPort() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -397,9 +436,7 @@ public class UriComponentsBuilderTests { assertEquals("https://84.198.58.199/mvc-showcase", result.toString()); } - // SPR-12813 - - @Test + @Test // SPR-12813 public void fromHttpRequestWithForwardedPortMultiValueHeader() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -415,9 +452,7 @@ public class UriComponentsBuilderTests { assertEquals("http://a.example.org/mvc-showcase", result.toString()); } - // SPR-12816 - - @Test + @Test // SPR-12816 public void fromHttpRequestWithForwardedProtoMultiValueHeader() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setScheme("http"); @@ -434,9 +469,7 @@ public class UriComponentsBuilderTests { assertEquals("https://a.example.org/mvc-showcase", result.toString()); } - // SPR-12742 - - @Test + @Test // SPR-12742 public void fromHttpRequestWithTrailingSlash() throws Exception { UriComponents before = UriComponentsBuilder.fromPath("/foo/").build(); UriComponents after = UriComponentsBuilder.newInstance().uriComponents(before).build(); @@ -506,9 +539,7 @@ public class UriComponentsBuilderTests { assertEquals(Arrays.asList("foo", "bar"), result.getPathSegments()); } - // SPR-12398 - - @Test + @Test // SPR-12398 public void pathWithDuplicateSlashes() throws URISyntaxException { UriComponents uriComponents = UriComponentsBuilder.fromPath("/foo/////////bar").build(); assertEquals("/foo/bar", uriComponents.getPath()); @@ -645,7 +676,7 @@ public class UriComponentsBuilderTests { @Test public void emptySegments() throws Exception { assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").path("/x/y/z").build().toString(), equalTo("http://example.com/abc/x/y/z")); - assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").pathSegment("x", "y", "z").build().toString(), equalTo("http://example.com/abc/x/y/z")); + assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").pathSegment("x", "y", "z").build().toString(), equalTo("http://example.com/abc/x/y/z")); assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").path("/x/").path("/y/z").build().toString(), equalTo("http://example.com/abc/x/y/z")); assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").pathSegment("x").path("y").build().toString(), equalTo("http://example.com/abc/x/y")); } @@ -686,9 +717,7 @@ public class UriComponentsBuilderTests { assertEquals("f2", result2.getFragment()); } - // SPR-11856 - - @Test + @Test // SPR-11856 public void fromHttpRequestForwardedHeader() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Forwarded", "proto=https; host=84.198.58.199");