-
Notifications
You must be signed in to change notification settings - Fork 984
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 /dns protocol in multiaddr #1575
Conversation
The /dns protocol has been added to the spec and has had a de-facto meaning for years. See multiformats/multiaddr#100 This adds address parsing and encoding support for /dns to the multiaddr format library.
Address resolution for this is not yet implemented in libp2p, but other users of the |
Support for it is probably very easy to add to |
OK, I will try to do here shortly. |
The whole thing with back-translating from an redirect URL looks a bit baroque, but at least now the transport does not completely ignore IPv6 addresses resolved from a hostname in a redirect URL.
@tomaka I have added Commits c02bf8d and possibly f76331f may be too disruptive, as they change the behavior of URL-to-multiaddr translations, albeit improving it with regard to IPv6 support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -70,7 +70,7 @@ fn from_url_inner_http_ws(url: url::Url, lossy: bool) -> std::result::Result<Mul | |||
if let Ok(ip) = hostname.parse::<IpAddr>() { | |||
Protocol::from(ip) | |||
} else { | |||
Protocol::Dns4(hostname.into()) | |||
Protocol::Dns(hostname.into()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's actually a great change 🎉
@@ -371,7 +373,7 @@ fn location_to_multiaddr<T>(location: &str) -> Result<Multiaddr, Error<T>> { | |||
let mut a = Multiaddr::empty(); | |||
match url.host() { | |||
Some(url::Host::Domain(h)) => { | |||
a.push(Protocol::Dns4(h.into())) | |||
a.push(Protocol::Dns(h.into())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
The
/dns
protocol has been added to the spec and has had a de-facto meaning for years (citing multiformats/multiaddr#100)This adds address parsing and encoding support for
/dns
to the multiaddr library.