Skip to content
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

Set getRemoteAddress to detect comma seperated RFC 7239 lists #1546

Closed
wants to merge 1 commit into from
Closed

Set getRemoteAddress to detect comma seperated RFC 7239 lists #1546

wants to merge 1 commit into from

Conversation

IcyApril
Copy link

RFC 7239 allows comma separated X-Forwarded-For headers. This modification ensures in the event the getRemoteAddress gets one via the alternativeHeaders variable, the correct client IP can be returned. This is done by getting the first IP in the list after detecting for a comma separated IP list.

@vancoz vancoz added the PS label Jul 23, 2015
@@ -58,6 +58,12 @@ public function getRemoteAddress($ipToLong = false)
if (!$this->remoteAddress) {
$this->remoteAddress = $this->request->getServer('REMOTE_ADDR');
}

if ((filter_var($this->remoteAddress, FILTER_VALIDATE_IP) !== true) && (strpos($this->remoteAddress, ',') !== true)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@IcyApril $this->remoteAddress will not be X-Forwarded-For headers. $this->request->getServer('REMOTE_ADDR') It calls http://framework.zend.com/apidoc/2.3/classes/Zend.Http.PhpEnvironment.Request.html#getServer under hood.

@IcyApril
Copy link
Author

There is an alternativeHeaders field in the event REMOTE_ADDR; we must account for the fact this may be a comma separated list.

@magento-cicd2
Copy link
Contributor

We have automated a Magento Contributor License Agreement verifier for contributions sent to our GitHub projects.
Please see the CLA agreement in the Pull Request comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants