Repository: seanmonstar/reqwest (language: rust)

Issue:
DNS bypass
It would be nice if there were a way we could bypass DNS resolution and connect directly to an IP address we specify during the request.  Something like...

`client.skip_dns('127.0.0.1').get("https:/google.com").send()`

SNI / cert validation could still work off the domain in the URI.


Discussion / hints:
That seems like a fair desire, unsure about the best API to expose.
So I toyed with this a little by modifying the async_impl/request struct like: 

```rust
pub struct Request {
    ip: Option<std::net::Ipv4Addr>,
    method: Method,
    url: Url,
    headers: HeaderMap,
    body: Option<Body>,
}
```
and then tried compiling / updating over and over again, making the necessary changes at each step to get past whatever errors were introduced.  I tried to stay true to how the rest of the library treated an instance of a `Request` at each step, but I got stuck at... 
```
C:\dns_bypass (master -> origin)λ cargo build
   Compiling hyper v0.12.27 (C:\dns_bypass\.cargo\hyper)
error[E0308]: mismatched types
   --> .cargo\hyper\src\proto\h1\dispatch.rs:464:47
    |
464 |                         Ok(Async::Ready(Some((ip, head, body))))
    |                                               ^^ expected struct proto::MessageHead, found struct http::Ip
    |
    = note: expected type proto::MessageHead<proto::RequestLine>
               found type http::Ip
```

I didn't think it should be merged into request "headers" as its rather part of the IP header, but taking this approach meant I needed to modify the h2, hyper, and http libraries as well, and possibly more?
Before you go to far down that rabbit hole, I think it'd be best to think about the exposed API to the user. The implementation likely doesn't need to bother touching h2/hyper/http, and would probably just adjust reqwest's `connect.rs` stuff...
Ahh.  Well it's too late for me I already went down the rabbit hole!  Anyhow, I think you're right it's best to avoid changes on any dependent libraries.  I'll try again sometime this week hopefully and report back.

For the API, I already don't like .skip_dns() because "DNS" is just one of the ways by which an IP could be resolved.  I do like that the default behavior is to attempt resolution, since I believe the users of reqwest would prefer not to worry about it.  

Perhaps skip_resolution would be better?  I still like 'skip', since it implies that the standard behavior is to attempt IP resolution, and shows that by calling that function, no subsequent resolution would occur.  

I don't think support for non-IP protocols would be anything in-scope for this library, so accepting an std::net::IpAddr would be best?

Having anything more than a .skip_resolution(std::net::IpAddr) builder method to support this behavior, I think, would be unnecessary.
Does curl and other libraries provide an easy option to do this?
curl has `--resolve <host:port:address> Resolve the host+port to this address`
wget does similar by over-riding the host header, but I imagine that isn't going to be compatible with SNI?

Now that I'm looking at it, I can see why curl would have a --resolve as opposed to a skip_resolution, as it would matter with redirects.  If you skip resolution on the first host, but then get redirected, do you then use DNS for the redirect if it goes to a new host?

In this case, you'd want to simply set up a resolution mask where host A maps to IP A, and let all other names resolve normally.  In curls example, they are allowing host A to map to IP A, only on a specific port, otherwise use DNS as normal (I think even for host A on a different port).

That seems a bit more complex than what I was envisioning, where I would simply want to establish a static Host->IP resolution, ignoring the port.

Now that I've seen how curl does it, I think I'd want to leave DNS resolution "on" for any names that weren't already defined, and have a `.resolve(&str, std::net::IpAddr)` to define name->ip's that would be usable prior to issuing each request.  Each .resolve() would append to an internal dns cache that would then be checked prior to doing DNS resolution.  I anticipate that there wouldn't be so many DNS over-rides that one would want .resolve() to accept an array of names->IP's.  

Since the default resolver is already replaceable with a custom one, maybe all this is really a bogus feature request?  All I'd need to do is figure out how to mimic the default resolver and then add my very basic "mask" in front of that resolver for an early return.  I'm not sure how to do that, but already it seems that would be a more acceptable direction to take, rather than have all reqwest requests perform a dns cache check for something that probably doesn't exist 99% of the time.
Maybe I was confused, I'm looking now at the reqwest source and I don't see how to set my own resolver on either the client or the request.  Was that part of hyper?
@seanmonstar Can you reopen this issue? Specifying custom DNS resolution is quite helpful.
I was the one that closed it, re-opening now.
A quite useful feature.
That's really useful, especially for us to bypass DNS spoofing.
It seems that it involves modifying hyper’s source code for per-request DNS bypassing to work.
I think that the API presented to users for this could also be useful for #39.
I've played around with an implementation of this and it's definitely bubbling up to cause some ugliness so I think there are some choices to make. 

