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

Remove port from IP address if it's there #6755

Merged
merged 1 commit into from
Mar 27, 2017
Merged

Remove port from IP address if it's there #6755

merged 1 commit into from
Mar 27, 2017

Conversation

samhotchkiss
Copy link
Contributor

No description provided.

@samhotchkiss samhotchkiss added [Feature] Protect Also known as Brute Force Attack Protection [Status] Needs Review This PR is ready for review. labels Mar 27, 2017
@jeherve jeherve added [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended labels Mar 27, 2017
Copy link
Contributor

@briancolinger briancolinger left a comment

Choose a reason for hiding this comment

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

Looks good. Works as expected.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 27, 2017
@rcoll
Copy link
Member

rcoll commented Mar 27, 2017

Love it. 🚢

@rcoll rcoll merged commit 220eed6 into master Mar 27, 2017
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 27, 2017
@rcoll rcoll deleted the fix/protect-ports branch March 27, 2017 17:32
jeherve added a commit that referenced this pull request Mar 28, 2017
samhotchkiss pushed a commit that referenced this pull request Mar 29, 2017
* Readme: remove old release and add skeleton for 4.8.

* Changelog: add #6572

* Changelog: add #6567

* Changelog: add #6542

* Changelog: add #6527

* Changelog: add #6508

* Changelog: add #6478

* Changelog: add #6477

* Changelog: add #6249

* Update stable version and remove old version from readme.

* Changelog: add 4.7.1 to changelog.

* Readme: add new contributor.

* Sync: update docblock @SInCE version.

Related: #6053

* Changelog: add release post.

* changelog: add #6053

* Changelog: add #6413

* Changelog: add #6482

* Changelog: add #6584

* Changelog add #6603

* Changelog: add #6606

* Changelog: add #6611

* Changelog: add #6635

* Changelog: add #6639

* Changelog: add #6684

* Changelog: add #6710

* Changelog: add #6711

* Changelog: add #5461

* Testing list: update Settings UI feedback prompt.

Props @MichaelArestad

* Changelog: add #6789

* Changelog: add #6778

* Changelog: add #6777

* Changelog: add #6775

* Changelog: add #6755

* Changelog: add #6731

* Changelog: add #6721

* Changelog: add #6705

* Changelog: add #6702

* Changelog: add #6671

* Changelog: add #6637

* Changelog: add #6582

* Changelog: add #6566

* Changelog: add #6555

* Changelog: add #6529

* Changelog: add #6344

* Changelog: add #5763

* Changelog: add #5503

* Changelog: update #6637 changelog.

@see 40e115c#commitcomment-21523982

* Changelog: add #6699

* Changelog: add #6632

* Changelog: add #6769

* Changelog: add #6707

* Changelog: add #6590
@da2x
Copy link
Contributor

da2x commented Apr 21, 2017

REVERT! This breaks IPv6 support!

$_SERVER['REMOTE_ADDR'] will never return a INET address-styled colon-separated port suffix anyway. What was this ever meant to fix?

@jeherve
Copy link
Member

jeherve commented Apr 21, 2017

@da2x Thanks for the report!

$_SERVER['REMOTE_ADDR'] will never return a INET address-styled colon-separated port suffix anyway.

It unfortunately does in some server configurations.

@da2x
Copy link
Contributor

da2x commented Apr 21, 2017

It unfortunately does in some server configurations.

Ha. People. They do the craziest thing when passing variables to PHP.

Anyway, you can keep this fix in place, but it will require additional checks. Count for how many colons there are in the string. More than one? Then you’ve got an IPv6 address as there will always be at least two. If there is only one colon and also four dots in the string? Assume it’s an InetSocketAddress notation representing a IP-port pair and proceed with splitting the host from port by splitting at the colon. (Also: issue a PHP warning.)

Should also remove any leading [ and trailing ]:port, but again: this shouldn’t happen unless the environment is broken and someone is being very creative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Protect Also known as Brute Force Attack Protection [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants