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

Support ipv6 link-local urls (#470) #794

Closed
wants to merge 1 commit into from

Conversation

socs
Copy link
Contributor

@socs socs commented Mar 1, 2023

URL parsing fails when the host is a ipv6 link-local address due to zone identifiers being omitted by the url spec.
https://url.spec.whatwg.org/#host-representation

This fix resolves the issue by stripping the zone id from the url but adding it back into the host field during conversion to ConnectionInfo.

URL parsing fails when the host is a ipv6 link-local address due to
zone identifiers being omitted by the url spec.
https://url.spec.whatwg.org/#host-representation

This fix resolves the issue by stripping the zone id from the url but
adding it back into the host field during conversion to ConnectionInfo.
@socs
Copy link
Contributor Author

socs commented Mar 1, 2023

Ipv6 url support was added in #679 however it did not allow for link-local zone id which I've attempted to rectify here. The reason to omit it in a general url library may be reasonable but for redis urls this ought to be supported.

@jaymell
Copy link
Contributor

jaymell commented Mar 17, 2023

I must profess ignorance to the nuances of IPV6, but are there no url-parsing libs that support this functionality? This just seems like a strange problem for a Redis client to have to solve. How do other libraries handle this situation?

@socs
Copy link
Contributor Author

socs commented Mar 17, 2023

I don't think you will find many url libs that handle link-local ipv6 zones, see this post if you're wondering why.
I think we can close this one though, since redis-rs doesn't use redis url strings internally anymore.
If one wants to connect to a ipv6 link-local address, use eg (host, port) tuple where the host part with zone id is accepted.

The more crucial fix is in 796. Without it redis-rs does not work on a ipv6 link-local redis cluster.

@socs socs closed this Mar 17, 2023
@jaymell
Copy link
Contributor

jaymell commented Mar 18, 2023

Ok, that's good to hear -- glad we finally got the simplified connection-map stuff merged, as it simplified a lot of things. I'll take a look at #796 soon. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants