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.

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

IPAddr.new accepts invalid IPv6 addresses #52

Closed
HoneyryderChuck opened this issue Jan 21, 2023 · 6 comments
Closed

IPAddr.new accepts invalid IPv6 addresses #52

HoneyryderChuck opened this issue Jan 21, 2023 · 6 comments

Comments

@HoneyryderChuck
Copy link

require "ipaddr"
ip = "fe80::1213:31ff:fe67:8b94%en0"
IPAddr.new(ip)
# in ruby 3.1 and 3.2, #<IPAddr: IPv6:fe80:0000:0000:0000:1213:31ff:fe67:8b94%en0/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff>
# in ruby 3.0:  invalid address: fe80::1213:31ff:fe67:8b94%en0 (IPAddr::InvalidAddressError)
@amatsuda
Copy link
Member

Bisected the behavior change down to 09a6408
I have no idea whether this is a bug or not though. /cc @jeremyevans

@jeremyevans
Copy link
Contributor

@HoneyryderChuck Can you explain why you think the address is invalid? Prior to the change, IPAddr didn't handle zone identifiers in IPv6 address, and after the change it does. There is no ability to check whether the given zone identifier is valid for the current system, though. See https://bugs.ruby-lang.org/issues/10911 for background.

@HoneyryderChuck
Copy link
Author

I see. The problem then is in Ruby's socket code then, as I can't setup a udp socket with the same ipv6 address (not on my laptop, will provide backtrace later).

I was using ipaddr to eliminate "faulty" nameservers from the options provided by the system, but according to rhe bug report you linked, they're not faulty after all, it's just that UDPSocket can't deal with them.

@HoneyryderChuck
Copy link
Author

HoneyryderChuck commented Jan 21, 2023

The change in behaviour comes from this code; prior to ruby 3.1, it was not accepting link-local IPv6 addresses such as "fe80::1213:31ff:fe67:8b94%en0".

The subsequent issue that happens after the behaviour change is due to this line. Putting it in a small reproducible example:

require "uri"
port = 53
uri = URI::Generic.build(scheme: "udp", port: port)
ip = "[fe80::1213:31ff:fe67:8b94%en0]"
uri.hostname = ip
# URI::InvalidComponentError: bad component(expected host component): [fe80::1213:31ff:fe67:8b94%en0]

So the issue appears to be in uri, not ipaddr. Shall I move the discussion to that repo?

EDIT: udp sockets have no problem with the ipv6 notation above:

ip = IPAddr.new("[fe80::1213:31ff:fe67:8b94%en0]")
io = UDPSocket.new(ip.family)
 io.sendmsg_nonblock("ping", 0, Socket.sockaddr_in(port, ip.to_s)) #=> 4

@jeremyevans
Copy link
Contributor

Section 3.2.2 of RFC 3986 states regarding use of IPv6 addresses in URIs:

This syntax does not support IPv6 scoped addressing zone identifiers.

So for RFC 3986 it's expected that URIs won't support IPv6 addresses with zone identifiers even though they are valid addresses.

Looking at the relevant part of the WHATWG URL spec (https://url.spec.whatwg.org/#valid-ipv6-address-string), it refers to section 2.2 of RFC 4291, which also doesn't indicate support for IPv6 addresses with zone identifiers.

In short, this doesn't appear to be a bug in either ipaddr or uri, it's just what you want to do is not supported by URIs.

@HoneyryderChuck
Copy link
Author

Oh, you're absolutely right, never thought that uris would not accept something considered valid for IPs. TIL. Thanks for the explanation. Will close the ticket.

HoneyryderChuck added a commit to HoneyryderChuck/httpx that referenced this issue Jan 22, 2023
It was clarified in
ruby/ipaddr#52 (comment) that
there is no bug in `uri`, but rather that ipv6 addresses with
zone-identifier aren't supposed to be used in uris.
HoneyryderChuck added a commit to HoneyryderChuck/httpx that referenced this issue Jan 22, 2023
It was clarified in
ruby/ipaddr#52 (comment) that
there is no bug in `uri`, but rather that ipv6 addresses with
zone-identifier aren't supposed to be used in uris.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants