From 2e983694f641d4284c7fd6622ca7fbd866bf7740 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Thu, 27 Feb 2020 12:57:13 -0800 Subject: [PATCH] Re-enable trust-dns optional feature (#787) --- Cargo.toml | 4 +- src/connect.rs | 24 +++++----- src/dns.rs | 119 ++++++++++++++++++++++++++++--------------------- src/lib.rs | 8 ++-- 4 files changed, 85 insertions(+), 70 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5de9054..90b01ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ brotli = ["async-compression", "async-compression/brotli"] json = ["serde_json"] -#trust-dns = ["trust-dns-resolver"] +trust-dns = ["trust-dns-resolver"] stream = [] @@ -113,7 +113,7 @@ async-compression = { version = "0.3.0", default-features = false, features = [" tokio-socks = { version = "0.2", optional = true } ## trust-dns -#trust-dns-resolver = { version = "0.11", optional = true } +trust-dns-resolver = { version = "0.19", optional = true } [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] env_logger = "0.6" diff --git a/src/connect.rs b/src/connect.rs index a6557d7..f2cb647 100644 --- a/src/connect.rs +++ b/src/connect.rs @@ -19,8 +19,8 @@ use std::time::Duration; use std::mem::MaybeUninit; use pin_project_lite::pin_project; -//#[cfg(feature = "trust-dns")] -//use crate::dns::TrustDnsResolver; +#[cfg(feature = "trust-dns")] +use crate::dns::TrustDnsResolver; use crate::proxy::{Proxy, ProxyScheme}; use crate::error::BoxError; #[cfg(feature = "default-tls")] @@ -28,9 +28,9 @@ use self::native_tls_conn::NativeTlsConn; #[cfg(feature = "rustls-tls")] use self::rustls_tls_conn::RustlsTlsConn; -//#[cfg(feature = "trust-dns")] -//type HttpConnector = hyper::client::HttpConnector; -//#[cfg(not(feature = "trust-dns"))] +#[cfg(feature = "trust-dns")] +type HttpConnector = hyper::client::HttpConnector; +#[cfg(not(feature = "trust-dns"))] type HttpConnector = hyper::client::HttpConnector; #[derive(Clone)] @@ -413,14 +413,14 @@ fn into_uri(scheme: Scheme, host: Authority) -> Uri { .expect("scheme and authority is valid Uri") } -//#[cfg(feature = "trust-dns")] -//fn http_connector() -> crate::Result { -// TrustDnsResolver::new() -// .map(HttpConnector::new_with_resolver) -// .map_err(crate::error::dns_system_conf) -//} +#[cfg(feature = "trust-dns")] +fn http_connector() -> crate::Result { + TrustDnsResolver::new() + .map(HttpConnector::new_with_resolver) + .map_err(crate::error::builder) +} -//#[cfg(not(feature = "trust-dns"))] +#[cfg(not(feature = "trust-dns"))] fn http_connector() -> crate::Result { Ok(HttpConnector::new()) } diff --git a/src/dns.rs b/src/dns.rs index a76749d..6df4213 100644 --- a/src/dns.rs +++ b/src/dns.rs @@ -1,76 +1,91 @@ use std::future::Future; -use std::net::IpAddr; -use std::sync::{Arc, Mutex, Once}; -use std::{io, vec}; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{self, Poll}; +use std::io; -use futures_util::future; use hyper::client::connect::dns as hyper_dns; -use tokio; -use trust_dns_resolver::{system_conf, AsyncResolver, BackgroundLookupIp}; +use hyper::service::Service; +use tokio::sync::Mutex; +use trust_dns_resolver::{ + config::{ResolverConfig, ResolverOpts}, + lookup_ip::LookupIpIntoIter, + system_conf, AsyncResolver, TokioConnection, TokioConnectionProvider, +}; -// If instead the type were just `AsyncResolver`, it breaks the default recursion limit -// for the compiler to determine if `reqwest::Client` is `Send`. This is because `AsyncResolver` -// has **a lot** of internal generic types that pushes us over the limit. -// -// "Erasing" the internal resolver type saves us from this limit. -type ErasedResolver = Box BackgroundLookupIp + Send + Sync>; -type Background = Box + Send>; +use crate::error::BoxError; + +type SharedResolver = Arc>; + +lazy_static! { + static ref SYSTEM_CONF: io::Result<(ResolverConfig, ResolverOpts)> = system_conf::read_system_conf(); +} #[derive(Clone)] pub(crate) struct TrustDnsResolver { - inner: Arc, + state: Arc>, } -struct Inner { - background: Mutex>, - once: Once, - resolver: ErasedResolver, +enum State { + Init, + Ready(SharedResolver), } impl TrustDnsResolver { pub(crate) fn new() -> io::Result { - let (conf, opts) = system_conf::read_system_conf()?; - let (resolver, bg) = AsyncResolver::new(conf, opts); - - let resolver: ErasedResolver = Box::new(move |name| resolver.lookup_ip(name.as_str())); - let background = Mutex::new(Some(Box::new(bg) as Background)); - let once = Once::new(); + SYSTEM_CONF.as_ref().map_err(|e| { + io::Error::new(e.kind(), format!("error reading DNS system conf: {}", e)) + })?; + // At this stage, we might not have been called in the context of a + // Tokio Runtime, so we must delay the actual construction of the + // resolver. Ok(TrustDnsResolver { - inner: Arc::new(Inner { - background, - once, - resolver, - }), + state: Arc::new(Mutex::new(State::Init)), }) } } -impl hyper_dns::Resolve for TrustDnsResolver { - type Addrs = vec::IntoIter; - type Future = Box> + Send>; +impl Service for TrustDnsResolver { + type Response = LookupIpIntoIter; + type Error = BoxError; + type Future = Pin> + Send>>; - fn resolve(&self, name: hyper_dns::Name) -> Self::Future { - let inner = self.inner.clone(); - Box::new(future::lazy(move || { - inner.once.call_once(|| { - // The `bg` (background) future needs to be spawned onto an executor, - // but a `reqwest::Client` may be constructed before an executor is ready. - // So, the `bg` future cannot be spawned *until* the executor starts to - // `poll` this future. - let bg = inner - .background - .lock() - .expect("resolver background lock") - .take() - .expect("background only taken once"); + fn poll_ready(&mut self, _: &mut task::Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } - tokio::spawn(bg); - }); + fn call(&mut self, name: hyper_dns::Name) -> Self::Future { + let resolver = self.clone(); + Box::pin(async move { + let mut lock = resolver.state.lock().await; - (inner.resolver)(name) - .map(|lookup| lookup.iter().collect::>().into_iter()) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string())) - })) + let resolver = match &*lock { + State::Init => { + let resolver = new_resolver(tokio::runtime::Handle::current()).await?; + *lock = State::Ready(resolver.clone()); + resolver + }, + State::Ready(resolver) => resolver.clone(), + }; + + // Don't keep lock once the resolver is constructed, otherwise + // only one lookup could be done at a time. + drop(lock); + + let lookup = resolver.lookup_ip(name.as_str()).await?; + Ok(lookup.into_iter()) + }) } } + +/// Takes a `Handle` argument as an indicator that it must be called from +/// within the context of a Tokio runtime. +async fn new_resolver(handle: tokio::runtime::Handle) -> Result { + let (config, opts) = SYSTEM_CONF + .as_ref() + .expect("can't construct TrustDnsResolver if SYSTEM_CONF is error") + .clone(); + let resolver = AsyncResolver::new(config, opts, handle).await?; + Ok(Arc::new(resolver)) +} diff --git a/src/lib.rs b/src/lib.rs index c267157..99cc250 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,6 +177,8 @@ //! - **json**: Provides serialization and deserialization for JSON bodies. //! - **stream**: Adds support for `futures::Stream`. //! - **socks**: Provides SOCKS5 proxy support. +//! - **trust-dns**: Enables a trust-dns async resolver instead of default +//! threadpool using `getaddrinfo`. //! //! //! [hyper]: http://hyper.rs @@ -188,8 +190,6 @@ //! [redirect]: crate::redirect //! [Proxy]: ./struct.Proxy.html //! [cargo-features]: https://doc.rust-lang.org/stable/cargo/reference/manifest.html#the-features-section -////! - **trust-dns**: Enables a trust-dns async resolver instead of default -////! threadpool using `getaddrinfo`. macro_rules! if_wasm { ($($item:item)*) => {$( @@ -295,8 +295,8 @@ if_hyper! { mod connect; #[cfg(feature = "cookies")] pub mod cookie; - //#[cfg(feature = "trust-dns")] - //mod dns; + #[cfg(feature = "trust-dns")] + mod dns; mod proxy; pub mod redirect; #[cfg(feature = "__tls")]