fix: Always use the same address family with ssh and quic

- Correctly propagate the AddressFamily resulting from the DNS lookup down to the ssh subprocess.
- remove duplicated remote_host argument from Channel::transact, extract it from Parameters iunstead
- tidy up Channel::transact arguments, remove unused try_from<&CliArgs> for CopyJobSpec

src/cli/args.rs

@@ -3,7 +3,7 @@
 
 use clap::{ArgAction::SetTrue, Args as _, FromArgMatches as _, Parser};
 
-use crate::{client::CopyJobSpec, config::Manager, util::AddressFamily};
+use crate::{config::Manager, util::AddressFamily};
 
 /// Options that switch us into another mode i.e. which don't require source/destination arguments
 pub(crate) const MODE_OPTIONS: &[&str] = &["server", "help_buffers", "show_config", "config_files"];
@@ -110,11 +110,3 @@ impl From<&CliArgs> for Manager {
         mgr
     }
 }
-
-impl TryFrom<&CliArgs> for CopyJobSpec {
-    type Error = anyhow::Error;
-
-    fn try_from(args: &CliArgs) -> Result<Self, Self::Error> {
-        CopyJobSpec::try_from(&args.client_params)
-    }
-}

src/client/control.rs

@@ -14,7 +14,7 @@ use tracing::{debug, trace, warn};
 use crate::{
     config::Configuration,
     protocol::control::{ClientMessage, ClosedownReport, ConnectionType, ServerMessage, BANNER},
-    util::{AddressFamily, Credentials},
+    util::Credentials,
 };
 
 use super::Parameters;
@@ -35,17 +35,15 @@ impl Channel {
     }
 
     /// Opens the control channel, checks the banner, sends the Client Message, reads the Server Message.
-    #[allow(clippy::too_many_arguments)]
     pub async fn transact(
         credentials: &Credentials,
-        remote_host: &str,
         connection_type: ConnectionType,
         display: &MultiProgress,
         config: &Configuration,
         parameters: &Parameters,
     ) -> Result<(Channel, ServerMessage)> {
         trace!("opening control channel");
-        let mut new1 = Self::launch(display, config, remote_host, parameters)?;
+        let mut new1 = Self::launch(display, config, parameters, connection_type)?;
         new1.wait_for_banner().await?;
 
         let mut pipe = new1
@@ -80,19 +78,19 @@ impl Channel {
     fn launch(
         display: &MultiProgress,
         config: &Configuration,
-        remote_host: &str,
         parameters: &Parameters,
+        connection_type: ConnectionType,
     ) -> Result<Self> {
+        let remote_host = parameters.remote_host()?;
         let mut server = tokio::process::Command::new(&config.ssh);
         let _ = server.kill_on_drop(true);
-        let _ = match config.address_family {
-            None => &mut server,
-            Some(AddressFamily::V4) => server.arg("-4"),
-            Some(AddressFamily::V6) => server.arg("-6"),
+        let _ = match connection_type {
+            ConnectionType::Ipv4 => server.arg("-4"),
+            ConnectionType::Ipv6 => server.arg("-6"),
         };
         let _ = server.args(&config.ssh_opt);
         let _ = server.args([
-            remote_host,
+            &remote_host,
             "qcp",
             "--server",
             // Remote receive bandwidth = our transmit bandwidth

src/client/job.rs

@@ -68,21 +68,6 @@ impl CopyJobSpec {
         }
     }
 
-    /*
-    pub(crate) fn remote_user_host(&self) -> anyhow::Result<&str> {
-        let src = self.source.as_ref().ok_or(anyhow::anyhow!(
-            "both source and destination must be specified"
-        ))?;
-        let dest = self.destination.as_ref().ok_or(anyhow::anyhow!(
-            "both source and destination must be specified"
-        ))?;
-        Ok(src
-            .host
-            .as_ref()
-            .unwrap_or_else(|| dest.host.as_ref().unwrap()))
-    }
-    */
-
     pub(crate) fn remote_user_host(&self) -> &str {
         self.source
             .host

src/client/main_loop.rs

@@ -48,14 +48,14 @@ pub async fn client_main(
     // Prep --------------------------
     let job_spec = crate::client::CopyJobSpec::try_from(&parameters)?;
     let credentials = Credentials::generate()?;
-    let remote_host = job_spec.remote_host();
-    let remote_address = lookup_host_by_family(remote_host, config.address_family)?;
+    // If the user didn't specify the address family: we do the DNS lookup, figure it out and tell ssh to use that.
+    // (Otherwise if we resolved a v4 and ssh a v6 - as might happen with round-robin DNS - that could be surprising.)
+    let remote_address = lookup_host_by_family(job_spec.remote_host(), config.address_family)?;
 
     // Control channel ---------------
     timers.next("control channel");
     let (mut control, server_message) = Channel::transact(
         &credentials,
-        remote_host,
         remote_address.into(),
         &display,
         config,

src/client/options.rs

@@ -1,7 +1,7 @@
 //! Options specific to qcp client-mode
 // (c) 2024 Ross Younger
 
-use super::FileSpec;
+use super::{CopyJobSpec, FileSpec};
 use clap::Parser;
 
 #[derive(Debug, Parser, Clone, Default)]
@@ -66,7 +66,7 @@ pub struct Parameters {
     pub destination: Option<FileSpec>,
 }
 
-impl TryFrom<&Parameters> for crate::client::CopyJobSpec {
+impl TryFrom<&Parameters> for CopyJobSpec {
     type Error = anyhow::Error;
 
     fn try_from(args: &Parameters) -> Result<Self, Self::Error> {
@@ -91,3 +91,9 @@ impl TryFrom<&Parameters> for crate::client::CopyJobSpec {
         })
     }
 }
+
+impl Parameters {
+    pub(crate) fn remote_host(&self) -> anyhow::Result<String> {
+        Ok(CopyJobSpec::try_from(self)?.remote_host().to_string())
+    }
+}