From 6615e9c5efec3c72056e9c2ad02e217e08db51a5 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 18 Jun 2020 13:12:07 +0200 Subject: [PATCH] Support multi-value X-Forwarded-Prefix headers Prior to this commit, the Forwarded headers for Spring MVC and Spring WebFlux did not support multiple prefix values for the `"X-Forwarded-Prefix"` HTTP header. This commit splits and processes multiple prefixes defined in the dedicated header. Closes gh-25254 --- .../web/filter/ForwardedHeaderFilter.java | 13 +++++-- .../adapter/ForwardedHeaderTransformer.java | 22 +++++++---- .../filter/ForwardedHeaderFilterTests.java | 24 +++++++++++- .../ForwardedHeaderTransformerTests.java | 39 ++++++++++++++----- 4 files changed, 77 insertions(+), 21 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 f797d064dd4..8555446aa7e 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 @@ -343,11 +343,18 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } } if (result != null) { - while (result.endsWith("/")) { - result = result.substring(0, result.length() - 1); + StringBuilder prefix = new StringBuilder(result.length()); + String[] rawPrefixes = StringUtils.tokenizeToStringArray(result, ","); + for (String rawPrefix : rawPrefixes) { + int endIndex = rawPrefix.length(); + while (endIndex > 0 && rawPrefix.charAt(endIndex - 1) == '/') { + endIndex--; + } + prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix)); } + return prefix.toString(); } - return result; + return null; } @Nullable diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java b/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java index b3990166cb1..17d2cf00c8d 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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,6 +27,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; import org.springframework.util.LinkedCaseInsensitiveMap; +import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; /** @@ -128,15 +129,20 @@ public class ForwardedHeaderTransformer implements Function 1 && prefix.charAt(endIndex - 1) == '/') { - endIndex--; + String header = headers.getFirst("X-Forwarded-Prefix"); + if (header != null) { + StringBuilder prefix = new StringBuilder(header.length()); + String[] rawPrefixes = StringUtils.tokenizeToStringArray(header, ","); + for (String rawPrefix : rawPrefixes) { + int endIndex = rawPrefix.length(); + while (endIndex > 1 && rawPrefix.charAt(endIndex - 1) == '/') { + endIndex--; + } + prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix)); } - prefix = (endIndex != prefix.length() ? prefix.substring(0, endIndex) : prefix); + return prefix.toString(); } - return prefix; + return header; } } 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 ca073a04985..be753121d52 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-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -47,9 +47,13 @@ import static org.mockito.Mockito.mock; 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 static final String X_FORWARDED_SSL = "x-forwarded-ssl"; @@ -350,6 +354,24 @@ public class ForwardedHeaderFilterTests { assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost/prefix/mvc-showcase"); } + @Test + void shouldConcatenatePrefixes() throws Exception { + this.request.addHeader(X_FORWARDED_PREFIX, "/first,/second"); + this.request.setRequestURI("/mvc-showcase"); + + HttpServletRequest actual = filterAndGetWrappedRequest(); + assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost/first/second/mvc-showcase"); + } + + @Test + void shouldConcatenatePrefixesWithTrailingSlashes() throws Exception { + this.request.addHeader(X_FORWARDED_PREFIX, "/first/,/second//"); + this.request.setRequestURI("/mvc-showcase"); + + HttpServletRequest actual = filterAndGetWrappedRequest(); + assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost/first/second/mvc-showcase"); + } + @Test public void requestURLNewStringBuffer() throws Exception { this.request.addHeader(X_FORWARDED_PREFIX, "/prefix/"); diff --git a/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java b/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java index 25d27be12fe..a2b235432ae 100644 --- a/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -40,8 +40,7 @@ public class ForwardedHeaderTransformerTests { @Test - public void removeOnly() { - + void removeOnly() { this.requestMutator.setRemoveOnly(true); HttpHeaders headers = new HttpHeaders(); @@ -57,7 +56,7 @@ public class ForwardedHeaderTransformerTests { } @Test - public void xForwardedHeaders() throws Exception { + void xForwardedHeaders() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("X-Forwarded-Host", "84.198.58.199"); headers.add("X-Forwarded-Port", "443"); @@ -70,7 +69,7 @@ public class ForwardedHeaderTransformerTests { } @Test - public void forwardedHeader() throws Exception { + void forwardedHeader() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("Forwarded", "host=84.198.58.199;proto=https"); ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); @@ -80,7 +79,7 @@ public class ForwardedHeaderTransformerTests { } @Test - public void xForwardedPrefix() throws Exception { + void xForwardedPrefix() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("X-Forwarded-Prefix", "/prefix"); ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); @@ -91,7 +90,7 @@ public class ForwardedHeaderTransformerTests { } @Test // gh-23305 - public void xForwardedPrefixShouldNotLeadToDecodedPath() throws Exception { + void xForwardedPrefixShouldNotLeadToDecodedPath() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("X-Forwarded-Prefix", "/prefix"); ServerHttpRequest request = MockServerHttpRequest @@ -107,7 +106,7 @@ public class ForwardedHeaderTransformerTests { } @Test - public void xForwardedPrefixTrailingSlash() throws Exception { + void xForwardedPrefixTrailingSlash() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("X-Forwarded-Prefix", "/prefix////"); ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); @@ -118,7 +117,7 @@ public class ForwardedHeaderTransformerTests { } @Test // SPR-17525 - public void shouldNotDoubleEncode() throws Exception { + void shouldNotDoubleEncode() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.add("Forwarded", "host=84.198.58.199;proto=https"); @@ -133,6 +132,28 @@ public class ForwardedHeaderTransformerTests { assertForwardedHeadersRemoved(request); } + @Test + void shouldConcatenatePrefixes() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Forwarded-Prefix", "/first,/second"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertThat(request.getURI()).isEqualTo(new URI("https://example.com/first/second/path")); + assertThat(request.getPath().value()).isEqualTo("/first/second/path"); + assertForwardedHeadersRemoved(request); + } + + @Test + void shouldConcatenatePrefixesWithTrailingSlashes() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Forwarded-Prefix", "/first/,/second//"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertThat(request.getURI()).isEqualTo(new URI("https://example.com/first/second/path")); + assertThat(request.getPath().value()).isEqualTo("/first/second/path"); + assertForwardedHeadersRemoved(request); + } + private MockServerHttpRequest getRequest(HttpHeaders headers) { return MockServerHttpRequest.get(BASE_URL).headers(headers).build();