hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.58k stars 1.6k forks source link

Expose separate ipv4/ipv6 local address binding in HttpConnector #2543

Open let4be opened 3 years ago

let4be commented 3 years ago

As of right now it's not possible to separately set local address for ipv4 and ipv6(only together) It would make sense to expose those separate(say I wanna bind local address just for ipv4 or just for ipv6)

consider modifying https://github.com/hyperium/hyper/blob/e79d09396da955f235c267832312543d8230d867/src/client/connect/http.rs#L171 https://github.com/hyperium/hyper/blob/e79d09396da955f235c267832312543d8230d867/src/client/connect/http.rs#L187

let4be commented 3 years ago

ops... nvm

let4be commented 3 years ago

On the other hand it could be useful providing less confusing API in the next major version, for example by exposing set_local_ipv4 set_local_ipv6 separately

Existing API usage looks pretty ugly if you wanna do something like this

if self.addr_ipv4.is_some() ^ self.addr_ipv6.is_some() {
    if self.addr_ipv4.is_some() {
        http.set_local_address(Some(IpAddr::V4(self.addr_ipv4.unwrap())));
    } else if self.addr_ipv6.is_some() {
        http.set_local_address(Some(IpAddr::V6(self.addr_ipv6.unwrap())));
    }
} else if self.addr_ipv4.is_some() && self.addr_ipv6.is_some() {
    http.set_local_addresses(self.addr_ipv4.unwrap(), self.addr_ipv6.unwrap());
}

instead of short and easy

http.set_local_ipv4(self.addr_ipv4);
http.set_local_ipv6(self.addr_ipv6);

could be useful when exposing local address binding logic from a library(that uses hyper) to be at user's discretion

as an alternative we could make set_local_addresses accept Option instead of direct address values, this essentially solves the problem as well

nox commented 3 years ago

Note that for now, you don't need all those unwraps:

match (self.addr_ipv4, self.addr_ipv6) {
    (Some(ipv4), Some(ipv6)) => http.set_local_addresses(ipv4, ipv6),
    (Some(ipv4), None) => http.set_local_address(Some(IpAddr::V4(ipv4))),
    (None, Some(ipv6)) => http.set_local_address(Some(IpAddr::V6(ipv6))),
    (None, None) => {}
}