Skip to content

Commit

Permalink
fix host predicate use host of InetSocketAddress insted of headers
Browse files Browse the repository at this point in the history 
Fixes gh-3037
  • Loading branch information
@ashraf-revo @spencergibb
ashraf-revo authored and spencergibb committed Dec 6, 2023
1 parent 8263985 commit a5da112
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 13 deletions.
30 changes: 17 additions & 13 deletions ...n/java/org/springframework/cloud/gateway/handler/predicate/HostRoutePredicateFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.cloud.gateway.handler.predicate;

import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -59,22 +60,25 @@ public Predicate<ServerWebExchange> apply(Config config) {
return new GatewayPredicate() {
@Override
public boolean test(ServerWebExchange exchange) {
String host = exchange.getRequest().getHeaders().getFirst("Host");
String match = null;
for (int i = 0; i < config.getPatterns().size(); i++) {
String pattern = config.getPatterns().get(i);
if (pathMatcher.match(pattern, host)) {
match = pattern;
break;
InetSocketAddress address = exchange.getRequest().getHeaders().getHost();
if (address != null) {
String match = null;
String host = address.getHostName();
for (int i = 0; i < config.getPatterns().size(); i++) {
String pattern = config.getPatterns().get(i);
if (pathMatcher.match(pattern, host)) {
match = pattern;
break;
}
}
}

if (match != null) {
Map<String, String> variables = pathMatcher.extractUriTemplateVariables(match, host);
ServerWebExchangeUtils.putUriTemplateVariables(exchange, variables);
return true;
}
if (match != null) {
Map<String, String> variables = pathMatcher.extractUriTemplateVariables(match, host);
ServerWebExchangeUtils.putUriTemplateVariables(exchange, variables);
return true;
}

}
return false;
}

Expand Down
5 changes: 5 additions & 0 deletions ...a/org/springframework/cloud/gateway/handler/predicate/HostRoutePredicateFactoryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public void mulitHostRouteDslWorks() {
expectHostRoute("www.hostmultidsl2.org", "host_multi_dsl");
}

@Test
public void sameHostWithPort() {
expectHostRoute("hostpatternarg.org:8080", "without_pattern");
}

@Test
public void toStringFormat() {
Config config = new Config().setPatterns(Arrays.asList("pattern1", "pattern2"));
Expand Down
9 changes: 9 additions & 0 deletions spring-cloud-gateway-server/src/test/resources/application.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ spring:
predicates:
- Header=Foo, .*


# =====================================
- id: without_pattern
uri: ${test.uri}
predicates:
- name: Host
args:
pattern: 'hostpatternarg.org'

# =====================================
- id: host_backwards_compatible_test
uri: ${test.uri}
Expand Down

4 comments on commit a5da112

@aaHomeOfficeGit
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this security oriented fix, but this did break the compatibility with the older versions of SCG.
The issue is that the "Host" header includes the ":" if it is non standard port, and the match condition demanded that the route predicate config includes the port number. (This is only a problem if you are running the SCG on a non default port.)

With the inet class the match condition only requires the host name, and old predicate config which includes the port do no longer match. Hence: lost config compatibility.

I know that the inclusion of the port was a special thing for SCG, other rev proxies do not match on port, and the port could be a redundant info in most cases, although if the SCG is behind another rev proxy then that may not be the case. Regardless, I think the backward compatibility should have been maintained with older versions. May be a global config option, or some other claver way?

Thanks AA

@spencergibb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaHomeOfficeGit will you open a new issue for me?

@spencergibb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, how did it break backwards compatibility?

@aaHomeOfficeGit
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @spencergibb, we have a system which runs on a non standard port 9443.
When the client makes a call the Http header "Host" has the value of "some.fqdn.com:9443".

Also for arch reasons, in the SCG config we need to filter by fqdn, so we had a host predicate with "Host=some.fqdn.com:9443,other.fqdn.com:9443" value.

After the upgrade to 4.1.0 the routs were not matching. The fix was to change the host predicate to:
"Host=some.fqdn.com,other.fqdn.com"

It should be possible to reproduce, we were able to reproduce it on a dev's box by running the scg on 8089 port ... the old scg needs the ports in the predicate value, the new one does not work with the ports, only without ports.
(Actually we now have both with and without port the fqdns in the predicate value, this way we have achieved some version compatibility, but it is a bit of an efficiency hit...not much, may be 1us or so)

Please sign in to comment.