Skip to content

Commit c326e44

Browse files
sdeleuzesdeleuze
committed
Refine forwarded protocol support
This commit refines forwarded protocol support in order to support proxies that only set "X-Forwarded-Proto" header and not "X-Forwarded-Port" by performing a reset of the port in such case. "Forwarded" header support has been updated accordingly since it also supports similar use case, as described in SPR-15504. Issue: SPR-16262
1 parent 30c0616 commit c326e44

File tree

6 files changed

+154
-12
lines changed

6 files changed

+154
-12
lines changed

‎spring-web/src/main/java/org/springframework/web/cors/reactive/CorsUtils.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ public static boolean isPreFlightRequest(ServerHttpRequest request) {
5151

5252
/**
5353
* Check if the request is a same-origin one, based on {@code Origin}, {@code Host},
54-
* {@code Forwarded} and {@code X-Forwarded-Host} headers.
54+
* {@code Forwarded}, {@code X-Forwarded-Proto}, {@code X-Forwarded-Host} and
55+
* @code X-Forwarded-Port} headers.
5556
* @return {@code true} if the request is a same-origin one, {@code false} in case
5657
* of cross-origin request.
5758
*/

‎spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
* @author Phillip Webb
5454
* @author Oliver Gierke
5555
* @author Brian Clozel
56+
* @author Sebastien Deleuze
5657
* @since 3.1
5758
* @see #newInstance()
5859
* @see #fromPath(String)
@@ -731,16 +732,23 @@ UriComponentsBuilder adaptFromForwardedHeaders(HttpHeaders headers) {
731732
String forwardedHeader = headers.getFirst("Forwarded");
732733
if (StringUtils.hasText(forwardedHeader)) {
733734
String forwardedToUse = StringUtils.tokenizeToStringArray(forwardedHeader, ",")[0];
734-
Matcher matcher = FORWARDED_HOST_PATTERN.matcher(forwardedToUse);
735+
Matcher matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
735736
if (matcher.find()) {
736-
adaptForwardedHost(matcher.group(1).trim());
737+
scheme(matcher.group(1).trim());
738+
port(null);
737739
}
738-
matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
740+
matcher = FORWARDED_HOST_PATTERN.matcher(forwardedToUse);
739741
if (matcher.find()) {
740-
scheme(matcher.group(1).trim());
742+
adaptForwardedHost(matcher.group(1).trim());
741743
}
742744
}
743745
else {
746+
String protocolHeader = headers.getFirst("X-Forwarded-Proto");
747+
if (StringUtils.hasText(protocolHeader)) {
748+
scheme(StringUtils.tokenizeToStringArray(protocolHeader, ",")[0]);
749+
port(null);
750+
}
751+
744752
String hostHeader = headers.getFirst("X-Forwarded-Host");
745753
if (StringUtils.hasText(hostHeader)) {
746754
adaptForwardedHost(StringUtils.tokenizeToStringArray(hostHeader, ",")[0]);
@@ -750,16 +758,11 @@ UriComponentsBuilder adaptFromForwardedHeaders(HttpHeaders headers) {
750758
if (StringUtils.hasText(portHeader)) {
751759
port(Integer.parseInt(StringUtils.tokenizeToStringArray(portHeader, ",")[0]));
752760
}
753-
754-
String protocolHeader = headers.getFirst("X-Forwarded-Proto");
755-
if (StringUtils.hasText(protocolHeader)) {
756-
scheme(StringUtils.tokenizeToStringArray(protocolHeader, ",")[0]);
757-
}
758761
}
759762

760763
if ((this.scheme != null) && ((this.scheme.equals("http") && "80".equals(this.port)) ||
761764
(this.scheme.equals("https") && "443".equals(this.port)))) {
762-
this.port = null;
765+
port(null);
763766
}
764767

765768
return this;

‎spring-web/src/main/java/org/springframework/web/util/WebUtils.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,8 @@ else if (CollectionUtils.isEmpty(allowedOrigins)) {
686686

687687
/**
688688
* Check if the request is a same-origin one, based on {@code Origin}, {@code Host},
689-
* {@code Forwarded} and {@code X-Forwarded-Host} headers.
689+
* {@code Forwarded}, {@code X-Forwarded-Proto}, {@code X-Forwarded-Host} and
690+
* @code X-Forwarded-Port} headers.
690691
* @return {@code true} if the request is a same-origin one, {@code false} in case
691692
* of cross-origin request
692693
* @since 4.2

‎spring-web/src/test/java/org/springframework/web/cors/reactive/CorsUtilsTests.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,54 @@ public void isNotPreFlightRequest() {
6666
assertFalse(CorsUtils.isPreFlightRequest(request));
6767
}
6868

69+
@Test // SPR-16262
70+
public void isSameOriginWithXForwardedHeaders() {
71+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", null, -1, "https://mydomain1.com"));
72+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", null, -1, "https://mydomain1.com"));
73+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", -1, "https://mydomain2.com"));
74+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", -1, "https://mydomain2.com"));
75+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
76+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
77+
}
78+
79+
@Test // SPR-16262
80+
public void isSameOriginWithForwardedHeader() {
81+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https", "https://mydomain1.com"));
82+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https", "https://mydomain1.com"));
83+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
84+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
85+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
86+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
87+
}
88+
89+
private boolean checkSameOriginWithXForwardedHeaders(String serverName, int port, String forwardedProto, String forwardedHost, int forwardedPort, String originHeader) {
90+
String url = "http://" + serverName;
91+
if (port != -1) {
92+
url = url + ":" + port;
93+
}
94+
MockServerHttpRequest.BaseBuilder<?> builder = get(url)
95+
.header(HttpHeaders.ORIGIN, originHeader);
96+
if (forwardedProto != null) {
97+
builder.header("X-Forwarded-Proto", forwardedProto);
98+
}
99+
if (forwardedHost != null) {
100+
builder.header("X-Forwarded-Host", forwardedHost);
101+
}
102+
if (forwardedPort != -1) {
103+
builder.header("X-Forwarded-Port", String.valueOf(forwardedPort));
104+
}
105+
return CorsUtils.isSameOrigin(builder.build());
106+
}
107+
108+
private boolean checkSameOriginWithForwardedHeader(String serverName, int port, String forwardedHeader, String originHeader) {
109+
String url = "http://" + serverName;
110+
if (port != -1) {
111+
url = url + ":" + port;
112+
}
113+
MockServerHttpRequest.BaseBuilder<?> builder = get(url)
114+
.header("Forwarded", forwardedHeader)
115+
.header(HttpHeaders.ORIGIN, originHeader);
116+
return CorsUtils.isSameOrigin(builder.build());
117+
}
118+
69119
}

‎spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,22 @@ public void fromHttpRequestWithForwardedHostWithDefaultPort() {
413413
assertEquals(-1, result.getPort());
414414
}
415415

416+
@Test // SPR-16262
417+
public void fromHttpRequestWithForwardedProtoWithDefaultPort() {
418+
MockHttpServletRequest request = new MockHttpServletRequest();
419+
request.setScheme("http");
420+
request.setServerName("example.org");
421+
request.setServerPort(10080);
422+
request.addHeader("X-Forwarded-Proto", "https");
423+
424+
HttpRequest httpRequest = new ServletServerHttpRequest(request);
425+
UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build();
426+
427+
assertEquals("https", result.getScheme());
428+
assertEquals("example.org", result.getHost());
429+
assertEquals(-1, result.getPort());
430+
}
431+
416432

417433
@Test
418434
public void fromHttpRequestWithForwardedHostWithForwardedScheme() {
@@ -865,4 +881,23 @@ public void fromHttpRequestForwardedHeaderWithoutHostPortAndWithServerPort() thr
865881
assertEquals(-1, result.getPort());
866882
assertEquals("https://84.198.58.199/rest/mobile/users/1", result.toUriString());
867883
}
884+
885+
@Test // SPR-16262
886+
public void fromHttpRequestForwardedHeaderWithProtoAndServerPort() throws Exception {
887+
MockHttpServletRequest request = new MockHttpServletRequest();
888+
request.addHeader("Forwarded", "proto=https");
889+
request.setScheme("http");
890+
request.setServerPort(8080);
891+
request.setServerName("example.com");
892+
request.setRequestURI("/rest/mobile/users/1");
893+
894+
HttpRequest httpRequest = new ServletServerHttpRequest(request);
895+
UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build();
896+
897+
assertEquals("https", result.getScheme());
898+
assertEquals("example.com", result.getHost());
899+
assertEquals("/rest/mobile/users/1", result.getPath());
900+
assertEquals(-1, result.getPort());
901+
assertEquals("https://example.com/rest/mobile/users/1", result.toUriString());
902+
}
868903
}

‎spring-web/src/test/java/org/springframework/web/util/WebUtilsTests.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,26 @@ public void isSameOrigin() {
140140
"http://[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8080"));
141141
}
142142

143+
@Test // SPR-16262
144+
public void isSameOriginWithXForwardedHeaders() {
145+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", null, -1, "https://mydomain1.com"));
146+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", null, -1, "https://mydomain1.com"));
147+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", -1, "https://mydomain2.com"));
148+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", -1, "https://mydomain2.com"));
149+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
150+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
151+
}
152+
153+
@Test // SPR-16262
154+
public void isSameOriginWithForwardedHeader() {
155+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https", "https://mydomain1.com"));
156+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https", "https://mydomain1.com"));
157+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
158+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
159+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
160+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
161+
}
162+
143163

144164
private boolean checkValidOrigin(String serverName, int port, String originHeader, List<String> allowed) {
145165
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
@@ -163,4 +183,36 @@ private boolean checkSameOrigin(String serverName, int port, String originHeader
163183
return WebUtils.isSameOrigin(request);
164184
}
165185

186+
private boolean checkSameOriginWithXForwardedHeaders(String serverName, int port, String forwardedProto, String forwardedHost, int forwardedPort, String originHeader) {
187+
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
188+
ServerHttpRequest request = new ServletServerHttpRequest(servletRequest);
189+
servletRequest.setServerName(serverName);
190+
if (port != -1) {
191+
servletRequest.setServerPort(port);
192+
}
193+
if (forwardedProto != null) {
194+
request.getHeaders().set("X-Forwarded-Proto", forwardedProto);
195+
}
196+
if (forwardedHost != null) {
197+
request.getHeaders().set("X-Forwarded-Host", forwardedHost);
198+
}
199+
if (forwardedPort != -1) {
200+
request.getHeaders().set("X-Forwarded-Port", String.valueOf(forwardedPort));
201+
}
202+
request.getHeaders().set(HttpHeaders.ORIGIN, originHeader);
203+
return WebUtils.isSameOrigin(request);
204+
}
205+
206+
private boolean checkSameOriginWithForwardedHeader(String serverName, int port, String forwardedHeader, String originHeader) {
207+
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
208+
ServerHttpRequest request = new ServletServerHttpRequest(servletRequest);
209+
servletRequest.setServerName(serverName);
210+
if (port != -1) {
211+
servletRequest.setServerPort(port);
212+
}
213+
request.getHeaders().set("Forwarded", forwardedHeader);
214+
request.getHeaders().set(HttpHeaders.ORIGIN, originHeader);
215+
return WebUtils.isSameOrigin(request);
216+
}
217+
166218
}

0 commit comments

Comments
 (0)