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

Corrected --proxy-headers client ip/host when using a unix socket #636

Merged
merged 11 commits into from
Jul 28, 2020

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Apr 14, 2020

should fix #634

@euri10 euri10 requested review from rafalp and tomchristie May 4, 2020 15:06
uvicorn/protocols/utils.py Outdated Show resolved Hide resolved
uvicorn/protocols/utils.py Outdated Show resolved Hide resolved
@tomchristie
Copy link
Member

tomchristie commented May 11, 2020

Also let's go with more descriptive Pull Request names, please. 😃
"634 supervisord" is a little obscure.

References to issues ought to go in the description, eg "Refs #634" rather than in the PR name.

@euri10 euri10 changed the title 634 supervisord Corrected prox-headers client ip/host when using a unix socket May 19, 2020
@euri10 euri10 changed the title Corrected prox-headers client ip/host when using a unix socket Corrected --proxy-headers client ip/host when using a unix socket May 19, 2020
@euri10
Copy link
Member Author

euri10 commented May 19, 2020

Also let's go with more descriptive Pull Request names, please.
"634 supervisord" is a little obscure.

References to issues ought to go in the description, eg "Refs #634" rather than in the PR name.

done ! no excuses :)

@euri10 euri10 requested a review from rafalp May 19, 2020 16:40
@euri10
Copy link
Member Author

euri10 commented Jul 24, 2020

Cool, it's using the new CI with windows fine 😁
This is ready for a review @tomchristie !
Thanks

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Looks great, yup.

@euri10
Copy link
Member Author

euri10 commented Jul 24, 2020

sorry I cant squash and merge, got the same issue where it wants to have tests made before the CI change to pass.
not sure if I can do something or how you solved it for the previous one

@euri10 euri10 merged commit a796e1d into encode:master Jul 28, 2020
euri10 added a commit to euri10/uvicorn that referenced this pull request Jul 29, 2020
stefanw added a commit to stefanw/uvicorn that referenced this pull request Jul 30, 2020
euri10 added a commit that referenced this pull request Jul 31, 2020
…socket (#729)

* Revert "Corrected --proxy-headers client ip/host when using a unix socket (#636)"

This reverts commit a796e1d

* Distinguish case fd/unix socket to return correctly client

* Handle windows case

* Added test for AF_UNIX socket type
Modified MockSocket peername to pass tuples instead of list because socket.getpeername() and socket.getsockname() return tuples

* Black

* Removed test, black works locally but not in CI....

* Same deal on the server side of things

* Test on AF_UNIX only if it is in socket

* Simpler handling

* Removed debug leftovers
@euri10 euri10 deleted the 634_supervisord branch October 6, 2020 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--proxy-headers with supervisor get client ip failed
3 participants