浏览代码

Fix exception documentation for BaseClient.Connect() and Session.Connect().
Added SftpPathNotFoundException to doc for SftpClient.Get(String) and SftpClient.GetAttributes(String).
Do not throw exception in Session.Disconnect() and Session.Dispose() when socket is not connected.
Fixes issue #2148.

Gert Driesen 11 年之前
父节点
当前提交
acaae44836

+ 2 - 0
Renci.SshClient/Build/nuget/SSH.NET.nuspec

@@ -22,6 +22,7 @@
 
           Fixes:
 
+          * SftpClient is throwing undocumented exceptions (issue #2148)
           * Client channels are no longer closed on dispose (issue #1943)
           * SftpClient.Exists(string) returns true for a path that does not exist (issues #1952, #1696 and #1574)
           * ObjectDisposedException when channel is closing (issues #1942 and #1944)
@@ -29,6 +30,7 @@
           * SshCommand doesn't cleanup subscribed events (issue #2295)
           * Lines before protocol identification string are not skipped (issue #1935 and #2223)
           * ShellStream.ReadLine produces incorrect output when reading multi-byte characters (issue #2190)
+          * Dispose and Disconnect throw SocketException when socket is not connected (issue #2148)
 
           2014.4.6-beta1
           ==============

+ 74 - 91
Renci.SshClient/Renci.SshNet.Tests/Classes/SessionTest.cs

@@ -5,8 +5,8 @@ using System.Net.Sockets;
 using System.Text;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Renci.SshNet.Common;
-using Renci.SshNet.Messages;
 using Renci.SshNet.Tests.Common;
+using Renci.SshNet.Tests.Properties;
 
 namespace Renci.SshNet.Tests.Classes
 {
@@ -210,114 +210,97 @@ namespace Renci.SshNet.Tests.Classes
             }
         }
 
-
-        /// <summary>
-        ///A test for SessionSemaphore
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void SessionSemaphoreTest()
+        [TestMethod]
+        public void Connect_HostNameInvalid_ShouldThrowSocketExceptionWithErrorCodeHostNotFound()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            SemaphoreLight actual;
-            actual = target.SessionSemaphore;
-            Assert.Inconclusive("Verify the correctness of this test method.");
-        }
+            var connectionInfo = new ConnectionInfo("invalid.", 40, "user",
+                new KeyboardInteractiveAuthenticationMethod("user"));
+            var session = new Session(connectionInfo);
 
-        /// <summary>
-        ///A test for IsConnected
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void IsConnectedTest()
-        {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            bool actual;
-            actual = target.IsConnected;
-            Assert.Inconclusive("Verify the correctness of this test method.");
-        }
+            try
+            {
 
-        /// <summary>
-        ///A test for ClientInitMessage
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void ClientInitMessageTest()
-        {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            Message actual;
-            actual = target.ClientInitMessage;
-            Assert.Inconclusive("Verify the correctness of this test method.");
+                session.Connect();
+                Assert.Fail();
+            }
+            catch (SocketException ex)
+            {
+                Assert.AreEqual(ex.ErrorCode, (int)SocketError.HostNotFound);
+            }
         }
 
-        /// <summary>
-        ///A test for UnRegisterMessage
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void UnRegisterMessageTest()
+        [TestMethod]
+        public void Connect_ProxyHostNameInvalid_ShouldThrowSocketExceptionWithErrorCodeHostNotFound()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            string messageName = string.Empty; // TODO: Initialize to an appropriate value
-            target.UnRegisterMessage(messageName);
-            Assert.Inconclusive("A method that does not return a value cannot be verified.");
+            var connectionInfo = new ConnectionInfo("localhost", 40, "user", ProxyTypes.Http, "invalid.", 80,
+                "proxyUser", "proxyPwd", new KeyboardInteractiveAuthenticationMethod("user"));
+            var session = new Session(connectionInfo);
+
+            try
+            {
+                session.Connect();
+                Assert.Fail();
+            }
+            catch (SocketException ex)
+            {
+                Assert.AreEqual(ex.ErrorCode, (int)SocketError.HostNotFound);
+            }
         }
 
-        /// <summary>
-        ///A test for RegisterMessage
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void RegisterMessageTest()
+        [TestMethod]
+        public void DisconnectShouldNotThrowExceptionWhenSocketIsNotConnected()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            string messageName = string.Empty; // TODO: Initialize to an appropriate value
-            target.RegisterMessage(messageName);
-            Assert.Inconclusive("A method that does not return a value cannot be verified.");
+            var connectionInfo = new ConnectionInfo("localhost", 6767, Resources.USERNAME,
+                new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
+            var session = new Session(connectionInfo);
+
+            try
+            {
+                session.Connect();
+                Assert.Fail();
+            }
+            catch (SocketException)
+            {
+                session.Disconnect();
+            }
         }
 
-        /// <summary>
-        ///A test for Dispose
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void DisposeTest()
+        [TestMethod]
+        public void DisconnectShouldNotThrowExceptionWhenConnectHasNotBeenInvoked()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            target.Dispose();
-            Assert.Inconclusive("A method that does not return a value cannot be verified.");
+            var connectionInfo = new ConnectionInfo("localhost", 6767, Resources.USERNAME,
+                new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
+            var session = new Session(connectionInfo);
+
+            session.Disconnect();
         }
 
-        /// <summary>
-        ///A test for Disconnect
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void DisconnectTest()
+        [TestMethod]
+        public void DisposeShouldNotThrowExceptionWhenSocketIsNotConnected()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            target.Disconnect();
-            Assert.Inconclusive("A method that does not return a value cannot be verified.");
+            var connectionInfo = new ConnectionInfo("localhost", 6767, Resources.USERNAME,
+                new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
+            var session = new Session(connectionInfo);
+
+            try
+            {
+                session.Connect();
+                Assert.Fail();
+            }
+            catch (SocketException)
+            {
+                session.Dispose();
+            }
         }
 
-        /// <summary>
-        ///A test for Connect
-        ///</summary>
-        [TestMethod()]
-        [Ignore]
-        public void ConnectTest()
+        [TestMethod]
+        public void DisposeShouldNotThrowExceptionWhenConenectHasNotBeenInvoked()
         {
-            ConnectionInfo connectionInfo = null; // TODO: Initialize to an appropriate value
-            Session target = new Session(connectionInfo); // TODO: Initialize to an appropriate value
-            target.Connect();
-            Assert.Inconclusive("A method that does not return a value cannot be verified.");
+            var connectionInfo = new ConnectionInfo("localhost", 6767, Resources.USERNAME,
+                new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
+            var session = new Session(connectionInfo);
+
+            session.Disconnect();
         }
 
         private static ConnectionInfo CreateConnectionInfo(IPEndPoint serverEndPoint, TimeSpan timeout)

+ 21 - 0
Renci.SshClient/Renci.SshNet/BaseClient.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Net.Sockets;
 using System.Threading;
 using Renci.SshNet.Common;
 
@@ -142,10 +143,30 @@ namespace Renci.SshNet
         /// </summary>
         /// <exception cref="InvalidOperationException">The client is already connected.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
+        /// <exception cref="SocketException">Socket connection to the SSH server or proxy server could not be established, or an error occurred while resolving the hostname.</exception>
+        /// <exception cref="SshConnectionException">SSH session could not be established.</exception>
+        /// <exception cref="SshAuthenticationException">Authentication of SSH session failed.</exception>
+        /// <exception cref="ProxyException">Failed to establish proxy connection.</exception>
         public void Connect()
         {
             CheckDisposed();
 
+            // TODO (see issue #1758):
+            // we're not stopping the keep-alive timer and disposing the session here
+            // 
+            // we could do this but there would still be side effects as concrete
+            // implementations may still hang on to the original session
+            // 
+            // therefore it would be better to actually invoke the Disconnect method
+            // (and then the Dispose on the session) but even that would have side effects
+            // eg. it would remove all forwarded ports from SshClient
+            // 
+            // I think we should modify our concrete clients to better deal with a
+            // disconnect. In case of SshClient this would mean not removing the 
+            // forwarded ports on disconnect (but only on dispose ?) and link a
+            // forwarded port with a client instead of with a session
+            //
+            // To be discussed with Oleg (or whoever is interested)
             if (Session != null && Session.IsConnected)
                 throw new InvalidOperationException("The client is already connected.");
 

+ 6 - 1
Renci.SshClient/Renci.SshNet/Session.cs

@@ -460,6 +460,10 @@ namespace Renci.SshNet
         /// <summary>
         /// Connects to the server.
         /// </summary>
+        /// <exception cref="SocketException">Socket connection to the SSH server or proxy server could not be established, or an error occurred while resolving the hostname.</exception>
+        /// <exception cref="SshConnectionException">SSH session could not be established.</exception>
+        /// <exception cref="SshAuthenticationException">Authentication of SSH session failed.</exception>
+        /// <exception cref="ProxyException">Failed to establish proxy connection.</exception>
         public void Connect()
         {
             if (this.IsConnected)
@@ -1699,7 +1703,8 @@ namespace Renci.SshNet
                 {
                     if (_socket != null)
                     {
-                        SocketDisconnect();
+                        if (_socket.Connected)
+                            SocketDisconnect();
                         _socket.Dispose();
                         _socket = null;
                     }

+ 7 - 5
Renci.SshClient/Renci.SshNet/SftpClient.cs

@@ -224,7 +224,7 @@ namespace Renci.SshNet
         /// <exception cref="ArgumentNullException"><paramref name="path"/> is <b>null</b>.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
         /// <exception cref="SftpPermissionDeniedException">Permission to change directory denied by remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
-        /// <exception cref="SftpPathNotFoundException">The path in <paramref name="path"/> was not found on the remote host.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="SshException">A SSH error where <see cref="P:System.Exception.Message"/> is the message from the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
         public void ChangeDirectory(string path)
@@ -248,7 +248,7 @@ namespace Renci.SshNet
         /// <exception cref="ArgumentNullException"><paramref name="path"/> is <b>null</b>.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
         /// <exception cref="SftpPermissionDeniedException">Permission to change permission on the path(s) was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
-        /// <exception cref="SftpPathNotFoundException">The path in <paramref name="path"/> was not found on the remote host.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="SshException">A SSH error where <see cref="P:System.Exception.Message"/> is the message from the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
         public void ChangePermissions(string path, short mode)
@@ -287,7 +287,7 @@ namespace Renci.SshNet
         /// <param name="path">Directory to be deleted path.</param>
         /// <exception cref="ArgumentException"><paramref name="path"/> is <b>null</b> or contains whitespace characters.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
-        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> does not exist.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="SftpPermissionDeniedException">Permission to delete the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
         /// <exception cref="SshException">A SSH error where <see cref="P:System.Exception.Message"/> is the message from the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
@@ -312,7 +312,7 @@ namespace Renci.SshNet
         /// <param name="path">File to be deleted path.</param>
         /// <exception cref="ArgumentException"><paramref name="path"/> is <b>null</b> or contains whitespace characters.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
-        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> does not exist.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="SftpPermissionDeniedException">Permission to delete the file was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
         /// <exception cref="SshException">A SSH error where <see cref="P:System.Exception.Message"/> is the message from the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
@@ -501,6 +501,7 @@ namespace Renci.SshNet
         /// <param name="path">The path.</param>
         /// <returns>Reference to <see cref="Renci.SshNet.Sftp.SftpFile"/> file object.</returns>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
         public SftpFile Get(string path)
@@ -1112,7 +1113,7 @@ namespace Renci.SshNet
         /// <param name="path">The name of the file or directory to be deleted. Wildcard characters are not supported.</param>
         /// <exception cref="ArgumentNullException"><paramref name="path"/> is <b>null</b>.</exception>
         /// <exception cref="SshConnectionException">Client is not connected.</exception>
-        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> does not exist.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
         public void Delete(string path)
         {
@@ -1505,6 +1506,7 @@ namespace Renci.SshNet
         /// <param name="path">The path to the file.</param>
         /// <returns>The <see cref="SftpFileAttributes"/> of the file on the path.</returns>
         /// <exception cref="ArgumentNullException"><paramref name="path"/> is <b>null</b>.</exception>
+        /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
         public SftpFileAttributes GetAttributes(string path)
         {