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

Issue #1719: resolving hostnames for bind #1723

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Issue #1719: resolving hostnames for bind #1723

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f3ceae2
Using getaddrinfo to resolve hostnames properly
wanneut Mar 12, 2018
b1779b5
better style
wanneut Mar 13, 2018
61ea13e
fixed socket.close
wanneut Mar 14, 2018
f0f2928
spcaes
wanneut Mar 14, 2018
06bd5d4
adopted some tests
wanneut Mar 14, 2018
3f633ec
reseted requirements
wanneut Mar 14, 2018
0c45c4a
more reasonable defaults: [::] and localhost will lsiten on IPv6+IPv4
wanneut Mar 14, 2018
25935b7
127.0.0.1 -> localhost
wanneut Mar 14, 2018
4874c1b
_sock_type gets only family
wanneut Mar 15, 2018
2bdd721
adopted last test
wanneut Mar 15, 2018
c94048e
IPv6 schme handling, test coverage
Mar 15, 2018
d3568c8
test for tcp://
wanneut Mar 15, 2018
7904f3f
Removed test depending on modern /etc/hosts
wanneut Apr 18, 2018
0ebf15c
accept IPv4 only environment
wanneut Apr 18, 2018
6f91687
rerun checks
wanneut Apr 25, 2018
51466dc
making code unreadable
wanneut Apr 25, 2018
c597560
removed trailing whitespace
wanneut Apr 25, 2018
4c20ec1
-
wanneut Apr 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions gunicorn/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ def worker_class(self):
@property
def address(self):
s = self.settings['bind'].get()
return [util.parse_address(_compat.bytes_to_str(bind)) for bind in s]

Copy link
Owner

Choose a reason for hiding this comment

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

why replacing that code?

Copy link
Author

Choose a reason for hiding this comment

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

It was described at the beginning. It can handle only one address.

addresses = []
for bind in s:
addresses.extend(util.parse_address(_compat.bytes_to_str(bind)))
return addresses

@property
def uid(self):
Expand Down Expand Up @@ -534,9 +538,9 @@ class Bind(Setting):
validator = validate_list_string

