-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prior to 4.1.0 host predicate match needed port number, now if port is present the match does not match, lost config compatibility. #3190
Comments
In our env we are running SCG on non standard ports like 9443 or 8089. When the client makes a call the HTTP header Host has the value which includes the non-standard port number like fqdn:9443 Until 4.1.0 scg version the host predicate definition had to include the comma separated list of fqdn1:9443,fqdn2:9443. In a simple case the port number matching may make no sense, but there are cases when let say a https offloading rev proxy is shielding the scg, and that rev proxy may even have listeners on multiple ports forwarding to the same scg. Let say the scg is listening on let say a 8090 port, but this port will never been seen by a client. In this case the scg may want to have a route which matches to a specific Host header's port. E.g fqdn:9443->service1.v1, fqdn:10443->service1.v2. May be a new predicate would be better for Port on Host http header? ... except the Host header may not always have port number (I would need to check the specs) |
The https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html has a clarification on when the :port is part of the host header... the optional of port part of the host value is also in https://www.rfc-editor.org/rfc/rfc9110.html#name-host-and-authority section 7.2 hence it is clear the compatibility issue when switching from host header value match to first converting the host header to inetaddress and then matching on the getHostName one more issue: using the inetAddress.getHostName() will try to do a rev dns lookup if the host header provided an ip address (http://10.0.0.22:8080/...) ..may be a better option is to use getHostString() which does not result in a ip reverse lookup. |
Thanks for the detailed analysis. The property /cc @ashraf-revo |
Thanks @spencergibb for the quick resolution! Happy Holidays! |
The a5da112#commitcomment-135442096 fix,
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.
More details: 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"
Workaround: add both with and without port the fqdns in the predicate value, this way we have achieved some version compatibility. (I mean same config works on earlier and also 4.1.0 scg version)
The text was updated successfully, but these errors were encountered: