fix(volo-http): fix error on ipv6 literal address resolving (#597)

* fix(volo-http): fix error on ipv6 literal address resolving

In previous implementation, the client cannot resolve IPv6 literal
address in URI like `http://[::1]:8080/` and it will throw
`BadHostName` without any context.

This commit fixes that and adds context for errors like `BadScheme`
and `BadHostName`, which will now print the error message along with
its content, it should make troubleshooting easier.

* chore(volo-http): bump volo-http to 0.4.1

---------

Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>

Author: yukiiiteru

Cargo.lock

@@ -21,21 +21,6 @@ async fn main() -> Result<(), BoxError> {
         .finish();
     tracing::subscriber::set_global_default(subscriber).expect("setting default subscriber failed");
 
-    // simple `get` function with dns resolve
-    println!(
-        "{}",
-        get("http://httpbin.org/get").await?.into_string().await?
-    );
-
-    // HTTPS `get`
-    #[cfg(feature = "__tls")]
-    {
-        println!(
-            "{}",
-            get("https://httpbin.org/get").await?.into_string().await?
-        );
-    }
-
     // create client by builder
     let client = {
         let mut builder = ClientBuilder::new().layer_outer_front(TargetLayer::new_address(
@@ -48,29 +33,6 @@ async fn main() -> Result<(), BoxError> {
         builder.build()?
     };
 
-    // set host and override the default one
-    println!(
-        "{}",
-        client
-            .request_builder()
-            .host("httpbin.org")
-            .uri("/get")
-            .send()
-            .await?
-            .into_string()
-            .await?
-    );
-
-    println!(
-        "{}",
-        client
-            .get("http://127.0.0.1:8080/")
-            .send()
-            .await?
-            .into_string()
-            .await?
-    );
-
     // use default target address
     println!(
         "{:?}",
@@ -108,6 +70,15 @@ async fn main() -> Result<(), BoxError> {
             .into_string()
             .await?
     );
+    println!(
+        "{}",
+        client
+            .get("http://[::1]:8080/")
+            .send()
+            .await?
+            .into_string()
+            .await?
+    );
 
     // invalid request because there is no target address
     println!(
@@ -119,5 +90,20 @@ async fn main() -> Result<(), BoxError> {
             .expect_err("this request should fail"),
     );
 
+    // simple `get` function with dns resolve
+    println!(
+        "{}",
+        get("http://httpbin.org/get").await?.into_string().await?
+    );
+
+    // HTTPS `get`
+    #[cfg(feature = "__tls")]
+    {
+        println!(
+            "{}",
+            get("https://httpbin.org/get").await?.into_string().await?
+        );
+    }
+
     Ok(())
 }

examples/src/http/example-http-client.rs

@@ -1,6 +1,6 @@
 [package]
 name = "volo-http"
-version = "0.4.0"
+version = "0.4.1"
 edition.workspace = true
 homepage.workspace = true
 repository.workspace = true

volo-http/Cargo.toml

@@ -4,7 +4,7 @@
 
 use std::{
     net::{IpAddr, SocketAddr},
-    ops::Deref,
+    ops::{Deref, Index},
     sync::Arc,
 };
 
@@ -61,6 +61,14 @@ impl DnsResolver {
 
     /// Resolve a host to an IP address.
     pub async fn resolve(&self, host: &str) -> Option<IpAddr> {
+        let host = {
+            let bytes = host.as_bytes();
+            match (bytes.first(), bytes.last()) {
+                (Some(b'['), Some(b']')) => host.index(1..host.len() - 1),
+                _ => host,
+            }
+        };
+
         // Note that the Resolver will try to parse the host as an IP address first, so we don't
         // need to parse it manually.
         self.resolver.lookup_ip(host).await.ok()?.into_iter().next()
@@ -144,7 +152,9 @@ impl Discover for DnsResolver {
             return Ok(vec![Arc::new(instance)]);
         };
         tracing::error!("[Volo-HTTP] DnsResolver: no address resolved");
-        Err(LoadBalanceError::Discover(Box::new(bad_host_name())))
+        Err(LoadBalanceError::Discover(Box::new(bad_host_name(
+            endpoint.service_name(),
+        ))))
     }
 
     fn key(&self, endpoint: &Endpoint) -> Self::Key {
@@ -155,3 +165,31 @@ impl Discover for DnsResolver {
         None
     }
 }
+
+#[cfg(test)]
+mod dns_tests {
+    use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
+
+    use crate::client::dns::DnsResolver;
+
+    #[tokio::test]
+    async fn static_resolve() {
+        let resolver = DnsResolver::default();
+
+        assert_eq!(
+            resolver.resolve("127.0.0.1").await,
+            Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))),
+        );
+        assert_eq!(
+            resolver.resolve("::1").await,
+            Some(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))),
+        );
+        assert_eq!(
+            resolver.resolve("[::1]").await,
+            Some(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))),
+        );
+        assert_eq!(resolver.resolve("[::1").await, None);
+        assert_eq!(resolver.resolve("::1]").await, None);
+        assert_eq!(resolver.resolve("[::1]:8080").await, None);
+    }
+}

