From 66471e674195be1aef3255ece7552cbda5d3b7b9 Mon Sep 17 00:00:00 2001
From: Gert Driesen <gert.driesen@telenet.be>
Date: Wed, 15 Nov 2017 21:50:49 +0100
Subject: [PATCH] When the request to open a subsystem fails, close the channel
 session and throw a SshException. Fixes #308.

---
 .../Renci.SshNet.Tests.NET35.csproj           |   5 +-
 ...ssion_Connect_SendSubsystemRequestFails.cs | 122 ++++++++++++++++++
 .../Renci.SshNet.Tests.csproj                 |   1 +
 src/Renci.SshNet/SubsystemSession.cs          |  12 +-
 4 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 src/Renci.SshNet.Tests/Classes/SubsystemSession_Connect_SendSubsystemRequestFails.cs

diff --git a/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj b/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj
index d9f97d99..b6ab7123 100644
--- a/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj
+++ b/src/Renci.SshNet.Tests.NET35/Renci.SshNet.Tests.NET35.csproj
@@ -1551,6 +1551,9 @@
     <Compile Include="..\Renci.SshNet.Tests\Classes\SubsystemSession_Connect_NeverConnected.cs">
       <Link>Classes\SubsystemSession_Connect_NeverConnected.cs</Link>
     </Compile>
+    <Compile Include="..\Renci.SshNet.Tests\Classes\SubsystemSession_Connect_SendSubsystemRequestFails.cs">
+      <Link>Classes\SubsystemSession_Connect_SendSubsystemRequestFails.cs</Link>
+    </Compile>
     <Compile Include="..\Renci.SshNet.Tests\Classes\SubsystemSession_Disconnect_Connected.cs">
       <Link>Classes\SubsystemSession_Disconnect_Connected.cs</Link>
     </Compile>
@@ -1707,7 +1710,7 @@
   <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
   <ProjectExtensions>
     <VisualStudio>
-      <UserProperties ProjectLinkerExcludeFilter="\\?desktop(\\.*)?$;\\?silverlight(\\.*)?$;\.desktop;\.silverlight;\.xaml;^service references(\\.*)?$;\.clientconfig;^web references(\\.*)?$" ProjectLinkReference="c45379b9-17b1-4e89-bc2e-6d41726413e8" />
+      <UserProperties ProjectLinkReference="c45379b9-17b1-4e89-bc2e-6d41726413e8" ProjectLinkerExcludeFilter="\\?desktop(\\.*)?$;\\?silverlight(\\.*)?$;\.desktop;\.silverlight;\.xaml;^service references(\\.*)?$;\.clientconfig;^web references(\\.*)?$" />
     </VisualStudio>
   </ProjectExtensions>
   <!-- To modify your build process, add your task inside one of the targets below and uncomment it. 
