improved client IP detection

[Overtorment]

On Heroku ip wasnt detected correctly. Thats a fix.

[oscarotero]

Hi, @Overtorment thanks for your contribution. I think this change is unnecesary, because these headers are checked. The values of $server['HTTP_X_FORWARDED_FOR'] should be checked in the header X-Forwarded-For.

[Overtorment]

@oscarotero thats what I thought too, but I deployed to https://heroku.com/ and got the wrong IP

[Overtorment]

hmmm. worth doublechecking, maybe smth to do with the order of headers...

[Overtorment]

@oscarotero am I doing smth wrong? ^^^ shows internal ip starting with 10.

[Overtorment]

ok, found the issue:

$server['REMOTE_ADDR'] takes priority since its always first in the array, and its a reverse proxy address on heroku

@oscarotero how would you advise to fix that? Ill prepare a PR

[oscarotero]

Ok, I see. Maybe an option to use REMOTE_ADDR, or change the priority. A PR would be great. Thank you.

[Overtorment]

@oscarotero I have just rearranged code so that HTTP_X_FORWARDED_FOR is before REMOTE_ADDR

[oscarotero]

Your solution is not valid, because only cover your needs. It's better to keep the code as was, just with the addition to an option to use or not remote_addr.

[oscarotero]

I preffer to create a new PR for this.

[Overtorment]

Okay, I think its the best and simplest solution. Litterally no code written, just moved a bit. Now headers (if any) take priority over $server['REMOTE_ADDR'] In case you explicitly need $server['REMOTE_ADDR'] you can disable headers via configuration.

[Overtorment]

@oscarotero I rebased it to 1 commit to keep git history tidy

[oscarotero]

Ok, I'm ok with this but there's one more thing: The default config shoud be trust in REMOTE_ADDR, instead the headers, for security reasons and to keep backward compatibility, so the headers array should be empty by default. To ease the configuration of headers, the argument of headers() should have a default value:

Middlewares::clientIp(); //trust in REMOTE_ADDR
Middlewares::clientIp()->headers(); //Use the headers before
Middlewares::clientIp()->headers(['X-Forwarded']); //Use just this header before, instead all

The only change is empty the headers array and add it as the default value of headers method:

public function headers (array $headers = ['Forwarded', 'Forwarded-For', 'Client-Ip', 'X-Forwarded', 'X-Forwarded-For', 'X-Cluster-Client-Ip']) {
        $this->headers = $headers;

        return $this;
}

[Overtorment]

@oscarotero fixed. Unit test as well

[oscarotero]

Thank you