Currently the HttpConnector is an enum with either the GaiResolver or the TrustDNS resolver. Here's my attempt to make a generic wrapper resolver that would allow a user to add specific overrides:

```
    async fn new_wrapping_resolver<R, U>(
        mut wrapped_resolver: R,
        overrides: std::sync::Arc<HashMap<String, SocketAddr>>,
    ) -> impl Service<Name, Response = impl Iterator<Item = SocketAddr>>
    where
        R: Service<Name, Response = U> + 'static,
        U: Iterator<Item = SocketAddr>,
    {
        type EitherOverrideOrWrapped<U> = itertools::Either<std::iter::Once<SocketAddr>, U>;
        tower::service_fn(move |name: Name| {
            if let Some(overridden) = overrides.get(name.as_str()) {
                return futures_util::future::ok::<EitherOverrideOrWrapped<U>, R::Error>(
                    EitherOverrideOrWrapped::Left(std::iter::once(*overridden)),
                )
                .left_future();
            } else {
                return wrapped_resolver
                    .call(name)
                    .map(|result| result.map(|iter| EitherOverrideOrWrapped::Right(iter)))
                    .right_future();
            }
        })
    }

```

This is the first time I've written this kind of futures + iterators code so this may be unidiomatic (happy to rework if there are better options). NB: this adds tower and itertools as new dependencies of reqwest.

The implication of this though is that the resolver becomes a new unnamed type, and there is an overhead for clients that are not using overrides at all. 

Other options for a design here that I can see:
* A new WrappedResolver Struct generic over the resolver. Then two new constructors for HttpConnector with names `new_gai_with_overrides()` and `new_trust_dns_with_overrides()`. Then a third variant of HttpConnector with a generic inner type. I don't know enough about how the different resolvers are used to know whether this has more implications and this means two new functions per resolver being added.
* Make HttpConnector a generic new_type over hyper::client::HttpConnector rather than an enum. This would simplify connect.rs (I think) but would presumably impact the rest of the crate.

If anyone has any input on how this could be cleanly implemented I'm happy to take a swing and get a PR up. 
As for user API my suggestion would be:

```
    reqwest::Client::builder()
        .resolve("example.com", "127.0.0.1")
        .resolve("example2.com", "127.0.0.1")
        .build()?
```
This would be in keeping with curls API. 

There is a question as to whether we want to allow custom resolves to map the same domain to multiple IP addresses. The use case I'm motivated by here is for testing against a local running TLS server so this isn't an issue but there may be other use cases that would prefer multiple potential IPs.


Fix patch:
diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs
index 2e80479fb..f7d669504 100644
--- a/src/async_impl/client.rs
+++ b/src/async_impl/client.rs
@@ -1,9 +1,9 @@
 #[cfg(any(feature = "native-tls", feature = "__rustls",))]
 use std::any::Any;
-use std::convert::TryInto;
 use std::net::IpAddr;
 use std::sync::Arc;
 use std::time::Duration;
+use std::{collections::HashMap, convert::TryInto, net::SocketAddr};
 use std::{fmt, str};
 
 use bytes::Bytes;
@@ -107,6 +107,7 @@ struct Config {
     trust_dns: bool,
     error: Option<crate::Error>,
     https_only: bool,
+    dns_overrides: HashMap<String, SocketAddr>,
 }
 
 impl Default for ClientBuilder {
@@ -164,6 +165,7 @@ impl ClientBuilder {
                 #[cfg(feature = "cookies")]
                 cookie_store: None,
                 https_only: false,
+                dns_overrides: HashMap::new(),
             },
         }
     }
@@ -194,9 +196,21 @@ impl ClientBuilder {
             }
 
             let http = match config.trust_dns {
-                false => HttpConnector::new_gai(),
+                false => {
+                    if config.dns_overrides.is_empty() {
+                        HttpConnector::new_gai()
+                    } else {
+                        HttpConnector::new_gai_with_overrides(config.dns_overrides)
+                    }
+                }
                 #[cfg(feature = "trust-dns")]
-                true => HttpConnector::new_trust_dns()?,
+                true => {
+                    if config.dns_overrides.is_empty() {
+                        HttpConnector::new_trust_dns()?
+                    } else {
+                        HttpConnector::new_trust_dns_with_overrides(config.dns_overrides)?
+                    }
+                }
                 #[cfg(not(feature = "trust-dns"))]
                 true => unreachable!("trust-dns shouldn't be enabled unless the feature is"),
             };