diff --git a/src/Renci.SshNet.Tests/Classes/SubsystemSession_Connect_SendSubsystemRequestFails.cs b/src/Renci.SshNet.Tests/Classes/SubsystemSession_Connect_SendSubsystemRequestFails.cs
new file mode 100644
index 00000000..c3639a40
--- /dev/null
+++ b/src/Renci.SshNet.Tests/Classes/SubsystemSession_Connect_SendSubsystemRequestFails.cs
@@ -0,0 +1,122 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using Renci.SshNet.Channels;
+using Renci.SshNet.Common;
+
+namespace Renci.SshNet.Tests.Classes
+{
+    [TestClass]
+    public class SubsystemSession_Connect_SendSubsystemRequestFails
+    {
+        private Mock<ISession> _sessionMock;
+        private Mock<IChannelSession> _channelMock;
+        private string _subsystemName;
+        private SubsystemSessionStub _subsystemSession;
+        private int _operationTimeout;
+        private IList<EventArgs> _disconnectedRegister;
+        private IList<ExceptionEventArgs> _errorOccurredRegister;
+        private SshException _actualException;
+        private MockSequence _sequence;
+
+        [TestInitialize]
+        public void Setup()
+        {
+            Arrange();
+            Act();
+        }
+
+        protected void Arrange()
+        {
+            var random = new Random();
+            _subsystemName = random.Next().ToString(CultureInfo.InvariantCulture);
+            _operationTimeout = 30000;
+            _disconnectedRegister = new List<EventArgs>();
+            _errorOccurredRegister = new List<ExceptionEventArgs>();
+
+            _sessionMock = new Mock<ISession>(MockBehavior.Strict);
+            _channelMock = new Mock<IChannelSession>(MockBehavior.Strict);
+
+            _sequence = new MockSequence();
+            _sessionMock.InSequence(_sequence).Setup(p => p.CreateChannelSession()).Returns(_channelMock.Object);
+            _channelMock.InSequence(_sequence).Setup(p => p.Open());
+            _channelMock.InSequence(_sequence).Setup(p => p.SendSubsystemRequest(_subsystemName)).Returns(false);
+            _channelMock.InSequence(_sequence).Setup(p => p.Dispose());
+
+            _subsystemSession = new SubsystemSessionStub(_sessionMock.Object,
+                                                         _subsystemName,
+                                                         _operationTimeout);
+            _subsystemSession.Disconnected += (sender, args) => _disconnectedRegister.Add(args);
+            _subsystemSession.ErrorOccurred += (sender, args) => _errorOccurredRegister.Add(args);
+        }
+
+        protected void Act()
+        {
+            try
+            {
+                _subsystemSession.Connect();
+                Assert.Fail();
+            }
+            catch (SshException ex)
+            {
+                _actualException = ex;
+            }
+        }
+
+        [TestMethod]
+        public void ConnectShouldHaveThrownSshException()
+        {
+            Assert.IsNotNull(_actualException);
+            Assert.IsNull(_actualException.InnerException);
+            Assert.AreEqual(string.Format(CultureInfo.InvariantCulture, "Subsystem '{0}' could not be executed.", _subsystemName), _actualException.Message);
+        }
+
+        [TestMethod]
+        public void ChannelShouldBeNull()
+        {
+            Assert.IsNull(_subsystemSession.Channel);
+        }
+
+        [TestMethod]
+        public void DisconnectHasNeverFired()
+        {
+            Assert.AreEqual(0, _disconnectedRegister.Count);
+        }
+
+        [TestMethod]
+        public void ErrorOccurredHasNeverFired()
+        {
+            Assert.AreEqual(0, _errorOccurredRegister.Count);
+        }
+
+        [TestMethod]
+        public void IsOpenShouldReturnFalse()
+        {
+            Assert.IsFalse(_subsystemSession.IsOpen);
+        }
+
+        [TestMethod]
+        public void DisposeOnChannelShouldBeInvokedOnce()
+        {
+            _channelMock.Verify(p => p.Dispose(), Times.Once);
+        }
+
+        [TestMethod]
+        public void ErrorOccuredOnSessionShouldNoLongerBeSignaledViaErrorOccurredOnSubsystemSession()
+        {
+            _sessionMock.Raise(p => p.ErrorOccured += null, new ExceptionEventArgs(new Exception()));
+
+            Assert.AreEqual(0, _errorOccurredRegister.Count);
+        }
+
+        [TestMethod]
+        public void DisconnectedOnSessionShouldNoLongerBeSignaledViaDisconnectedOnSubsystemSession()
+        {
+            _sessionMock.Raise(p => p.Disconnected += null, new EventArgs());
+
+            Assert.AreEqual(0, _disconnectedRegister.Count);
+        }
+    }
+}
diff --git a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
index b22ecaa1..5b4d0ff7 100644
--- a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
+++ b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
@@ -553,6 +553,7 @@
     <Compile Include="Classes\SubsystemSession_Connect_Disconnected.cs" />
     <Compile Include="Classes\SubsystemSession_Connect_Disposed.cs" />
     <Compile Include="Classes\SubsystemSession_Connect_NeverConnected.cs" />
+    <Compile Include="Classes\SubsystemSession_Connect_SendSubsystemRequestFails.cs" />
     <Compile Include="Classes\SubsystemSession_Disconnect_Connected.cs" />
     <Compile Include="Classes\SubsystemSession_Disconnect_Disposed.cs" />
     <Compile Include="Classes\SubsystemSession_Disconnect_NeverConnected.cs" />
diff --git a/src/Renci.SshNet/SubsystemSession.cs b/src/Renci.SshNet/SubsystemSession.cs
index 4aa52d5c..943a2db9 100644
--- a/src/Renci.SshNet/SubsystemSession.cs
+++ b/src/Renci.SshNet/SubsystemSession.cs
@@ -95,6 +95,7 @@ namespace Renci.SshNet
         /// </summary>
         /// <exception cref="InvalidOperationException">The session is already connected.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the session was disposed.</exception>
+        /// <exception cref="SshException">The channel session could not be opened, or the subsystem could not be executed.</exception>
         public void Connect()
         {
             EnsureNotDisposed();
@@ -116,7 +117,16 @@ namespace Renci.SshNet
             _channel.Exception += Channel_Exception;
             _channel.Closed += Channel_Closed;
             _channel.Open();
-            _channel.SendSubsystemRequest(_subsystemName);
+
+            if (!_channel.SendSubsystemRequest(_subsystemName))
+            {
+                // close channel session
+                Disconnect();
+                // signal subsystem failure
+                throw new SshException(string.Format(CultureInfo.InvariantCulture,
+                                                     "Subsystem '{0}' could not be executed.",
+                                                     _subsystemName));
+            }
 
             OnChannelOpen();
         }