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

Trusted hosts network resolver #157

Merged
merged 44 commits into from Dec 17, 2019

Conversation

@kamarton
Copy link
Contributor

kamarton commented Nov 20, 2019

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Tests pass? ✔️
Fixed issues #119
  • more tests
  • more method comments
@kamarton

This comment has been minimized.

Copy link
Contributor Author

kamarton commented Nov 20, 2019

@samdark please pre check the code. Based on a few tests, the code is already functional.

samdark added 8 commits Nov 20, 2019
@samdark samdark requested a review from yiisoft/reviewers Nov 21, 2019
@samdark

This comment has been minimized.

Copy link
Member

samdark commented Nov 22, 2019

Somogyi Márton added 7 commits Nov 23, 2019
Somogyi Márton
Somogyi Márton
Somogyi Márton
Somogyi Márton
Somogyi Márton
@kamarton

This comment has been minimized.

Copy link
Contributor Author

kamarton commented Nov 29, 2019

@samdark reviewable (partially depend #183)

@kamarton kamarton changed the title [WIP] Trusted hosts network resolver Trusted hosts network resolver Nov 29, 2019
src/Middleware/TrustedHostsNetworkResolver.php Outdated Show resolved Hide resolved
src/Middleware/TrustedHostsNetworkResolver.php Outdated Show resolved Hide resolved
return $new;
}

public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface

This comment has been minimized.

Copy link
@xepozz

xepozz Nov 29, 2019

Please split this method on some methods.

This comment has been minimized.

Copy link
@kamarton

kamarton Dec 5, 2019

Author Contributor

I see that this function of too many lines of code. It can be splitted, but for multiple functions, 4-5 parameters are transported, which is not very nice.

src/Middleware/TrustedHostsNetworkResolver.php Outdated Show resolved Hide resolved
@kamarton

This comment has been minimized.

Copy link
Contributor Author

kamarton commented Nov 30, 2019

@samdark Yii2 did not support it, but many systems support x-forwarded-port (or alternate) headers. Is this make sense for us?
eg. https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-port

UPDATE: implemented

@kamarton kamarton changed the title Trusted hosts network resolver [WIP] Trusted hosts network resolver Nov 30, 2019
@kamarton kamarton changed the title [WIP] Trusted hosts network resolver Trusted hosts network resolver Dec 10, 2019
@samdark samdark merged commit 3c50044 into yiisoft:master Dec 17, 2019
1 of 3 checks passed
1 of 3 checks passed
Travis CI - Pull Request Build Errored
Details
continuous-integration/styleci/pr Issues have been identified with 1 file
Details
Scrutinizer Analysis: 3 new issues, 26 updated code elements – Tests: passed
Details
@samdark

This comment has been minimized.

Copy link
Member

samdark commented Dec 17, 2019

Merged. Thank you very much!

@samdark samdark added this to the 3.0.0-alpha1 milestone Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.