SshClientSettings/SshConfigSettings: make List/Dictionary properties …

…mutable.
tmds · Sep 26, 2024 · 364d190 · 364d190
1 parent 50efe40
commit 364d190
Show file tree

Hide file tree

Showing 12 changed files with 185 additions and 87 deletions.
diff --git a/README.md b/README.md
@@ -206,42 +206,42 @@ class SftpFile : Stream
 }
 class SshClientSettings
 {
-  static IReadOnlyList<Credential> DefaultCredentials { get; } // = [ PrivateKeyCredential("~/.ssh/id_rsa"), KerberosCredential() ]
-  static IReadOnlyList<string> DefaultUserKnownHostsFilePaths { get; } // = [ '~/.ssh/known_hosts' ]
-  static IReadOnlyList<string> DefaultGlobalKnownHostsFilePaths { get; } // = [ '/etc/ssh/known_hosts' ]
+  static IReadOnlyList<Credential> DefaultCredentials { get; } = [ PrivateKeyCredential("~/.ssh/id_rsa"), KerberosCredential() ]
+  static IReadOnlyList<string> DefaultUserKnownHostsFilePaths { get; } = [ '~/.ssh/known_hosts' ]
+  static IReadOnlyList<string> DefaultGlobalKnownHostsFilePaths { get; } = [ '/etc/ssh/known_hosts' ]
 
   SshClientSettings();
   SshClientSettings(string destination);
 
   TimeSpan ConnectTimeout { get; set; } // = 15s
 
-  string UserName { get; set; }
-  string HostName { get; set; }
-  int Port { get; set; }
+  string UserName { get; set; } = Environment.UserName;
+  string HostName { get; set; } = "";
+  int Port { get; set; } = 22;
 
   IReadOnlyList<Credential> Credentials { get; set; } = DefaultCredentials;
 
   bool AutoConnect { get; set; } = true;
-  bool AutoReconnect { get; set; }
+  bool AutoReconnect { get; set; } = false;
 
-  IReadOnlyList<string> GlobalKnownHostsFilePaths { get; set; } = DefaultGlobalKnownHostsFilePaths;
-  IReadOnlyList<string> UserKnownHostsFilePaths { get; set; } = DefaultUserKnownHostsFilePaths;
+  List<string> GlobalKnownHostsFilePaths { get; set; } = DefaultGlobalKnownHostsFilePaths;
+  List<string> UserKnownHostsFilePaths { get; set; } = DefaultUserKnownHostsFilePaths;
   HostAuthentication? HostAuthentication { get; set; } // not called when known to be trusted/revoked.
   bool UpdateKnownHostsFileAfterAuthentication { get; set; } = false;
   bool HashKnownHosts { get; set; } = false;
 
   int MinimumRSAKeySize { get; set; } = 2048;
 
-  IReadOnlyDictionary<string, string>? EnvironmentVariables { get; set; }
+  Dictionary<string, string> EnvironmentVariables { get; set; } = [];
 }
 class SshConfigSettings
 {
   static SshConfigSettings DefaultConfig { get; }  // use DefaultConfigFilePaths.
   static SshConfigSettings NoConfig { get; } // use [ ]
   static IReadOnlyList<string> DefaultConfigFilePaths { get; } // [ '~/.ssh/config', '/etc/ssh/ssh_config' ]
 
-  IReadOnlyList<string> ConfigFilePaths { get; set; }
-  IReadOnlyDictionary<SshConfigOption, SshConfigOptionValue> Options { get; set; }
+  List<string> ConfigFilePaths { get; set; } = DefaultConfigFilePaths;
+  Dictionary<SshConfigOption, SshConfigOptionValue> Options { get; set; }
 
   TimeSpan ConnectTimeout { get; set; } // = 15s, overridden by config timeout (if set)
 

diff --git a/examples/ssh/Program.cs b/examples/ssh/Program.cs
@@ -96,7 +96,7 @@ static void PrintExceptions(Task[] tasks)
 
     private static SshConfigSettings CreateSshConfigSettings(string[] options)
     {
-        SshConfigSettings configSettings = new SshConfigSettings(SshConfigSettings.DefaultConfigFilePaths);
+        SshConfigSettings configSettings = new SshConfigSettings();
 
         Dictionary<SshConfigOption, SshConfigOptionValue> optionsDict = new();
         foreach (var option in options)

diff --git a/src/Tmds.Ssh/SshClient.cs b/src/Tmds.Ssh/SshClient.cs
@@ -65,6 +65,9 @@ private SshClient(
         TimeSpan connectTimeout,
         ILoggerFactory? loggerFactory)
     {
+        settings?.Validate();
+        configSettings?.Validate();
+
         _settings = settings;
         _destination = destination;
         _sshConfigOptions = configSettings;

diff --git a/src/Tmds.Ssh/SshClientSettings.SshConfig.cs b/src/Tmds.Ssh/SshClientSettings.SshConfig.cs
@@ -18,7 +18,7 @@ partial class SshClientSettings
 
     internal static async ValueTask<SshClientSettings> LoadFromConfigAsync(string? userName, string host, int? port, SshConfigSettings options, CancellationToken cancellationToken = default)
     {
-        SshConfig sshConfig = await SshConfig.DetermineConfigForHost(userName, host, port, options.Options, options.ConfigFilePaths, cancellationToken);
+        SshConfig sshConfig = await SshConfig.DetermineConfigForHost(userName, host, port, options.OptionsOrDefault, options.ConfigFilePaths, cancellationToken);
 
         List<Name> ciphers = DetermineAlgorithms(sshConfig.Ciphers, DefaultEncryptionAlgorithms, SupportedEncryptionAlgorithms);
         List<Name> hostKeyAlgorithms = DetermineAlgorithms(sshConfig.HostKeyAlgorithms, DefaultServerHostKeyAlgorithms, SupportedServerHostKeyAlgorithms);
@@ -32,8 +32,6 @@ internal static async ValueTask<SshClientSettings> LoadFromConfigAsync(string? u
             HostName = sshConfig.HostName ?? host,
             UserName = sshConfig.UserName ?? Environment.UserName,
             Port = sshConfig.Port ?? DefaultPort,
-            UserKnownHostsFilePaths = sshConfig.UserKnownHostsFiles ?? DefaultUserKnownHostsFilePaths,
-            GlobalKnownHostsFilePaths = sshConfig.GlobalKnownHostsFiles ?? DefaultGlobalKnownHostsFilePaths,
             ConnectTimeout = sshConfig.ConnectTimeout > 0 ? TimeSpan.FromSeconds(sshConfig.ConnectTimeout.Value) : options.ConnectTimeout,
             KeyExchangeAlgorithms = kexAlgorithms,
             ServerHostKeyAlgorithms = hostKeyAlgorithms,
@@ -47,8 +45,20 @@ internal static async ValueTask<SshClientSettings> LoadFromConfigAsync(string? u
             MinimumRSAKeySize = sshConfig.RequiredRSASize ?? DefaultMinimumRSAKeySize,
             Credentials = DetermineCredentials(sshConfig),
             HashKnownHosts = sshConfig.HashKnownHosts ?? DefaultHashKnownHosts,
-            EnvironmentVariables = CreateEnvironmentVariables(Environment.GetEnvironmentVariables(), sshConfig.SendEnv)
         };
+        if (sshConfig.UserKnownHostsFiles is not null)
+        {
+            settings.UserKnownHostsFilePaths = sshConfig.UserKnownHostsFiles;
+        }
+        if (sshConfig.GlobalKnownHostsFiles is not null)
+        {
+            settings.GlobalKnownHostsFilePaths = sshConfig.GlobalKnownHostsFiles;
+        }
+        var envvars = CreateEnvironmentVariables(Environment.GetEnvironmentVariables(), sshConfig.SendEnv);
+        if (envvars is not null)
+        {
+            settings.EnvironmentVariables = envvars;
+        }
 
         SshConfig.StrictHostKeyChecking hostKeyChecking = sshConfig.HostKeyChecking ?? SshConfig.StrictHostKeyChecking.Ask;
         switch (hostKeyChecking)
@@ -100,7 +110,7 @@ internal static async ValueTask<SshClientSettings> LoadFromConfigAsync(string? u
         return settings;
     }
 
-    internal static IReadOnlyDictionary<string, string>? CreateEnvironmentVariables(IDictionary systemEnvironment, List<System.String>? sendEnv)
+    internal static Dictionary<string, string>? CreateEnvironmentVariables(IDictionary systemEnvironment, List<System.String>? sendEnv)
     {
         if (sendEnv is null || sendEnv.Count == 0)
         {
@@ -123,7 +133,7 @@ internal static async ValueTask<SshClientSettings> LoadFromConfigAsync(string? u
         return envvars;
     }
 
-    private static IReadOnlyList<Credential> DetermineCredentials(SshConfig config)
+    private static List<Credential> DetermineCredentials(SshConfig config)
     {
         bool addPubKeyCredentials = config.PubKeyAuthentication ?? true;
         bool addGssApiCredentials = config.GssApiAuthentication ?? false;

diff --git a/src/Tmds.Ssh/SshClientSettings.cs b/src/Tmds.Ssh/SshClientSettings.cs
@@ -9,10 +9,21 @@ public sealed partial class SshClientSettings
     private int _port = DefaultPort;
     private string _hostName = "";
     private string _userName = "";
-    private IReadOnlyList<Credential> _credentials = DefaultCredentials;
+    private List<Credential>? _credentials;
     private TimeSpan _connectTimeout = DefaultConnectTimeout;
-    private IReadOnlyList<string> _userKnownHostsFilePaths = DefaultUserKnownHostsFilePaths;
-    private IReadOnlyList<string> _globalKnownHostsFilePaths = DefaultGlobalKnownHostsFilePaths;
+    private List<string>? _userKnownHostsFilePaths;
+    private List<string>? _globalKnownHostsFilePaths;
+    private Dictionary<string, string>? _environmentVariables;
+
+    // Avoid allocations from the public getters.
+    internal IReadOnlyList<Credential> CredentialsOrDefault
+        => _credentials ?? DefaultCredentials;
+    internal IReadOnlyList<string> UserKnownHostsFilePathsOrDefault
+        => _userKnownHostsFilePaths ?? DefaultUserKnownHostsFilePaths;
+    internal IReadOnlyList<string> GlobalKnownHostsFilePathsOrDefault
+        => _globalKnownHostsFilePaths ?? DefaultGlobalKnownHostsFilePaths;
+    internal Dictionary<string, string>? EnvironmentVariablesOrDefault
+        => _environmentVariables;
 
     public SshClientSettings()
     { }
@@ -71,9 +82,9 @@ public string HostName
         }
     }
 
-    public IReadOnlyList<Credential> Credentials
+    public List<Credential> Credentials
     {
-        get => _credentials;
+        get => _credentials ??= new List<Credential>(DefaultCredentials);
         set
         {
             ArgumentNullException.ThrowIfNull(value);
@@ -95,6 +106,54 @@ public TimeSpan ConnectTimeout
         }
     }
 
+    internal void Validate()
+    {
+        if (_credentials is not null)
+        {
+            foreach (var item in _credentials)
+            {
+                if (item is null)
+                {
+                    throw new ArgumentException($"{nameof(Credentials)} contains 'null'." , $"{nameof(Credentials)}");
+                }
+            }
+        }
+        if (_userKnownHostsFilePaths is not null)
+        {
+            foreach (var item in _userKnownHostsFilePaths)
+            {
+                if (item is null)
+                {
+                    throw new ArgumentException($"{nameof(UserKnownHostsFilePaths)} contains 'null'." , $"{nameof(UserKnownHostsFilePaths)}");
+                }
+            }
+        }
+        if (_globalKnownHostsFilePaths is not null)
+        {
+            foreach (var item in _globalKnownHostsFilePaths)
+            {
+                if (item is null)
+                {
+                    throw new ArgumentException($"{nameof(GlobalKnownHostsFilePaths)} contains 'null'." , $"{nameof(GlobalKnownHostsFilePaths)}");
+                }
+            }
+        }
+        if (_environmentVariables is not null)
+        {
+            foreach (var item in _environmentVariables)
+            {
+                if (item.Key.Length == 0)
+                {
+                    throw new ArgumentException($"{nameof(EnvironmentVariables)} contains empty key." , $"{nameof(EnvironmentVariables)}");
+                }
+                if (item.Value is null)
+                {
+                    throw new ArgumentException($"{nameof(EnvironmentVariables)} contains 'null' value for key '{item.Key}'." , $"{nameof(EnvironmentVariables)}");
+                }
+            }
+        }
+    }
+
     public int Port
     {
         get => _port;
@@ -108,30 +167,22 @@ public int Port
         }
     }
 
-    public IReadOnlyList<string> UserKnownHostsFilePaths
+    public List<string> UserKnownHostsFilePaths
     {
-        get => _userKnownHostsFilePaths;
+        get => _userKnownHostsFilePaths ??= new List<string>(DefaultUserKnownHostsFilePaths);
         set
         {
             ArgumentNullException.ThrowIfNull(value);
-            foreach (var item in value)
-            {
-                ArgumentNullException.ThrowIfNullOrEmpty(item);
-            }
             _userKnownHostsFilePaths = value;
         }
     }
 
-    public IReadOnlyList<string> GlobalKnownHostsFilePaths
+    public List<string> GlobalKnownHostsFilePaths
     {
-        get => _globalKnownHostsFilePaths;
+        get => _globalKnownHostsFilePaths ??= new List<string>(DefaultGlobalKnownHostsFilePaths);
         set
         {
             ArgumentNullException.ThrowIfNull(value);
-            foreach (var item in value)
-            {
-                ArgumentNullException.ThrowIfNullOrEmpty(item);
-            }
             _globalKnownHostsFilePaths = value;
         }
     }
@@ -146,7 +197,15 @@ public IReadOnlyList<string> GlobalKnownHostsFilePaths
 
     public bool HashKnownHosts { get; set; } = DefaultHashKnownHosts;
 
-    public IReadOnlyDictionary<string, string>? EnvironmentVariables { get; set; }
+    public Dictionary<string, string>? EnvironmentVariables
+    {
+        get => _environmentVariables ??= new();
+        set
+        {
+            ArgumentNullException.ThrowIfNull(value);
+            _environmentVariables = value;
+        }
+    }
 
     public int MinimumRSAKeySize { get; set; } = DefaultMinimumRSAKeySize; // TODO throw if <0.
 

diff --git a/src/Tmds.Ssh/SshConfig.cs b/src/Tmds.Ssh/SshConfig.cs
@@ -60,15 +60,18 @@ public struct AlgorithmList
     public bool? HashKnownHosts { get; set; }
     public List<string>? SendEnv { get; set; }
 
-    internal static ValueTask<SshConfig> DetermineConfigForHost(string? userName, string host, int? port, IReadOnlyDictionary<SshConfigOption, SshConfigOptionValue> options, IReadOnlyList<string> configFiles, CancellationToken cancellationToken)
+    internal static ValueTask<SshConfig> DetermineConfigForHost(string? userName, string host, int? port, IReadOnlyDictionary<SshConfigOption, SshConfigOptionValue>? options, IReadOnlyList<string> configFiles, CancellationToken cancellationToken)
     {
         SshConfig config = new SshConfig()
         {
             UserName = userName,
             Port = port
         };
 
-        ConfigureFromOptions(config, options);
+        if (options is not null)
+        {
+            ConfigureFromOptions(config, options);
+        }
 
         string originalhost = host;
 

diff --git a/src/Tmds.Ssh/SshConfigOptionValue.cs b/src/Tmds.Ssh/SshConfigOptionValue.cs
@@ -27,7 +27,7 @@ public SshConfigOptionValue(IEnumerable<string> values)
     public static implicit operator SshConfigOptionValue(string value)
         => new SshConfigOptionValue(value);
 
-    public bool IsEmpty => FirstValue is not null;
+    public bool IsEmpty => FirstValue is null;
 
     public bool IsSingleValue =>
         _value is string || (_value is string[] values && values.Length == 1);