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

'missing connect error' panic when connecting to IPv6-only domain using a socket bound to IPv4 address #2291

Closed
alexwl opened this issue Sep 29, 2020 · 2 comments · Fixed by #2292 or differs/Legends#5
Labels
A-client Area: client. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@alexwl
Copy link
Contributor

alexwl commented Sep 29, 2020

This example uses reqwest for simplicity:

#[tokio::main]
async fn main() -> () {
    let builder = reqwest::ClientBuilder::new();
    let builder = builder.local_address(std::net::IpAddr::from([0, 0, 0, 0]));
    let client = builder.build().unwrap();
    match client.get("http://ipv6.google.com").send().await {
        Ok(r) => {
            println!("Response: {:?}", r);
        }
        Err(e) => {
            println!("Error: {:?}", e);
        }
    }
}

Error message:

thread 'main' panicked at 'missing connect error', /home/alexwl/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.13.8/src/client/connect/http.rs:544:13

IPv6-only domains (only AAAA DNS records, not A) like ipv6.google.com, are rare, but they do exist in the wild.

The problem is that split_by_preference removes IPv6 addresses from IpAddrs when the local_addr is IPv4:

pub(super) fn split_by_preference(self, local_addr: Option<IpAddr>) -> (IpAddrs, IpAddrs) {
if let Some(local_addr) = local_addr {
let preferred = self
.iter
.filter(|addr| addr.is_ipv6() == local_addr.is_ipv6())
.collect();
(IpAddrs::new(preferred), IpAddrs::new(vec![]))
} else {
let preferring_v6 = self
.iter
.as_slice()
.first()
.map(SocketAddr::is_ipv6)
.unwrap_or(false);
let (preferred, fallback) = self
.iter
.partition::<Vec<_>, _>(|addr| addr.is_ipv6() == preferring_v6);
(IpAddrs::new(preferred), IpAddrs::new(fallback))
}
}

connect panics when the self.addrs is empty and the err is None:

async fn connect(
&mut self,
local_addr: &Option<IpAddr>,
reuse_address: bool,
) -> io::Result<TcpStream> {
let mut err = None;
for addr in &mut self.addrs {
debug!("connecting to {}", addr);
match connect(&addr, local_addr, reuse_address, self.connect_timeout)?.await {
Ok(tcp) => {
debug!("connected to {}", addr);
return Ok(tcp);
}
Err(e) => {
trace!("connect error for {}: {:?}", addr, e);
err = Some(e);
}
}
}
Err(err.take().expect("missing connect error"))
}

I think connect should return an error instead of panicking when the err is None.

For example:

match err {
    Some(e) => Err(e),
    None => Err(std::io::Error::new(
        std::io::ErrorKind::NotConnected,
        "Network unreachable"
    ))
}
@seanmonstar
Copy link
Member

Thanks for the report! I agree completely with your assessment. Would you want to make a PR fixing this?

@seanmonstar seanmonstar added A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor. C-bug Category: bug. Something is wrong. This is bad! labels Sep 29, 2020
alexwl added a commit to alexwl/hyper that referenced this issue Sep 29, 2020
alexwl added a commit to alexwl/hyper that referenced this issue Sep 29, 2020
@alexwl
Copy link
Contributor Author

alexwl commented Sep 29, 2020

@seanmonstar Thanks for the response! The PR: #2292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: client. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
2 participants