if 'PORT' in os.environ:
default = ['0.0.0.0:{0}'.format(os.environ.get('PORT'))]
default = ['[::]:{0}'.format(os.environ.get('PORT'))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

At least on my machine, this returns only IPv6 addresses from socket.getaddrinfo(). Since this is a list, maybe we want to give it two entries so Gunicorn binds to IPv4 and IPv6 by default?

Copy link
Author

@wanneut wanneut Mar 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If you bind to :: you will listen to any address (IPv4 and IPv6) as long the IPV6_V6ONLY flag is not set.
In fact :: could be written as ::0.0.0.0 which is already the IPv4-compatible address for 0.0.0.0. (Older version of ::FFFF:0.0.0.0)

Copy link
Owner

Choose a reason for hiding this comment

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

what about the machine that doesn't support or has not been installed with ipv6 support?

Copy link
Author

@wanneut wanneut Apr 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The older version dosn't work on IPv6 installations at all.
This version, works everywhere (IPv4 like IPv6) exempt of some obscure Linux-configurations (I think there are not many other OSes where something can be done.), where someone explicitly removed IPv6 support. I think such users should know how to specify another bind address.
If someone shoots himself into his foot he should not wonder if it hurts.
Gunicorn requires Python 2 (which is newer than IPv6.) multithreading and several other things which are much more uncommon than IPv6.
At the end you have to decide what your defaults are.
I mean what about installations with disabled IPv4? It is deprecated for 20 years now. Time to turn it off? IPv6 is compatible to it. So it shouldn't be needed any more. The new code could handle that.

else:
default = ['127.0.0.1:8000']
default = ['localhost:8000']

desc = """\
The socket to bind.
Expand Down
33 changes: 14 additions & 19 deletions gunicorn/sock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ def __init__(self, address, conf, log, fd=None):
self.log = log
self.conf = conf

self.cfg_addr = address
self.cfg_addr = address[4]
if fd is None:
sock = socket.socket(self.FAMILY, socket.SOCK_STREAM)
sock = socket.socket(address[0], socket.SOCK_STREAM)
bound = False
else:
sock = socket.fromfd(fd, self.FAMILY, socket.SOCK_STREAM)
sock = socket.fromfd(fd, address[0], socket.SOCK_STREAM)
os.close(fd)
bound = True

Expand Down Expand Up @@ -74,8 +74,6 @@ def close(self):

class TCPSocket(BaseSocket):

FAMILY = socket.AF_INET

def __str__(self):
if self.conf.is_ssl:
scheme = "https"
Expand All @@ -92,27 +90,23 @@ def set_options(self, sock, bound=False):

class TCP6Socket(TCPSocket):

FAMILY = socket.AF_INET6

def __str__(self):
(host, port, _, _) = self.sock.getsockname()
return "http://[%s]:%d" % (host, port)


class UnixSocket(BaseSocket):

FAMILY = socket.AF_UNIX

def __init__(self, addr, conf, log, fd=None):
if fd is None:
try:
st = os.stat(addr)
st = os.stat(addr[4])
except OSError as e:
if e.args[0] != errno.ENOENT:
raise
else:
if stat.S_ISSOCK(st.st_mode):
os.remove(addr)
os.remove(addr[4])
else:
raise ValueError("%r is not a socket" % addr)
super(UnixSocket, self).__init__(addr, conf, log, fd=fd)
Expand All @@ -128,12 +122,11 @@ def bind(self, sock):


def _sock_type(addr):
if isinstance(addr, tuple):
if util.is_ipv6(addr[0]):
sock_type = TCP6Socket
else:
sock_type = TCPSocket
elif isinstance(addr, string_types):
if addr[0] == socket.AF_INET6:
sock_type = TCP6Socket
elif addr[0] == socket.AF_INET:
sock_type = TCPSocket
elif addr[0] == socket.AF_UNIX:
sock_type = UnixSocket
else:
raise TypeError("Unable to create socket from: %r" % addr)
Expand Down Expand Up @@ -166,7 +159,8 @@ def create_sockets(conf, log, fds=None):
for fd in fds:
sock = socket.fromfd(fd, socket.AF_UNIX, socket.SOCK_STREAM)
sock_name = sock.getsockname()
sock_type = _sock_type(sock_name)
sockinfo = (sock.family, sock.type, sock.proto, '', socket.getsockname())
sock_type = _sock_type(sockinfo)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you pass a tuple here? _sock_type() only uses the first element of the tuple sockinfo. Can't we just pass sock.family?

Copy link
Author

Choose a reason for hiding this comment

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

It was done so before. But I can change that.

listener = sock_type(sock_name, conf, log, fd=fd)
listeners.append(listener)

Expand Down Expand Up @@ -204,6 +198,7 @@ def create_sockets(conf, log, fds=None):
def close_sockets(listeners, unlink=True):
for sock in listeners:
sock_name = sock.getsockname()
socket_family = sock.family
sock.close()
if unlink and _sock_type(sock_name) is UnixSocket:
if unlink and socket_family == socket.AF_UNIX:
os.unlink(sock_name)
15 changes: 2 additions & 13 deletions gunicorn/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,9 @@ def unlink(filename):
if error.errno not in (errno.ENOENT, errno.ENOTDIR):
raise


def is_ipv6(addr):
try:
socket.inet_pton(socket.AF_INET6, addr)
except socket.error: # not a valid address
return False
except ValueError: # ipv6 not supported on this platform
return False
return True


def parse_address(netloc, default_port=8000):
if re.match(r'unix:(//)?', netloc):
return re.split(r'unix:(//)?', netloc)[-1]
return [(socket.AF_UNIX, socket.SOCK_STREAM, 0, '', re.split(r'unix:(//)?', netloc)[-1])]

if netloc.startswith("tcp://"):
netloc = netloc.split("tcp://")[1]
Expand All @@ -258,7 +247,7 @@ def parse_address(netloc, default_port=8000):
port = int(port)
else:
port = default_port
return (host, port)
return socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM)


def close_on_exec(fd):
Expand Down
4 changes: 3 additions & 1 deletion tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
from gunicorn.workers.sync import SyncWorker
from gunicorn import glogging
from gunicorn.instrument import statsd
from socket import AF_INET
from socket import SOCK_STREAM

dirname = os.path.dirname(__file__)
def cfg_module():
Expand Down Expand Up @@ -74,7 +76,7 @@ def test_property_access():
assert c.workers == 3

# Address is parsed
assert c.address == [("127.0.0.1", 8000)]
assert c.address == [(AF_INET, SOCK_STREAM, 6, '', ('127.0.0.1', 8000))]

# User and group defaults
assert os.geteuid() == c.uid
Expand Down
28 changes: 11 additions & 17 deletions tests/test_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,20 @@
from gunicorn import util
from gunicorn.errors import AppImportError
from gunicorn.six.moves.urllib.parse import SplitResult # pylint: disable=no-name-in-module
from socket import AF_INET
from socket import AF_INET6
from socket import AF_UNIX
from socket import SOCK_STREAM


@pytest.mark.parametrize('test_input, expected', [
('unix://var/run/test.sock', 'var/run/test.sock'),
('unix:/var/run/test.sock', '/var/run/test.sock'),
('', ('0.0.0.0', 8000)),
('[::1]:8000', ('::1', 8000)),
('localhost:8000', ('localhost', 8000)),
('127.0.0.1:8000', ('127.0.0.1', 8000)),
('localhost', ('localhost', 8000))
('unix://var/run/test.sock', [(AF_UNIX, SOCK_STREAM, 0, '', 'var/run/test.sock')]),
('unix:/var/run/test.sock', [(AF_UNIX, SOCK_STREAM, 0, '', '/var/run/test.sock')]),
('', [(AF_INET6, SOCK_STREAM, 6, '', ('::', 8000, 0, 0))]),
('[::1]:8007', [(AF_INET6, SOCK_STREAM, 6, '', ('::1', 8007, 0, 0))]),
('localhost:8007', [(AF_INET6, SOCK_STREAM, 6, '', ('::1', 8007, 0, 0)), (AF_INET, SOCK_STREAM, 6, '', ('127.0.0.1', 8007))]),
('127.0.0.1:8007', [(AF_INET, SOCK_STREAM, 6, '', ('127.0.0.1', 8007))]),
('localhost', [(AF_INET6, SOCK_STREAM, 6, '', ('::1', 8000, 0, 0)), (AF_INET, SOCK_STREAM, 6, '', ('127.0.0.1', 8000))])
])
def test_parse_address(test_input, expected):
assert util.parse_address(test_input) == expected
Expand All @@ -33,16 +37,6 @@ def test_http_date():
assert util.http_date(1508607753.740316) == 'Sat, 21 Oct 2017 17:42:33 GMT'


@pytest.mark.parametrize('test_input, expected', [
('1200:0000:AB00:1234:0000:2552:7777:1313', True),
('1200::AB00:1234::2552:7777:1313', False),
('21DA:D3:0:2F3B:2AA:FF:FE28:9C5A', True),
('1200:0000:AB00:1234:O000:2552:7777:1313', False),
])
def test_is_ipv6(test_input, expected):
assert util.is_ipv6(test_input) == expected


def test_warn(capsys):
util.warn('test warn')
_, err = capsys.readouterr()
Expand Down