Get correct User IP when has proxy

[everyx]

RT, we should match all pattern and use the last IP string as user IP.

This will also fix Mibew/google-maps-plugin#2

[everyx]

I just found a problem that, when the correct user IP is an IPv6 address, the regular expressions
/(\\d+\\.\\d+\\.\\d+\\.\\d+)/
is incorrect, should we change all expressions to
/(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$|^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$|^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*/
(from http://stackoverflow.com/questions/23483855/javascript-regex-to-validate-ipv4-and-ipv6-address-no-hostnames)

[snewell92]

Since Mibew uses symfony, can't we just do something like this to access the user's IP? Or does Mibew not have that part of symfony?

[everyx]

@snewell92 remote ip is get in function get_remote_host :

function get_remote_host()
{
    $ext_addr = $_SERVER['REMOTE_ADDR'];
    $has_proxy = isset($_SERVER['HTTP_X_FORWARDED_FOR'])
        && $_SERVER['HTTP_X_FORWARDED_FOR'] != $_SERVER['REMOTE_ADDR'];
    if ($has_proxy) {
        $ext_addr = $_SERVER['REMOTE_ADDR'] . ' (' . $_SERVER['HTTP_X_FORWARDED_FOR'] . ')';
    }
    return isset($_SERVER['REMOTE_HOST']) ? $_SERVER['REMOTE_HOST'] : $ext_addr;
}

It will return REMOTE_ADDR (REAL IP) if user has proxy, and the the get user ip does not consider this.

(PS: I'm not a professional PHP developer, maybe we should use $this->container->get('request')->getClientIp(); instand for custom function?)

[everyx]

I just found if user more than one proxy, it will be like this: REMOTE_ADDR (IP1, IP2), so we also should consider this situation.

[faf]

1. @everyx, Mibew gets IP address of a visitor from the headers of incoming request. AFAIK, in case of proxies the header X_FORWARDED_FOR should have a form of "client IP, proxy1 IP, ..., proxyN IP". Your code will return the IP of proxyN, not of the client.

   The actual issue is in this code. It sets the variable related to IP of a visitor in the form of "proxyN IP (client IP, proxy1 IP, ..., proxyN IP)" in case of proxyfied request.

   Probably we need to change that behaviour.

2. @everyx, If we add the support of IPv6 in the appropriate regexp, would you be able to test it?

3. @everyx, @snewell92 It is impossible to use the aformentioned part of the Symfony framework in the get_remote_host() function, because one need to keep intact the signature of the functions (otherwise there could be issues with backward compatibility for plugins, etc.). And the only way to make use of Request object without that change is to make it through global variables or instances which is a very bad approach.

[snewell92]

@faf In regards to 3. Ah, I understand. Global Request would indeed be bad. Okay! 👍

[everyx]

@faf Yes , You are right, the code is wrong.

I am use IPv4 ip in China, but my site have some IPv6 visitor in some times, so I can merge the test code and wait for a IPv6 user then test it.

Change the code chat.php#L584 maybe couldn't fix this problem completely, should we also change the get user ip code at UsersProcessor.php#L268-L273 and UsersProcessor.php#L440-L445?

[faf]

@everyx Sure thing. :)

I've just created the branch with the appropriate changes. Please, check it if you could.

[everyx]

@faf Great, I'll test it on this saturday

[everyx]

@faf I just test my comment code, it's works for me with IPv4 and IPv6, but I check your last commit in ipv6 branch, please check my comment, I think it should have a minimal bug fix :)

[faf]

@everyx, Thanks, it fixed now. Do you need any additional time for tests?

[everyx]

@faf I just find a very strict problem, I got a visitor's address like ip1,ip2, I just guess that the problem is HTTP_X_FORWARDED_FOR is not split by a comma with space, should we consider the situation here? the real world is buggy :)

[faf]

@everyx Ok, fixed. :)

[faf]

Closed due to correction of the appropriate issue.