-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Allow to bind to IPv6 address specified by name, not address #2689
base: master
Are you sure you want to change the base?
Conversation
sys.exit(1) | ||
if sock is not None: | ||
listeners.append(sock) | ||
some_sock_succeeded = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we bind to all addresses returned from getaddrinfo
, or just the first one that succeeds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should bind to them all, try at least. IIUC getaddrinfo
returns the result in unspecified order. It means that one call may return [IPv6 address, IPv4 address]
and the next [IPv4 address, IPv6 address]
. If binding only to the first one we'd be introducing non-determinism between runs.
gunicorn/sock.py
Outdated
|
||
listeners.append(sock) | ||
if not some_sock_succeeded: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see some_sock_succeeded
moved inside the outermost loop, and an exit if we fail to bind any of the specified bind addresses to at least one of their infos. I think the indentation of this check and the some_sock_succeeded = False
should be done for each specified bind address. That would be consistent with the previous behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be consistent with the previous behavior.
Indeed, though I wonder if it's the best behavior - if a hostname resolves to both IPv4 and IPv6 or the user passed multiple addresses, failure to bind to any of them results in sys.exit(1)
.
I guess this could be left in the name of backwards compatibility but doing sys.exit(1)
if any bind failed seemed pretty harsh to me.
@tilgovi can I do something to help getting this merged? |
Rebased and resolved conflicts |
(just my commentary, not what is blocking this PR from moving forward)
Interesting test cases: |
How can you get |
When a typo or incorrect variable expansion produces a host name that fails the sanity checks during conversion to ascii form ("Punycode"), e.g leading dot, consecutive dots, labels exceeding >63 ascii bytes. |
No description provided.