search

hyperium / http

Rust HTTP types
Apache License 2.0
1.14k stars 284 forks source link

Implement `std::net::ToSocketAddrs` for `Uri` and `Authority` #530

Closed LukasKalbertodt closed 2 years ago

LukasKalbertodt commented 2 years ago

It would be immensely helpful if those impls existed. Doing this yourself is surprisingly tricky due to the exact way the existing impls for &str and (&str, u16) work.

Would you be interested in adding these impls?

seanmonstar commented 2 years ago

My initial feeling is no. It's an easy way to accidentally do a blocking DNS lookup...

LukasKalbertodt commented 2 years ago

Ok? I don't know a lot about this. Are the existing impls in std also susceptible to blocking DNS lookups?

I imagined impls added to http to just call std's impls as appropriate, and not do any lookup on their own. So the task is mainly to bring Uri into either &str or (&str, u16) form and call its impl. This is a tiny bit tricky because of user info in the authority (which has to be ignored) and the bracketed IPv6 form.

But now that I think about it, maybe implementing ToSocketAddrs is not a great idea: usually functions like Server::bind take T: ToSocketAddrs for convenience, but passing a full URI there feels wrong. And adding a resolve method also seems wrong in this library. So yeah, I will close this issue now. I still think it's surprisingly und unnecessarily difficult to turn an Uri into an IP address or socket address. But I guess this library is not responsible for solving that.