diff --git a/client/examples/chat-cli/main.rs b/client/examples/chat-cli/main.rs index a7dc7bb93c..37238de590 100644 --- a/client/examples/chat-cli/main.rs +++ b/client/examples/chat-cli/main.rs @@ -48,11 +48,12 @@ fn main() { // Create a client. let mut client = runtime - .block_on(Client::new( - ConnectionArgs::HostnameAndOptionalPort(server_addr, false), - None, - runtime2, - )) + .block_on(async { + let addr = ConnectionArgs::resolve(&server_addr, false) + .await + .expect("dns resolve failed"); + Client::new(addr, None, runtime2).await + }) .expect("Failed to create client instance"); println!("Server info: {:?}", client.server_info()); diff --git a/client/src/addr.rs b/client/src/addr.rs index a880658ea0..fa8c871aea 100644 --- a/client/src/addr.rs +++ b/client/src/addr.rs @@ -4,8 +4,6 @@ use tracing::trace; #[derive(Clone, Debug)] pub enum ConnectionArgs { - /// :[] + preferIpv6 flag - HostnameAndOptionalPort(String, bool), IpAndPort(Vec), Mpsc(u64), } @@ -13,50 +11,31 @@ pub enum ConnectionArgs { impl ConnectionArgs { const DEFAULT_PORT: u16 = 14004; - /// Parse ip address or resolves hostname, moves HostnameAndOptionalPort to - /// IpAndPort state. - /// Note: If you use an ipv6 address, the number after - /// the last colon will be used as the port unless you use [] around the - /// address. - pub async fn resolve(&mut self) -> Result<(), std::io::Error> { - if let ConnectionArgs::HostnameAndOptionalPort(server_address, prefer_ipv6) = self { - // 1. Try if server_address already contains a port - if let Ok(addr) = server_address.parse::() { - trace!("Server address with port found"); - *self = ConnectionArgs::IpAndPort(vec![addr]); - return Ok(()); - } - - // 2, Try server_address and port - if let Ok(addr) = - format!("{}:{}", server_address, Self::DEFAULT_PORT).parse::() - { - trace!("Server address without port found"); - *self = ConnectionArgs::IpAndPort(vec![addr]); - return Ok(()); - } - - // 3. Assert it's a hostname + port - let new = match lookup_host(server_address.to_string()).await { - Ok(s) => { - trace!("Host lookup succeeded"); - Ok(Self::sort_ipv6(s, *prefer_ipv6)) - }, - Err(e) => { - // 4. Assume its a hostname without port - match lookup_host((server_address.to_string(), Self::DEFAULT_PORT)).await { - Ok(s) => { - trace!("Host lookup without ports succeeded"); - Ok(Self::sort_ipv6(s, *prefer_ipv6)) - }, - Err(_) => Err(e), - } - }, - }?; - - *self = new; + /// Parse ip address or resolves hostname. + /// Note: If you use an ipv6 address, the number after the last + /// colon will be used as the port unless you use [] around the address. + pub async fn resolve( + /* :[] */ server_address: &str, + prefer_ipv6: bool, + ) -> Result { + // `lookup_host` will internally try to parse it as a SocketAddr + // 1. Assume it's a hostname + port + match lookup_host(server_address).await { + Ok(s) => { + trace!("Host lookup succeeded"); + Ok(Self::sort_ipv6(s, prefer_ipv6)) + }, + Err(e) => { + // 2. Assume its a hostname without port + match lookup_host((server_address, Self::DEFAULT_PORT)).await { + Ok(s) => { + trace!("Host lookup without ports succeeded"); + Ok(Self::sort_ipv6(s, prefer_ipv6)) + }, + Err(_) => Err(e), // Todo: evaluate returning both errors + } + }, } - Ok(()) } fn sort_ipv6(s: impl Iterator, prefer_ipv6: bool) -> Self { @@ -73,27 +52,11 @@ mod tests { use super::*; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; - #[tokio::test] - async fn keep_mpcs() { - let mut args = ConnectionArgs::Mpsc(1337); - assert!(args.resolve().await.is_ok()); - assert!(matches!(args, ConnectionArgs::Mpsc(1337))); - } - - #[tokio::test] - async fn keep_ip() { - let mut args = ConnectionArgs::IpAndPort(vec![SocketAddr::new( - IpAddr::V4(Ipv4Addr::LOCALHOST), - ConnectionArgs::DEFAULT_PORT, - )]); - assert!(args.resolve().await.is_ok()); - assert!(matches!(args, ConnectionArgs::IpAndPort(..))); - } - #[tokio::test] async fn resolve_localhost() { - let mut args = ConnectionArgs::HostnameAndOptionalPort("localhost".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("localhost", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert!(args.len() == 1 || args.len() == 2); assert_eq!(args[0].ip(), IpAddr::V4(Ipv4Addr::LOCALHOST)); @@ -102,8 +65,9 @@ mod tests { panic!("wrong resolution"); } - let mut args = ConnectionArgs::HostnameAndOptionalPort("localhost:666".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("localhost:666", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert!(args.len() == 1 || args.len() == 2); assert_eq!(args[0].port(), 666); @@ -114,8 +78,9 @@ mod tests { #[tokio::test] async fn resolve_ipv6() { - let mut args = ConnectionArgs::HostnameAndOptionalPort("localhost".to_string(), true); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("localhost", true) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert!(args.len() == 1 || args.len() == 2); assert_eq!(args[0].ip(), Ipv6Addr::LOCALHOST); @@ -127,8 +92,9 @@ mod tests { #[tokio::test] async fn resolve() { - let mut args = ConnectionArgs::HostnameAndOptionalPort("google.com".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("google.com", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert!(args.len() == 1 || args.len() == 2); assert_eq!(args[0].port(), 14004); @@ -136,8 +102,9 @@ mod tests { panic!("wrong resolution"); } - let mut args = ConnectionArgs::HostnameAndOptionalPort("127.0.0.1".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("127.0.0.1", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert_eq!(args.len(), 1); assert_eq!(args[0].port(), 14004); @@ -146,8 +113,9 @@ mod tests { panic!("wrong resolution"); } - let mut args = ConnectionArgs::HostnameAndOptionalPort("55.66.77.88".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("55.66.77.88", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert_eq!(args.len(), 1); assert_eq!(args[0].port(), 14004); @@ -156,8 +124,9 @@ mod tests { panic!("wrong resolution"); } - let mut args = ConnectionArgs::HostnameAndOptionalPort("127.0.0.1:776".to_string(), false); - assert!(args.resolve().await.is_ok()); + let args = ConnectionArgs::resolve("127.0.0.1:776", false) + .await + .expect("resolve failed"); if let ConnectionArgs::IpAndPort(args) = args { assert_eq!(args.len(), 1); assert_eq!(args[0].port(), 776); diff --git a/client/src/error.rs b/client/src/error.rs index 346dcbcbd2..2a6ac310ef 100644 --- a/client/src/error.rs +++ b/client/src/error.rs @@ -19,8 +19,6 @@ pub enum Error { Banned(String), /// Persisted character data is invalid or missing InvalidCharacter, - /// is thrown when parsing the address or resolving Dns fails - DnsResolveFailed(String), //TODO: InvalidAlias, Other(String), } diff --git a/client/src/lib.rs b/client/src/lib.rs index 160e79eae6..e2cf78b335 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -185,21 +185,13 @@ pub struct CharacterList { impl Client { /// Create a new `Client`. pub async fn new( - mut addr: ConnectionArgs, + addr: ConnectionArgs, view_distance: Option, runtime: Arc, ) -> Result { let network = Network::new(Pid::new(), Arc::clone(&runtime)); - if let Err(e) = addr.resolve().await { - error!(?e, "Dns resolve failed"); - return Err(Error::DnsResolveFailed(e.to_string())); - } - let participant = match addr { - ConnectionArgs::HostnameAndOptionalPort(..) => { - unreachable!(".resolve() should have switched that state") - }, ConnectionArgs::IpAndPort(addrs) => { // Try to connect to all IP's and return the first that works let mut participant = None; diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 98259664ed..d6574ff2ab 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -27,6 +27,11 @@ pub enum Msg { Done(Result), } +pub enum ClientConnArgs { + Host(String), + Resolved(ConnectionArgs), +} + pub struct AuthTrust(String, bool); // Used to asynchronously parse the server address, resolve host names, @@ -41,7 +46,7 @@ impl ClientInit { #[allow(clippy::op_ref)] // TODO: Pending review in #587 #[allow(clippy::or_fun_call)] // TODO: Pending review in #587 pub fn new( - connection_args: ConnectionArgs, + connection_args: ClientConnArgs, username: String, view_distance: Option, password: String, @@ -78,6 +83,18 @@ impl ClientInit { .unwrap_or(false) }; + let connection_args = match connection_args { + ClientConnArgs::Host(host) => match ConnectionArgs::resolve(&host, false).await { + Ok(r) => r, + Err(_) => { + let _ = tx.send(Msg::Done(Err(Error::NoAddress))); + tokio::task::block_in_place(move || drop(runtime2)); + return; + }, + }, + ClientConnArgs::Resolved(r) => r, + }; + let mut last_err = None; const FOUR_MINUTES_RETRIES: u64 = 48; @@ -117,10 +134,8 @@ impl ClientInit { // Parsing/host name resolution successful but no connection succeeded. let _ = tx.send(Msg::Done(Err(last_err.unwrap_or(Error::NoAddress)))); - //Safe drop runtime - tokio::task::block_in_place(move || { - drop(runtime2); - }); + // Safe drop runtime + tokio::task::block_in_place(move || drop(runtime2)); }); ClientInit { diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index eb4e076bd9..6cc4df7ff0 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -15,7 +15,7 @@ use client::{ addr::ConnectionArgs, error::{InitProtocolError, NetworkConnectError, NetworkError}, }; -use client_init::{ClientInit, Error as InitError, Msg as InitMsg}; +use client_init::{ClientConnArgs, ClientInit, Error as InitError, Msg as InitMsg}; use common::{assets::AssetExt, comp, span}; use std::sync::Arc; use tokio::runtime; @@ -76,7 +76,9 @@ impl PlayState for MainMenuState { &mut global_state.info_message, "singleplayer".to_owned(), "".to_owned(), - ConnectionArgs::IpAndPort(vec![server_settings.gameserver_address]), + ClientConnArgs::Resolved(ConnectionArgs::IpAndPort(vec![ + server_settings.gameserver_address, + ])), &mut self.client_init, Some(runtime), ); @@ -127,11 +129,6 @@ impl PlayState for MainMenuState { localized_strings.get("main.login.server_not_found").into() }, InitError::ClientError(err) => match err { - client::Error::DnsResolveFailed(reason) => format!( - "{}: {}", - localized_strings.get("main.login.server_not_found"), - reason - ), client::Error::AuthErr(e) => format!( "{}: {}", localized_strings.get("main.login.authentication_error"), @@ -258,7 +255,7 @@ impl PlayState for MainMenuState { &mut global_state.info_message, username, password, - ConnectionArgs::HostnameAndOptionalPort(server_address, false), + ClientConnArgs::Host(server_address), &mut self.client_init, None, ); @@ -331,7 +328,7 @@ fn attempt_login( info_message: &mut Option, username: String, password: String, - connection_args: ConnectionArgs, + connection_args: ClientConnArgs, client_init: &mut Option, runtime: Option>, ) {