@@ -1037,6 +1051,19 @@ impl ClientBuilder {
         self.config.https_only = enabled;
         self
     }
+
+    /// Override DNS resolution for specific domains to particular IP addresses.
+    ///
+    /// Warning
+    ///
+    /// Since the DNS protocol has no notion of ports, if you wish to send
+    /// traffic to a particular port you must include this port in the URL
+    /// itself, any port in the overridden addr will be ignored and traffic sent
+    /// to the conventional port for the given scheme (e.g. 80 for http).
+    pub fn resolve(mut self, domain: &str, addr: SocketAddr) -> ClientBuilder {
+        self.config.dns_overrides.insert(domain.to_string(), addr);
+        self
+    }
 }
 
 type HyperClient = hyper::Client<Connector, super::body::ImplStream>;
@@ -1350,6 +1377,10 @@ impl Config {
         {
             f.field("tls_backend", &self.tls);
         }
+
+        if !self.dns_overrides.is_empty() {
+            f.field("dns_overrides", &self.dns_overrides);
+        }
     }
 }
 
diff --git a/src/connect.rs b/src/connect.rs
index 9f20347bc..375d40250 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -3,21 +3,24 @@ use futures_util::future::Either;
 use http::header::HeaderValue;
 use http::uri::{Authority, Scheme};
 use http::Uri;
-use hyper::client::connect::{Connected, Connection};
+use hyper::client::connect::{
+    dns::{GaiResolver, Name},
+    Connected, Connection,
+};
 use hyper::service::Service;
 #[cfg(feature = "native-tls-crate")]
 use native_tls_crate::{TlsConnector, TlsConnectorBuilder};
 use tokio::io::{AsyncRead, AsyncWrite, ReadBuf};
 
 use pin_project_lite::pin_project;
-use std::future::Future;
-use std::io;
 use std::io::IoSlice;
 use std::net::IpAddr;
 use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 use std::time::Duration;
+use std::{collections::HashMap, io};
+use std::{future::Future, net::SocketAddr};
 
 #[cfg(feature = "default-tls")]
 use self::native_tls_conn::NativeTlsConn;
