Add ClientBuilder::local_address option to bind to a local IP address #451
Conversation
CHANGELOG.md
Outdated
| @@ -5,6 +5,7 @@ | |||
| - Add `ClientBuilder::h2_prior_knowledge()` option to force HTTP2. | |||
| - Add `Response::content_length()` to get the content-length of a response. | |||
| - Enable ALPN h2 with the rustls-tls backend. | |||
| - Add `ClientBuilder::local_address` option to bind to a local IP address | |||
There was a problem hiding this comment.
Don't worry about changelog entries, I fill this in when tagging a release :)
src/async_impl/client.rs
Outdated
| @@ -325,6 +330,12 @@ impl ClientBuilder { | |||
| pub fn dns_threads(self, _threads: usize) -> ClientBuilder { | |||
| self | |||
| } | |||
|
|
|||
| /// Bind to a local IP Address | |||
| pub fn local_address(mut self, addr: Option<IpAddr>) -> ClientBuilder { | |||
There was a problem hiding this comment.
Consider an argument of addr: impl Into<Option<IpAddr>>. This way, in the common case, it reads more fluently, like builder.local_address(ip) instead of builder.local_address(Some(ip)).
src/client.rs
Outdated
| /// .local_address(Some(local_addr.into())) | ||
| /// .build().unwrap(); | ||
| /// ``` | ||
| pub fn local_address(self, addr: Option<IpAddr>) -> ClientBuilder { |
There was a problem hiding this comment.
Same thing here about impl Into<Option<IpAddr>>.
src/client.rs
Outdated
| /// | ||
| /// ``` | ||
| /// use std::net::Ipv4Addr; | ||
| /// let local_addr = Ipv4Addr::new(12, 4, 1, 8); |
There was a problem hiding this comment.
I believe this works too: IpAddr::from([12, 4, 1, 8])
src/connect.rs
Outdated
|
|
||
| pub(crate) fn local_address(&self, _addr: Option<IpAddr>) { | ||
| match self.inner { | ||
| #[cfg(not(feature = "tls"))] |
There was a problem hiding this comment.
In order to support even when TLS is enabled, we'll probably need to pass the option during construction instead.
…nnector construction
| let tls = try_!(tls.build()); | ||
|
|
||
| let mut http = http_connector()?; | ||
| http.set_local_address(local_addr.into()); |
There was a problem hiding this comment.
@seanmonstar Why is this the only place I had to use into()? The error was something along the lines of expecting std::option::Option, got type parameter when not using into()
There was a problem hiding this comment.
Actually looks like some of the builds failed with that same error. I will try adding into() to the rest.
itsHabib
force-pushed
the
itshabib-client-local-addr
branch
from
cbdf107 to
e5fc9db
Compare
|
Ready for another look whenever you are available! |
Resolves #414