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.

Sign up for GitHub

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

Jump to bottom

Get the request.client under case uvicorn is run with a fd or a unix socket #729

Merged
merged 10 commits into from Jul 31, 2020
Merged

Get the request.client under case uvicorn is run with a fd or a unix socket #729

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6256030
Revert "Corrected --proxy-headers client ip/host when using a unix so…
euri10 Jul 29, 2020
84c1bd8
Distinguish case fd/unix socket to return correctly client
euri10 Jul 29, 2020
4dc4643
Handle windows case
euri10 Jul 29, 2020
cbc59fe
Added test for AF_UNIX socket type
euri10 Jul 29, 2020
bc743e1
Black
euri10 Jul 29, 2020
9a8b955
Removed test, black works locally but not in CI....
euri10 Jul 29, 2020
253b7b7
Same deal on the server side of things
euri10 Jul 29, 2020
1ffa996
Test on AF_UNIX only if it is in socket
euri10 Jul 29, 2020
d43d0f8
Simpler handling
euri10 Jul 31, 2020
44f07fe
Removed debug leftovers
euri10 Jul 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 18 additions & 6 deletions tests/protocols/test_utils.py
View file
Open in desktop
Expand Up @@ -29,42 +29,54 @@ def test_get_local_addr_with_socket():
assert get_local_addr(transport) is None

transport = MockTransport(
{"socket": MockSocket(family=socket.AF_INET6, sockname=["::1", 123])}
{"socket": MockSocket(family=socket.AF_INET6, sockname=("::1", 123))}
)
assert get_local_addr(transport) == ("::1", 123)

transport = MockTransport(
{"socket": MockSocket(family=socket.AF_INET, sockname=["123.45.6.7", 123])}
{"socket": MockSocket(family=socket.AF_INET, sockname=("123.45.6.7", 123))}
)
assert get_local_addr(transport) == ("123.45.6.7", 123)

if hasattr(socket, "AF_UNIX"):
transport = MockTransport(
{"socket": MockSocket(family=socket.AF_UNIX, sockname=("127.0.0.1", 8000))}
)
assert get_local_addr(transport) == ("127.0.0.1", 8000)


def test_get_remote_addr_with_socket():
transport = MockTransport({"socket": MockSocket(family=socket.AF_IPX)})
assert get_remote_addr(transport) is None

transport = MockTransport(
{"socket": MockSocket(family=socket.AF_INET6, peername=["::1", 123])}
{"socket": MockSocket(family=socket.AF_INET6, peername=("::1", 123))}
)
assert get_remote_addr(transport) == ("::1", 123)

transport = MockTransport(
{"socket": MockSocket(family=socket.AF_INET, peername=["123.45.6.7", 123])}
{"socket": MockSocket(family=socket.AF_INET, peername=("123.45.6.7", 123))}
)
assert get_remote_addr(transport) == ("123.45.6.7", 123)

if hasattr(socket, "AF_UNIX"):
transport = MockTransport(
{"socket": MockSocket(family=socket.AF_UNIX, peername=("127.0.0.1", 8000))}
)
assert get_remote_addr(transport) == ("127.0.0.1", 8000)


def test_get_local_addr():
transport = MockTransport({"sockname": "path/to/unix-domain-socket"})
assert get_local_addr(transport) is None

transport = MockTransport({"sockname": ["123.45.6.7", 123]})
transport = MockTransport({"sockname": ("123.45.6.7", 123)})
assert get_local_addr(transport) == ("123.45.6.7", 123)


def test_get_remote_addr():
transport = MockTransport({"peername": None})
assert get_remote_addr(transport) is None

transport = MockTransport({"peername": ["123.45.6.7", 123]})
transport = MockTransport({"peername": ("123.45.6.7", 123)})
assert get_remote_addr(transport) == ("123.45.6.7", 123)
28 changes: 20 additions & 8 deletions uvicorn/protocols/utils.py
View file
Open in desktop
@@ -1,11 +1,6 @@
import socket
import urllib

if hasattr(socket, "AF_UNIX"):
SUPPORTED_SOCKET_FAMILIES = (socket.AF_INET, socket.AF_INET6, socket.AF_UNIX)
else:
SUPPORTED_SOCKET_FAMILIES = (socket.AF_INET, socket.AF_INET6)


def get_remote_addr(transport):
socket_info = transport.get_extra_info("socket")
Expand All @@ -20,9 +15,17 @@ def get_remote_addr(transport):
else:
family = socket_info.family

if family in SUPPORTED_SOCKET_FAMILIES:
if family in (socket.AF_INET, socket.AF_INET6):
return (str(info[0]), int(info[1]))

elif hasattr(socket, "AF_UNIX") and family is socket.AF_UNIX:
if isinstance(info, tuple):
# fd case
# <uvloop.PseudoSocket fd=13, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, laddr=('127.0.0.1', 8000), raddr=('127.0.0.1', 38634)>
return (str(info[0]), int(info[1]))
else:
# unix socket case
# <uvloop.PseudoSocket fd=21, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, laddr=/tmp/gunicorn.sock>
return None
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd do something like this, to simplify:

HAS_AF_UNIX = hasattr(socket, "AF_UNIX")

def get_remote_addr(transport):
    ...

        if family in (socket.AF_INET, socket.AF_INET6) or (
            HAS_AF_UNIX and family == socket.AF_UNIX and isinstance(family, tuple)
        ):
            return (str(info[0]), int(info[1]))

        return None

Copy link
Member

Choose a reason for hiding this comment

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

Could we just change the whole block to...

return (str(info[0]), int(info[1])) if isinstance(info, tuple) else None

Copy link
Member Author

Choose a reason for hiding this comment

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

I did it that way, way simpler,
Just note that I kept in get_remote_addr the try except introduced it seems in #495

return None
info = transport.get_extra_info("peername")
if info is not None and isinstance(info, (list, tuple)) and len(info) == 2:
Expand All @@ -35,8 +38,17 @@ def get_local_addr(transport):
if socket_info is not None:
info = socket_info.getsockname()
family = socket_info.family
if family in SUPPORTED_SOCKET_FAMILIES:
if family in (socket.AF_INET, socket.AF_INET6):
return (str(info[0]), int(info[1]))
elif hasattr(socket, "AF_UNIX") and family is socket.AF_UNIX:
if isinstance(info, tuple):
# fd case
# <uvloop.PseudoSocket fd=13, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, laddr=('127.0.0.1', 8000), raddr=('127.0.0.1', 38634)>
return (str(info[0]), int(info[1]))
else:
# unix socket case
# <uvloop.PseudoSocket fd=21, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, laddr=/tmp/gunicorn.sock>
return None
return None
info = transport.get_extra_info("sockname")
if info is not None and isinstance(info, (list, tuple)) and len(info) == 2:
Expand Down