Skip to content

Implement dual-stack support #861

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

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

Implement dual-stack support #861

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f06028
fix: use dual-stack socket instead of IPv4-only
DerEnderKeks Feb 24, 2024
a77ccaf
style: move comment to make black happy
DerEnderKeks Mar 21, 2024
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
20 changes: 18 additions & 2 deletions pychromecast/socket_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,16 @@ def mdns_backoff(
self.host,
self.port,
)
self.socket.connect((self.host, self.port))

mapped_host = socket.getaddrinfo(
self.host,
self.port,
self.socket.family,
socket.SOCK_STREAM,
flags=(socket.AI_ADDRCONFIG | socket.AI_V4MAPPED),
)[0][4]

self.socket.connect(mapped_host)
context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
context.check_hostname = False
context.verify_mode = ssl.CERT_NONE
Expand Down Expand Up @@ -1133,7 +1142,14 @@ def new_socket() -> socket.socket:
Try to set SO_REUSEPORT for BSD-flavored systems if it's an option.
Catches errors if not.
"""
new_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
try:
new_sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
# ensuring dual-stack
new_sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0)
except OSError:
# falling back to IPv4 on systems without IPv6
new_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

new_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks strange to me. We are not making sure the socket type matches the data returned from getaddrinfo. The creation of the socket should be postponed until we have resolved the name using getaddrinfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't really a need to match the socket to the resolved address type, as the dual-stack socket can connect to both types. If it falls back to a v4-only socket and only a v6 address is returned, it would fail to connect either way,

Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont agree, if we are using getaddrinfo to get the host info, we should respect what it return.


try:
Expand Down