@@ -31,8 +34,11 @@ use crate::proxy::{Proxy, ProxyScheme};
 #[derive(Clone)]
 pub(crate) enum HttpConnector {
     Gai(hyper::client::HttpConnector),
+    GaiWithDnsOverrides(hyper::client::HttpConnector<DnsResolverWithOverrides<GaiResolver>>),
     #[cfg(feature = "trust-dns")]
     TrustDns(hyper::client::HttpConnector<TrustDnsResolver>),
+    #[cfg(feature = "trust-dns")]
+    TrustDnsWithOverrides(hyper::client::HttpConnector<DnsResolverWithOverrides<TrustDnsResolver>>),
 }
 
 impl HttpConnector {
@@ -40,6 +46,14 @@ impl HttpConnector {
         Self::Gai(hyper::client::HttpConnector::new())
     }
 
+    pub(crate) fn new_gai_with_overrides(overrides: HashMap<String, SocketAddr>) -> Self {
+        let gai = hyper::client::connect::dns::GaiResolver::new();
+        let overridden_resolver = DnsResolverWithOverrides::new(gai, overrides);
+        Self::GaiWithDnsOverrides(hyper::client::HttpConnector::new_with_resolver(
+            overridden_resolver,
+        ))
+    }
+
     #[cfg(feature = "trust-dns")]
     pub(crate) fn new_trust_dns() -> crate::Result<HttpConnector> {
         TrustDnsResolver::new()
@@ -47,6 +61,17 @@ impl HttpConnector {
             .map(Self::TrustDns)
             .map_err(crate::error::builder)
     }
+
+    #[cfg(feature = "trust-dns")]
+    pub(crate) fn new_trust_dns_with_overrides(
+        overrides: HashMap<String, SocketAddr>,
+    ) -> crate::Result<HttpConnector> {
+        TrustDnsResolver::new()
+            .map(|resolver| DnsResolverWithOverrides::new(resolver, overrides))
+            .map(hyper::client::HttpConnector::new_with_resolver)
+            .map(Self::TrustDnsWithOverrides)
+            .map_err(crate::error::builder)
+    }
 }
 
 macro_rules! impl_http_connector {
@@ -57,8 +82,11 @@ macro_rules! impl_http_connector {
                 fn $name(&mut self, $($par_name: $par_type),*)$( -> $return)? {
                     match self {
                         Self::Gai(resolver) => resolver.$name($($par_name),*),
+                        Self::GaiWithDnsOverrides(resolver) => resolver.$name($($par_name),*),
                         #[cfg(feature = "trust-dns")]
                         Self::TrustDns(resolver) => resolver.$name($($par_name),*),
+                        #[cfg(feature = "trust-dns")]
+                        Self::TrustDnsWithOverrides(resolver) => resolver.$name($($par_name),*),
                     }
                 }
             )+
@@ -77,29 +105,55 @@ impl Service<Uri> for HttpConnector {
     type Response = <hyper::client::HttpConnector as Service<Uri>>::Response;
     type Error = <hyper::client::HttpConnector as Service<Uri>>::Error;
     #[cfg(feature = "trust-dns")]
-    type Future = Either<
-        <hyper::client::HttpConnector as Service<Uri>>::Future,
-        <hyper::client::HttpConnector<TrustDnsResolver> as Service<Uri>>::Future,
-    >;
+    type Future =
+        Either<
+            Either<
+                <hyper::client::HttpConnector as Service<Uri>>::Future,
+                <hyper::client::HttpConnector<DnsResolverWithOverrides<GaiResolver>> as Service<
+                    Uri,
+                >>::Future,
+            >,
+            Either<
+                    <hyper::client::HttpConnector<TrustDnsResolver> as Service<Uri>>::Future,
+                <hyper::client::HttpConnector<DnsResolverWithOverrides<TrustDnsResolver>> as Service<Uri>>::Future
+                 >
+        >;
     #[cfg(not(feature = "trust-dns"))]
-    type Future = Either<
-        <hyper::client::HttpConnector as Service<Uri>>::Future,
-        <hyper::client::HttpConnector as Service<Uri>>::Future,
-    >;
+    type Future =
+        Either<
+            Either<
+                <hyper::client::HttpConnector as Service<Uri>>::Future,
+                <hyper::client::HttpConnector<DnsResolverWithOverrides<GaiResolver>> as Service<
+                    Uri,
+                >>::Future,
+            >,
+            Either<
+                <hyper::client::HttpConnector as Service<Uri>>::Future,
+                <hyper::client::HttpConnector as Service<Uri>>::Future,
+            >,
+        >;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
         match self {
             Self::Gai(resolver) => resolver.poll_ready(cx),
+            Self::GaiWithDnsOverrides(resolver) => resolver.poll_ready(cx),
             #[cfg(feature = "trust-dns")]
             Self::TrustDns(resolver) => resolver.poll_ready(cx),
+            #[cfg(feature = "trust-dns")]
+            Self::TrustDnsWithOverrides(resolver) => resolver.poll_ready(cx),
         }
     }
 
     fn call(&mut self, dst: Uri) -> Self::Future {
         match self {
-            Self::Gai(resolver) => Either::Left(resolver.call(dst)),
+            Self::Gai(resolver) => Either::Left(Either::Left(resolver.call(dst))),
+            Self::GaiWithDnsOverrides(resolver) => Either::Left(Either::Right(resolver.call(dst))),
             #[cfg(feature = "trust-dns")]
-            Self::TrustDns(resolver) => Either::Right(resolver.call(dst)),
+            Self::TrustDns(resolver) => Either::Right(Either::Left(resolver.call(dst))),
+            #[cfg(feature = "trust-dns")]
+            Self::TrustDnsWithOverrides(resolver) => {
+                Either::Right(Either::Right(resolver.call(dst)))
+            }
         }
     }
 }
@@ -908,6 +962,103 @@ mod socks {
     }
 }
 
