getRemoteAddress: Remove optional port number #18861
Conversation
| @@ -659,6 +659,8 @@ public function getRemoteAddress(): string { | |||
| if(isset($this->server[$header])) { | |||
| foreach(explode(',', $this->server[$header]) as $IP) { | |||
| $IP = trim($IP); | |||
| // Use IP only, remove the optional port (see RFC 7239, Section 5.3 and RFC 7230, Section 5.4) | |||
| $IP = explode(':', $IP)[0]; | |||
There was a problem hiding this comment.
I guess that will fail for ipv6.
There was a problem hiding this comment.
Good point. I've currently no system to test IPv6.
There was a problem hiding this comment.
According to https://tools.ietf.org/html/rfc5952#section-6 there are multiple formats, I'm not sure which variant is used by IIS. Is there any helper to check if the IP is an IPv6?
There was a problem hiding this comment.
A regex would be possible, but if you take all possibilities into account it might get slow.
There was a problem hiding this comment.
Is there any helper to check if the IP is an IPv6?
filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) will return false if $ip is not a ip v6. But there are some pitfalls: https://3v4l.org/ZG7qa
The `HTTP_X_FORWARED_FOR` header might contain the IP address and the port number, when used via an IIS proxy. See https://serverfault.com/a/757994 As our setup requires the use of IIS as an incoming SSL proxy, this change allows the remote IP to be correctly detected. In my opinion, as the port number is allowed according to the linked RFCs, NC should handle this case. Signed-off-by: Tommy Sparber <tommy@sparber.net>
|
I've fixed the issue for now on the IIS config, by rewriting the HTTP Header in der ARR / URL Rewrite Settings. Should I close this PR or should we still try to find a solution for IPv4/IPv6 ? |
ChristophWurst
added
2. developing
Thanks for your effort here, but I guess we will just close it. Also no other people have showed up to help with at least feedback if it works in their scenario. Also there seems to be a way to configure IIS in a way that it works with Nextcloud. Sorry that this took so long and we didn't gave feedback on this PR. |
The
HTTP_X_FORWARED_FORheader might contain the IP address and the port number, when used via an IIS proxy.See https://serverfault.com/a/757994
As our setup requires the use of IIS as an incoming SSL proxy, this change allows the remote IP to be correctly detected.
In my opinion, as the port number is allowed according to the linked RFCs, NC should handle this case.