-
Notifications
You must be signed in to change notification settings - Fork 227
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
ParseNetworkAddress might select IPv6 addresses over IPv4 when a hostname has both #722
Comments
Thanks for your investigation. So your change would be only be in function NetworkUtil::ParseNetworkAddress(), right? |
I also experienced this problem while wanting to connect to |
@corrados: Yes, I think changing that method should be enough. I tried it out and it seems to solve the issue (including connecting to @ann0see: I agree. I think some things have happened since #46 was opened that might make adding IPv6 support a bit easier than it was then. I will try things out a bit more and hopefully come up with a proposal for how support could be added. |
Selecting ipv6 before ipv4 is a bug in linux distros that have been zealously promoting ipv6. |
Calling a configuration choice -- it's config, there to enable that choice, explicitly -- a bug is really rather a political statement in itself, I feel. Devices may not have IPv4 addresses at all, so no matter how you order the preference of selection, you'll still get an IPv6 address. Not currently likely but... Jamulus should support IPv6, really - it does expand the range of problems users are likely to find of course. (i.e. the local IPv6 address gets selected but there's no routing at all for IPv6 being the most likely...) |
I would like ipv6 implemented correctly and I see an ipv6 problem already. ipv4 was made without consideration that it would be superseded, thus ipv4 applications were coded similarly to how Jamulus is (they break when gai.conf is not configured with regard to ipv4). Now, ipv6 applications will repeat this problem if it is not addressed. Therefore gai.conf should not break ipv4, And ipv6 should not break when ipv4 is given precedence in gai.conf. (ipv6 should be ipv4 aware... but ipv4 must remain ignorant of ipv6)
|
OK, I see what you mean -- if you have both IPv4 and IPv6 addressing and if you have IPv4 applications and if you use gai.conf to set the precedence of addressing, then the IPv4 stack shouldn't be broken. Agreed. |
Do we need both #46 and this issue? |
This is especially important for hostnames that have both IPv4 and IPv6 addresses, as NetworkUtil::ParseNetworkAddress() previously might have returned IPv6 addresses for such cases. This solves connecting to localhost on IPv6-enabled systems.
I think it makes sense to have both. While they originated from the same issue (hosts with both v4 and v6 addresses), #46 quickly turned into a general IPv6 feature request, while this one regards a very specific bug when trying to use IPv4 on IPv6-enabled hosts. Though I agree that it would be nicer to have just one IPv6 issue. I'm not 100% on how to follow the contribution guidelines here, but since the suggested fix is so small, I will go ahead and create a pull request for this one. |
Avoid selecting IPv6 results from hostname lookup (#722)
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()
usesQHostInfo::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 callsgetaddrinfo()
, 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 toCSocket::SendPacket()
is thattoIPv4Address()
fails and returns 0, which is interpreted as0.0.0.0
, so it tries to connect tolocalhost
)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.The text was updated successfully, but these errors were encountered: