The logic of `<str as async_std::net::ToSocketAddrs>` is very redundant

[iv4n]

When I connect to a domain:port

async_std::net::TcpStream::connect("google.com:80");

The internal logic of async_std::net::ToSockstAddrs::to_socket_addrs goes like this

addrs.to_socket_addrs
    -> addrs.parse::<std::net::SocketAddr>
        -> read_socket_addr
            -> read_socket_addr_v4
            -> read_socket_addr_v6
    -> std::net::ToSocketAddrs::to_socket_addrs(addrs)
        -> addrs.parse::<std::net::SocketAddr>
            -> read_socket_addr
                -> read_socket_addr_v4
                -> read_socket_addr_v6
        -> resolve_socket_addr(addrs.try_into::<LookupHost>())

The logic makes nonsense judgments for socket_v4 and socket_v6 for twice, so what's the purpose?

And is the following usage better?

let host = dns_lookup::lookup_host("google.com:80")?;
let target: Vec<SocketAddr> = host
    .into_iter()
    .map(|h| SocketAddr::new(h, 80))
    .collect();
async_std::TcpStream::connect(target);

[steffahn]

The rationale is probably that when the &str provided is parseable as an address without doing any DNS lookup then the result should be returned immediately without any overhead. The fallback then is using std::net::to_socket_addrs which immediately comes with the prize copying the &str into a String, letting the async_std runtime execute the potentially blocking call in a different thread, etc. That’s the reason why std::net::to_socket_addrs is not used unconditionally.

On the other hand, of course the second attempt to parse the &str as a SocketAddr is 100% redundant. My best guess why this happens is that the standard library does not expose any way to do the dns lookup without trying to parse it directly first. Also, this redundant parsing attempt is probably (Disclaimer: I have no benchmark on this) such a small overhead that it’s not worth adding extra dependencies like dns_lookup to async_std (the parse function ought to be able to find out pretty damn quickly that the beginning of such an address does not look like an IP address; and this overhead is only in the case where you definitely have to do the DNS lookup so percentage-wise it might be negligibly small).

[steffahn]

@iv4n

And is the following usage better?

let host = dns_lookup::lookup_host("google.com:80")?;
let target: Vec<SocketAddr> = host
    .into_iter()
    .map(|h| SocketAddr::new(h, 80))
    .collect();
async_std::TcpStream::connect(target);

This usage is fundamentally different since (I think) dns_lookup::lookup_host is a (potentially) blocking call, so it should not be made in an async context.

Also, the way you call async_std::TcpStream::connect does not work since you don’t _.await the future and don’t inspect the Result for errors either.

[iv4n]

Yeah, I think u r right, thanks for your reply

This implementation is not neat, why not exposing a DNS resolve interface in std lib? I have found this discuss(https://github.com/rust-lang/rust/issues/27705), but I don't understand why they think it's unstable, eventhough the internal implementations are still kept

@steffahn

Also, the way you call async_std::TcpStream::connect does not work since you don’t _.await the future and don’t inspect the Result for errors either.

Yeah I know how to call it properly (it's just a demo)