-
Notifications
You must be signed in to change notification settings - Fork 240
Description
Hi, thanks a lot for Jamulus. I'm trying to take a look at the feasibility of adding IPv6 support, and I believe I found a bug along the way.
If a hostname has both A and AAAA records, NetworkUtil::ParseNetworkAddress() might pick the address from the AAAA record instead of the one from the A record, and fail to connect to a server running on that host.
When given a hostname, NetworkUtil::ParseNetworkAddress() uses QHostInfo::fromName() to perform the lookup, and currently picks the first result from the list. The documentation of this method does not specify anything about the order of the list for cases where there are multiple addresses. Looking a few layers down in the implementation, it calls getaddrinfo(), whose behaviour seems to be system-dependent (see e.g. https://stackoverflow.com/a/11326970).
On my computer, getaddrinfo() returns IPv6 addresses before IPv4 addresses (when both are available). So when I try to connect to a hostname that has both A and AAAA records, Jamulus selects the IPv6 address, and fails to connect to the server running on that host.
(curiously, what happens when an IPv6 CHostAddress makes its way down to CSocket::SendPacket() is that toIPv4Address() fails and returns 0, which is interpreted as 0.0.0.0, so it tries to connect to localhost)
My suggestion is to make NetworkUtil::ParseNetworkAddress() look for the first IPv4 address in the list of returned addresses. The question is what to do if the returned result is non-empty, but only contains IPv6 addresses. Even though the rest of the function has some support for IPv6, I think it would make the most sense to return false when this happens. Right now, there's nothing useful that can be done with an IPv6 address anyway. If that sounds good, I could produce a pull request.