+pub(crate) mod itertools {
+    pub(crate) enum Either<A, B> {
+        Left(A),
+        Right(B),
+    }
+
+    impl<A, B> Iterator for Either<A, B>
+    where
+        A: Iterator,
+        B: Iterator<Item = <A as Iterator>::Item>,
+    {
+        type Item = <A as Iterator>::Item;
+
+        fn next(&mut self) -> Option<Self::Item> {
+            match self {
+                Either::Left(a) => a.next(),
+                Either::Right(b) => b.next(),
+            }
+        }
+    }
+}
+
+pin_project! {
+    pub(crate) struct WrappedResolverFuture<Fut> {
+        #[pin]
+        fut: Fut,
+    }
+}
+
+impl<Fut, FutOutput, FutError> std::future::Future for WrappedResolverFuture<Fut>
+where
+    Fut: std::future::Future<Output = Result<FutOutput, FutError>>,
+    FutOutput: Iterator<Item = SocketAddr>,
+{
+    type Output = Result<itertools::Either<FutOutput, std::iter::Once<SocketAddr>>, FutError>;
+
+    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        let this = self.project();
+        this.fut
+            .poll(cx)
+            .map(|result| result.map(itertools::Either::Left))
+    }
+}
+
+#[derive(Clone)]
+pub(crate) struct DnsResolverWithOverrides<Resolver>
+where
+    Resolver: Clone,
+{
+    dns_resolver: Resolver,
+    overrides: Arc<HashMap<String, SocketAddr>>,
+}
+
+impl<Resolver: Clone> DnsResolverWithOverrides<Resolver> {
+    fn new(dns_resolver: Resolver, overrides: HashMap<String, SocketAddr>) -> Self {
+        DnsResolverWithOverrides {
+            dns_resolver,
+            overrides: Arc::new(overrides),
+        }
+    }
+}
+
+impl<Resolver, Iter> Service<Name> for DnsResolverWithOverrides<Resolver>
+where
+    Resolver: Service<Name, Response = Iter> + Clone,
+    Iter: Iterator<Item = SocketAddr>,
+{
+    type Response = itertools::Either<Iter, std::iter::Once<SocketAddr>>;
+    type Error = <Resolver as Service<Name>>::Error;
+    type Future = Either<
+        WrappedResolverFuture<<Resolver as Service<Name>>::Future>,
+        futures_util::future::Ready<
+            Result<itertools::Either<Iter, std::iter::Once<SocketAddr>>, Self::Error>,
+        >,
+    >;
+
+    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
+        self.dns_resolver.poll_ready(cx)
+    }
+
+    fn call(&mut self, name: Name) -> Self::Future {
+        match self.overrides.get(name.as_str()) {
+            Some(dest) => {
+                let fut = futures_util::future::ready(Ok(itertools::Either::Right(
+                    std::iter::once(dest.to_owned()),
+                )));
+                Either::Right(fut)
+            }
+            None => {
+                let resolver_fut = self.dns_resolver.call(name);
+                let y = WrappedResolverFuture { fut: resolver_fut };
+                Either::Left(y)
+            }
+        }
+    }
+}
+
 mod verbose {
     use hyper::client::connect::{Connected, Connection};
     use std::fmt;


Test patch:
diff --git a/tests/client.rs b/tests/client.rs
index 2fbddc279..4890b347f 100644
--- a/tests/client.rs
+++ b/tests/client.rs
@@ -167,6 +167,54 @@ async fn body_pipe_response() {
     assert_eq!(res2.status(), reqwest::StatusCode::OK);
 }
 
+#[tokio::test]
+async fn overridden_dns_resolution_with_gai() {
+    let _ = env_logger::builder().is_test(true).try_init();
+    let server = server::http(move |_req| async { http::Response::new("Hello".into()) });
+
+    let overridden_domain = "rust-lang.org";
+    let url = format!(
+        "http://{}:{}/domain_override",
+        overridden_domain,
+        server.addr().port()
+    );
+    let client = reqwest::Client::builder()
+        .resolve(overridden_domain, server.addr())
+        .build()
+        .expect("client builder");
+    let req = client.get(&url);
+    let res = req.send().await.expect("request");
+
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+    let text = res.text().await.expect("Failed to get text");
+    assert_eq!("Hello", text);
+}
+
+#[cfg(feature = "trust-dns")]
+#[tokio::test]
+async fn overridden_dns_resolution_with_trust_dns() {
+    let _ = env_logger::builder().is_test(true).try_init();
+    let server = server::http(move |_req| async { http::Response::new("Hello".into()) });
+
+    let overridden_domain = "rust-lang.org";
+    let url = format!(
+        "http://{}:{}/domain_override",
+        overridden_domain,
+        server.addr().port()
+    );
+    let client = reqwest::Client::builder()
+        .resolve(overridden_domain, server.addr())
+        .trust_dns(true)
+        .build()
+        .expect("client builder");
+    let req = client.get(&url);
+    let res = req.send().await.expect("request");
+
+    assert_eq!(res.status(), reqwest::StatusCode::OK);
+    let text = res.text().await.expect("Failed to get text");
+    assert_eq!("Hello", text);
+}
+
 #[cfg(any(feature = "native-tls", feature = "__rustls",))]
 #[test]
 fn use_preconfigured_tls_with_bogus_backend() {