Skip to content

Commit

Permalink
Check both https and wss in forwarded header checks
Browse files Browse the repository at this point in the history
Closes gh-27097
  • Loading branch information
rstoyanchev committed Jul 13, 2021
1 parent 6ec7cff commit e1f51cb
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 12 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 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.
Expand Down Expand Up @@ -239,7 +239,7 @@ private static class ForwardedHeaderExtractingRequest extends ForwardedHeaderRem
int port = uriComponents.getPort();

this.scheme = uriComponents.getScheme();
this.secure = "https".equals(this.scheme);
this.secure = "https".equals(this.scheme) || "wss".equals(this.scheme);
this.host = uriComponents.getHost();
this.port = (port == -1 ? (this.secure ? 443 : 80) : port);

Expand Down
Expand Up @@ -882,7 +882,7 @@ else if (isForwardedSslOn(headers)) {
}

if (this.scheme != null && ((this.scheme.equals("http") && "80".equals(this.port)) ||

This comment has been minimized.

Copy link
@jhyot

jhyot Jul 14, 2021

@rstoyanchev (I hope this works - have not used this GitHub feature so far).
Should here maybe be added a check for "ws" && port 80? "ws" is the counterpart to "wss" if not secure.

This comment has been minimized.

Copy link
@rstoyanchev

rstoyanchev Jul 16, 2021

Author Contributor

I've updated that. Thanks.

(this.scheme.equals("https") && "443".equals(this.port)))) {
((this.scheme.equals("https") || this.scheme.equals("wss")) && "443".equals(this.port)))) {
port(null);
}

Expand Down
Expand Up @@ -30,6 +30,8 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import org.springframework.web.testfixture.servlet.MockFilterChain;
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
Expand Down Expand Up @@ -102,10 +104,11 @@ public void shouldNotFilter() {
assertThat(this.filter.shouldNotFilter(new MockHttpServletRequest())).isTrue();
}

@Test
public void forwardedRequest() throws Exception {
@ParameterizedTest
@ValueSource(strings = {"https", "wss"})
public void forwardedRequest(String protocol) throws Exception {
this.request.setRequestURI("/mvc-showcase");
this.request.addHeader(X_FORWARDED_PROTO, "https");
this.request.addHeader(X_FORWARDED_PROTO, protocol);
this.request.addHeader(X_FORWARDED_HOST, "84.198.58.199");
this.request.addHeader(X_FORWARDED_PORT, "443");
this.request.addHeader("foo", "bar");
Expand All @@ -115,8 +118,8 @@ public void forwardedRequest() throws Exception {
HttpServletRequest actual = (HttpServletRequest) this.filterChain.getRequest();

assertThat(actual).isNotNull();
assertThat(actual.getRequestURL().toString()).isEqualTo("https://84.198.58.199/mvc-showcase");
assertThat(actual.getScheme()).isEqualTo("https");
assertThat(actual.getRequestURL().toString()).isEqualTo(protocol + "://84.198.58.199/mvc-showcase");
assertThat(actual.getScheme()).isEqualTo(protocol);
assertThat(actual.getServerName()).isEqualTo("84.198.58.199");
assertThat(actual.getServerPort()).isEqualTo(443);
assertThat(actual.isSecure()).isTrue();
Expand Down
Expand Up @@ -28,6 +28,8 @@
import java.util.function.BiConsumer;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpRequest;
Expand Down Expand Up @@ -374,10 +376,11 @@ void fromHttpRequest() {
assertThat(result.getQuery()).isEqualTo("a=1");
}

@Test // SPR-12771
void fromHttpRequestResetsPortBeforeSettingIt() {
@ParameterizedTest // gh-17368, gh-27097
@ValueSource(strings = {"https", "wss"})
void fromHttpRequestResetsPortBeforeSettingIt(String protocol) {
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("X-Forwarded-Proto", "https");
request.addHeader("X-Forwarded-Proto", protocol);
request.addHeader("X-Forwarded-Host", "84.198.58.199");
request.addHeader("X-Forwarded-Port", 443);
request.setScheme("http");
Expand All @@ -388,7 +391,7 @@ void fromHttpRequestResetsPortBeforeSettingIt() {
HttpRequest httpRequest = new ServletServerHttpRequest(request);
UriComponents result = UriComponentsBuilder.fromHttpRequest(httpRequest).build();

assertThat(result.getScheme()).isEqualTo("https");
assertThat(result.getScheme()).isEqualTo(protocol);
assertThat(result.getHost()).isEqualTo("84.198.58.199");
assertThat(result.getPort()).isEqualTo(-1);
assertThat(result.getPath()).isEqualTo("/rest/mobile/users/1");
Expand Down

0 comments on commit e1f51cb

Please sign in to comment.