volo-http/src/client/dns.rs

@@ -17,7 +17,7 @@ use crate::{
     client::dns::Port,
     context::ClientContext,
     error::{
-        client::{bad_address, bad_scheme, no_address, Result},
+        client::{bad_scheme, no_address, port_unavailable, scheme_unavailable, Result},
         ClientError,
     },
     utils::consts,
@@ -105,7 +105,7 @@ fn check_scheme(scheme: &Scheme) -> Result<()> {
         #[cfg(not(feature = "__tls"))]
         {
             tracing::error!("[Volo-HTTP] https is not allowed when feature `tls` is not enabled");
-            return Err(bad_scheme());
+            return Err(bad_scheme(scheme.clone()));
         }
         #[cfg(feature = "__tls")]
         return Ok(());
@@ -114,7 +114,7 @@ fn check_scheme(scheme: &Scheme) -> Result<()> {
         return Ok(());
     }
     tracing::error!("[Volo-HTTP] scheme '{scheme}' is unsupported");
-    Err(bad_scheme())
+    Err(bad_scheme(scheme.clone()))
 }
 
 impl Target {
@@ -220,7 +220,7 @@ impl Target {
             Some(rt) => rt,
             None => {
                 tracing::warn!("[Volo-HTTP] set scheme to an empty target or uds is invalid");
-                return Err(bad_address());
+                return Err(scheme_unavailable());
             }
         };
         check_scheme(&scheme)?;
@@ -237,7 +237,7 @@ impl Target {
             Some(rt) => rt,
             None => {
                 tracing::warn!("[Volo-HTTP] set port to an empty target or uds is invalid");
-                return Err(bad_address());
+                return Err(port_unavailable());
             }
         };
         rt.port = port;
@@ -344,6 +344,7 @@ impl Apply<ClientContext> for Target {
                         callee.set_address(Address::Ip(sa));
                     }
                     RemoteHost::Name(host) => {
+                        println!("Target::apply, host = {host}");
                         let port = rt.port;
                         tracing::trace!("[Volo-HTTP] Target::apply: set target to {host}:{port}");
                         let callee = cx.rpc_info_mut().callee_mut();

volo-http/src/client/target.rs

@@ -96,7 +96,7 @@ impl UnaryService<PeerInfo> for HttpMakeConnection {
         match self {
             Self::Plain(plain) => {
                 if req.scheme != Scheme::HTTP {
-                    return Err(bad_scheme());
+                    return Err(bad_scheme(req.scheme));
                 }
                 plain.call(req).await
             }

volo-http/src/client/transport/connector.rs

@@ -278,14 +278,39 @@ macro_rules! simple_error {
             }
         }
     };
+
+    ($(#[$attr:meta])* $kind:ident => $name:ident($inner:ty) => $msg:literal) => {
+        paste! {
+            #[doc = $kind " error \"" $msg "\""]
+            $(#[$attr])*
+            #[derive(Debug, PartialEq, Eq)]
+            pub struct $name($inner);
+
+            $(#[$attr])*
+            impl ::std::fmt::Display for $name {
+                fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+                    write!(f, concat!($msg, ": {}"), self.0)
+                }
+            }
+
+            $(#[$attr])*
+            impl ::std::error::Error for $name {}
+
+            $(#[$attr])*
+            pub(crate) fn [<$name:snake>](inner: $inner) -> ClientError {
+                ClientError::new(ErrorKind::$kind, Some($name(inner)))
+            }
+        }
+    };
 }
 
 simple_error!(Builder => NoAddress => "missing target address");
-simple_error!(Builder => BadScheme => "bad scheme");
+simple_error!(Builder => BadScheme(::http::uri::Scheme) => "bad scheme");
 #[cfg(not(all(feature = "http1", feature = "http2")))]
 simple_error!(Builder => BadVersion => "bad http protocol version");
-simple_error!(Builder => BadHostName => "bad host name");
-simple_error!(Builder => BadAddress => "bad address");
+simple_error!(Builder => BadHostName(::faststr::FastStr) => "bad host name");
+simple_error!(Builder => SchemeUnavailable => "scheme is unavailable in current target");
+simple_error!(Builder => PortUnavailable => "port is unavailable in current target");
 simple_error!(Connect => Retry => "retry");
 simple_error!(Request => Timeout => "request timeout");
 simple_error!(LoadBalance => NoAvailableEndpoint => "no available endpoint");
@@ -302,8 +327,14 @@ mod client_error_tests {
     #[test]
     fn types_downcast() {
         assert!(no_address().source().unwrap().is::<NoAddress>());
-        assert!(bad_scheme().source().unwrap().is::<BadScheme>());
-        assert!(bad_host_name().source().unwrap().is::<BadHostName>());
+        assert!(bad_scheme(::http::uri::Scheme::HTTP)
+            .source()
+            .unwrap()
+            .is::<BadScheme>());
+        assert!(bad_host_name(::faststr::FastStr::from_static_str("foo"))
+            .source()
+            .unwrap()
+            .is::<BadHostName>());
         assert!(timeout().source().unwrap().is::<Timeout>());
         assert!(no_available_endpoint()
             .source()