Skip to content

Commit 1b3b058

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 8aa94ae commit 1b3b058

File tree

4 files changed

+104
-13
lines changed

4 files changed

+104
-13
lines changed

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

+15-12
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
* @author Phillip Webb
5353
* @author Oliver Gierke
5454
* @author Brian Clozel
55+
* @author Sebastien Deleuze
5556
* @since 3.1
5657
* @see #newInstance()
5758
* @see #fromPath(String)
@@ -675,16 +676,23 @@ UriComponentsBuilder adaptFromForwardedHeaders(HttpHeaders headers) {
675676
String forwardedHeader = headers.getFirst("Forwarded");
676677
if (StringUtils.hasText(forwardedHeader)) {
677678
String forwardedToUse = StringUtils.tokenizeToStringArray(forwardedHeader, ",")[0];
678-
Matcher matcher = FORWARDED_HOST_PATTERN.matcher(forwardedToUse);
679+
Matcher matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
679680
if (matcher.find()) {
680-
adaptForwardedHost(matcher.group(1).trim());
681+
scheme(matcher.group(1).trim());
682+
port(null);
681683
}
682-
matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
684+
matcher = FORWARDED_HOST_PATTERN.matcher(forwardedToUse);
683685
if (matcher.find()) {
684-
scheme(matcher.group(1).trim());
686+
adaptForwardedHost(matcher.group(1).trim());
685687
}
686688
}
687689
else {
690+
String protocolHeader = headers.getFirst("X-Forwarded-Proto");
691+
if (StringUtils.hasText(protocolHeader)) {
692+
scheme(StringUtils.tokenizeToStringArray(protocolHeader, ",")[0]);
693+
port(null);
694+
}
695+
688696
String hostHeader = headers.getFirst("X-Forwarded-Host");
689697
if (StringUtils.hasText(hostHeader)) {
690698
adaptForwardedHost(StringUtils.tokenizeToStringArray(hostHeader, ",")[0]);
@@ -694,16 +702,11 @@ UriComponentsBuilder adaptFromForwardedHeaders(HttpHeaders headers) {
694702
if (StringUtils.hasText(portHeader)) {
695703
port(Integer.parseInt(StringUtils.tokenizeToStringArray(portHeader, ",")[0]));
696704
}
697-
698-
String protocolHeader = headers.getFirst("X-Forwarded-Proto");
699-
if (StringUtils.hasText(protocolHeader)) {
700-
scheme(StringUtils.tokenizeToStringArray(protocolHeader, ",")[0]);
701-
}
702705
}
703706

704-
if ((this.scheme.equals("http") && "80".equals(this.port)) ||
705-
(this.scheme.equals("https") && "443".equals(this.port))) {
706-
this.port = null;
707+
if ((this.scheme != null) && ((this.scheme.equals("http") && "80".equals(this.port)) ||
708+
(this.scheme.equals("https") && "443".equals(this.port)))) {
709+
port(null);
707710
}
708711

709712
return this;

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,8 @@ else if (CollectionUtils.isEmpty(allowedOrigins)) {
812812

813813
/**
814814
* Check if the request is a same-origin one, based on {@code Origin}, {@code Host},
815-
* {@code Forwarded} and {@code X-Forwarded-Host} headers.
815+
* {@code Forwarded}, {@code X-Forwarded-Proto}, {@code X-Forwarded-Host} and
816+
* @code X-Forwarded-Port} headers.
816817
* @return {@code true} if the request is a same-origin one, {@code false} in case
817818
* of cross-origin request
818819
* @since 4.2

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

+35
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,22 @@ public void fromHttpRequestWithForwardedHostWithDefaultPort() {
401401
assertEquals(-1, result.getPort());
402402
}
403403

404+
@Test // SPR-16262
405+
public void fromHttpRequestWithForwardedProtoWithDefaultPort() {
406+
MockHttpServletRequest request = new MockHttpServletRequest();
407+
request.setScheme("http");
408+
request.setServerName("example.org");
409+
request.setServerPort(10080);
410+
request.addHeader("X-Forwarded-Proto", "https");
411+
412+
HttpRequest httpRequest = new ServletServerHttpRequest(request);
413+
UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build();
414+
415+
assertEquals("https", result.getScheme());
416+
assertEquals("example.org", result.getHost());
417+
assertEquals(-1, result.getPort());
418+
}
419+
404420

405421
@Test
406422
public void fromHttpRequestWithForwardedHostWithForwardedScheme() {
@@ -837,4 +853,23 @@ public void fromHttpRequestForwardedHeaderWithoutHostPortAndWithServerPort() thr
837853
assertEquals(-1, result.getPort());
838854
assertEquals("https://84.198.58.199/rest/mobile/users/1", result.toUriString());
839855
}
856+
857+
@Test // SPR-16262
858+
public void fromHttpRequestForwardedHeaderWithProtoAndServerPort() throws Exception {
859+
MockHttpServletRequest request = new MockHttpServletRequest();
860+
request.addHeader("Forwarded", "proto=https");
861+
request.setScheme("http");
862+
request.setServerPort(8080);
863+
request.setServerName("example.com");
864+
request.setRequestURI("/rest/mobile/users/1");
865+
866+
HttpRequest httpRequest = new ServletServerHttpRequest(request);
867+
UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build();
868+
869+
assertEquals("https", result.getScheme());
870+
assertEquals("example.com", result.getHost());
871+
assertEquals("/rest/mobile/users/1", result.getPath());
872+
assertEquals(-1, result.getPort());
873+
assertEquals("https://example.com/rest/mobile/users/1", result.toUriString());
874+
}
840875
}

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

+52
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,26 @@ public void isSameOrigin() {
160160
assertFalse(checkSameOrigin("[::1]", 8080, "http://[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8080"));
161161
}
162162

163+
@Test // SPR-16262
164+
public void isSameOriginWithXForwardedHeaders() {
165+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", null, -1, "https://mydomain1.com"));
166+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", null, -1, "https://mydomain1.com"));
167+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", -1, "https://mydomain2.com"));
168+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", -1, "https://mydomain2.com"));
169+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
170+
assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
171+
}
172+
173+
@Test // SPR-16262
174+
public void isSameOriginWithForwardedHeader() {
175+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https", "https://mydomain1.com"));
176+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https", "https://mydomain1.com"));
177+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
178+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
179+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
180+
assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
181+
}
182+
163183

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

206+
private boolean checkSameOriginWithXForwardedHeaders(String serverName, int port, String forwardedProto, String forwardedHost, int forwardedPort, 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+
if (forwardedProto != null) {
214+
request.getHeaders().set("X-Forwarded-Proto", forwardedProto);
215+
}
216+
if (forwardedHost != null) {
217+
request.getHeaders().set("X-Forwarded-Host", forwardedHost);
218+
}
219+
if (forwardedPort != -1) {
220+
request.getHeaders().set("X-Forwarded-Port", String.valueOf(forwardedPort));
221+
}
222+
request.getHeaders().set(HttpHeaders.ORIGIN, originHeader);
223+
return WebUtils.isSameOrigin(request);
224+
}
225+
226+
private boolean checkSameOriginWithForwardedHeader(String serverName, int port, String forwardedHeader, String originHeader) {
227+
MockHttpServletRequest servletRequest = new MockHttpServletRequest();
228+
ServerHttpRequest request = new ServletServerHttpRequest(servletRequest);
229+
servletRequest.setServerName(serverName);
230+
if (port != -1) {
231+
servletRequest.setServerPort(port);
232+
}
233+
request.getHeaders().set("Forwarded", forwardedHeader);
234+
request.getHeaders().set(HttpHeaders.ORIGIN, originHeader);
235+
return WebUtils.isSameOrigin(request);
236+
}
237+
186238
}

0 commit comments

Comments
 (0)