Skip to content

Commit e8db68c

Browse files
driesengdrieseng
committed
Improve exception messages.
1 parent 8d485f9 commit e8db68c

9 files changed

+150
-37
lines changed

‎src/Renci.SshNet.Tests/Classes/Connection/HttpConnectorTest_Connect_ProxyUserNameIsEmpty.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,14 @@ protected override void TearDown()
7575
{
7676
base.TearDown();
7777

78-
if (_clientSocket != null)
78+
if (_proxyServer != null)
7979
{
80-
_clientSocket.Shutdown(SocketShutdown.Send);
81-
_clientSocket.Close();
80+
_proxyServer.Dispose();
8281
}
8382

84-
if (_proxyServer != null)
83+
if (_clientSocket != null)
8584
{
86-
_proxyServer.Dispose();
85+
_clientSocket.Close();
8786
}
8887
}
8988

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
using System.Net;
88
using System.Net.Sockets;
99

10-
namespace Renci.SshNet.Tests.Classes
10+
namespace Renci.SshNet.Tests.Classes.Connection
1111
{
1212
[TestClass]
1313
public class ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer
@@ -58,7 +58,7 @@ protected void Arrange()
5858
_server.BytesReceived += (bytes, socket) =>
5959
{
6060
_dataReceivedByServer.AddRange(bytes);
61-
socket.Shutdown(SocketShutdown.Both);
61+
socket.Shutdown(SocketShutdown.Send);
6262
};
6363
_server.Disconnected += (socket) => _clientDisconnected = true;
6464

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerDoesNotRespondWithIdentificationStringBeforeTimeout.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
using System.Text;
1111
using System.Threading;
1212

13-
namespace Renci.SshNet.Tests.Classes
13+
namespace Renci.SshNet.Tests.Classes.Connection
1414
{
1515
[TestClass]
1616
public class ProtocolVersionExchangeTest_ServerDoesNotRespondWithIdentificationStringBeforeTimeout
@@ -65,8 +65,10 @@ protected void Arrange()
6565
{
6666
_dataReceivedByServer.AddRange(bytes);
6767
socket.Send(Encoding.UTF8.GetBytes("Welcome!\r\n"));
68+
/*
6869
Thread.Sleep(_timeout.Add(TimeSpan.FromMilliseconds(50)));
69-
socket.Shutdown(SocketShutdown.Both);
70+
socket.Shutdown(SocketShutdown.Send);
71+
*/
7072
};
7173
_server.Disconnected += (socket) => _clientDisconnected = true;
7274

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
using System.Net.Sockets;
1010
using System.Text;
1111

12-
namespace Renci.SshNet.Tests.Classes
12+
namespace Renci.SshNet.Tests.Classes.Connection
1313
{
1414
[TestClass]
1515
public class ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
using System.Net.Sockets;
1010
using System.Text;
1111

12-
namespace Renci.SshNet.Tests.Classes
12+
namespace Renci.SshNet.Tests.Classes.Connection
1313
{
1414
[TestClass]
1515
public class ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion
@@ -63,7 +63,7 @@ protected void Arrange()
6363
{
6464
_dataReceivedByServer.AddRange(bytes);
6565
socket.Send(_serverIdentification);
66-
socket.Shutdown(SocketShutdown.Both);
66+
socket.Shutdown(SocketShutdown.Send);
6767
};
6868
_server.Disconnected += (socket) => _clientDisconnected = true;
6969

@@ -89,8 +89,11 @@ protected void Act()
8989
[TestMethod]
9090
public void StartShouldHaveThrownSshConnectionException()
9191
{
92-
var expectedMessage = "Server response does not contain SSH protocol identification:" + Environment.NewLine +
93-
" 00000000 53 53 48 2D 32 2E 30 0D 0A SSH-2.0..";
92+
var expectedMessage = string.Format("The server response does not contain an SSH protocol identification:{0}{0}" +
93+
" 00000000 53 53 48 2D 32 2E 30 0D 0A SSH-2.0..{0}{0}" +
94+
"More information is available here:{0}" +
95+
"https://tools.ietf.org/html/rfc4253#section-4.2",
96+
Environment.NewLine);
9497

9598
Assert.IsNotNull(_actualException);
9699
Assert.IsNull(_actualException.InnerException);

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_Comments.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
using System.Net.Sockets;
1010
using System.Text;
1111

12-
namespace Renci.SshNet.Tests.Classes
12+
namespace Renci.SshNet.Tests.Classes.Connection
1313
{
1414
[TestClass]
1515
public class ProtocolVersionExchangeTest_ServerResponseValid_Comments
@@ -63,7 +63,7 @@ protected void Arrange()
6363
{
6464
_dataReceivedByServer.AddRange(bytes);
6565
socket.Send(_serverIdentification);
66-
socket.Shutdown(SocketShutdown.Both);
66+
socket.Shutdown(SocketShutdown.Send);
6767
};
6868
_server.Disconnected += (socket) => _clientDisconnected = true;
6969

‎src/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_NoComments.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
using System.Net.Sockets;
1010
using System.Text;
1111

12-
namespace Renci.SshNet.Tests.Classes
12+
namespace Renci.SshNet.Tests.Classes.Connection
1313
{
1414
[TestClass]
1515
public class ProtocolVersionExchangeTest_ServerResponseValid_NoComments
@@ -63,7 +63,7 @@ protected void Arrange()
6363
{
6464
_dataReceivedByServer.AddRange(bytes);
6565
socket.Send(_serverIdentification);
66-
socket.Shutdown(SocketShutdown.Both);
66+
socket.Shutdown(SocketShutdown.Send);
6767
};
6868
_server.Disconnected += (socket) => _clientDisconnected = true;
6969

‎src/Renci.SshNet.Tests/Common/AsyncSocketListener.cs

Lines changed: 114 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class AsyncSocketListener : IDisposable
1818
private Thread _receiveThread;
1919
private bool _started;
2020
private object _syncLock;
21+
private string _stackTrace;
2122

2223
public delegate void BytesReceivedHandler(byte[] bytesReceived, Socket socket);
2324
public delegate void ConnectedHandler(Socket socket);
@@ -56,6 +57,8 @@ public void Start()
5657

5758
_receiveThread = new Thread(StartListener);
5859
_receiveThread.Start(_listener);
60+
61+
_stackTrace = Environment.StackTrace;
5962
}
6063

6164
public void Stop()
@@ -78,7 +81,10 @@ public void Stop()
7881
}
7982

8083
DrainSocket(connectedClient);
84+
connectedClient.Dispose();
8185
}
86+
87+
_connectedClients.Clear();
8288
}
8389

8490
if (_listener != null)
@@ -116,34 +122,70 @@ private void AcceptCallback(IAsyncResult ar)
116122
_acceptCallbackDone.Set();
117123

118124
// Get the socket that listens for inbound connections
119-
var listener = (Socket) ar.AsyncState;
125+
var listener = (Socket)ar.AsyncState;
120126

121127
// Get the socket that handles the client request
122-
var handler = listener.EndAccept(ar);
128+
Socket handler;
129+
130+
try
131+
{
132+
handler = listener.EndAccept(ar);
133+
}
134+
catch (ObjectDisposedException ex)
135+
{
136+
// The listener is stopped through a Dispose() call, which in turn causes
137+
// Socket.EndAccept(IAsyncResult) to throw an ObjectDisposedException
138+
//
139+
// Since we consider this ObjectDisposedException normal when the listener
140+
// is being stopped, we only write a message to stderr if the listener
141+
// is considered to be up and running
142+
if (_started)
143+
{
144+
Console.Error.WriteLine("[{0}] Failure accepting new connection: {1}",
145+
typeof(AsyncSocketListener).FullName,
146+
ex);
147+
}
148+
149+
return;
150+
}
123151

124152
// Signal new connection
125153
SignalConnected(handler);
126154

127-
// Register client socket
128-
_connectedClients.Add(handler);
155+
lock (_syncLock)
156+
{
157+
// Register client socket
158+
_connectedClients.Add(handler);
159+
}
160+
161+
var state = new SocketStateObject(handler);
129162

130163
try
131164
{
132-
var state = new SocketStateObject(handler);
133165
handler.BeginReceive(state.Buffer, 0, state.Buffer.Length, 0, ReadCallback, state);
134166
}
135-
catch (ObjectDisposedException)
167+
catch (ObjectDisposedException ex)
136168
{
137-
// when the socket is closed, an ObjectDisposedException is thrown
138-
// by Socket.EndAccept(IAsyncResult)
169+
// The listener is stopped through a Dispose() call, which in turn causes
170+
// Socket.BeginReceive(...) to throw an ObjectDisposedException
171+
//
172+
// Since we consider this ObjectDisposedException normal when the listener
173+
// is being stopped, we only write a message to stderr if the listener
174+
// is considered to be up and running
175+
if (_started)
176+
{
177+
Console.Error.WriteLine("[{0}] Failure receiving new data: {1}",
178+
typeof(AsyncSocketListener).FullName,
179+
ex);
180+
}
139181
}
140182
}
141183

142184
private void ReadCallback(IAsyncResult ar)
143185
{
144186
// Retrieve the state object and the handler socket
145187
// from the asynchronous state object
146-
var state = (SocketStateObject) ar.AsyncState;
188+
var state = (SocketStateObject)ar.AsyncState;
147189
var handler = state.Socket;
148190

149191
int bytesRead;
@@ -152,11 +194,38 @@ private void ReadCallback(IAsyncResult ar)
152194
// Read data from the client socket.
153195
bytesRead = handler.EndReceive(ar);
154196
}
155-
catch (ObjectDisposedException)
197+
catch (SocketException ex)
156198
{
157-
// when the socket is closed, the callback will be invoked for any pending BeginReceive
158-
// we could use the Socket.Connected property to detect this here, but the proper thing
159-
// to do is invoke EndReceive knowing that it will throw an ObjectDisposedException
199+
// The listener is stopped through a Dispose() call, which in turn causes
200+
// Socket.EndReceive(...) to throw a SocketException or
201+
// ObjectDisposedException
202+
//
203+
// Since we consider such an exception normal when the listener is being
204+
// stopped, we only write a message to stderr if the listener is considered
205+
// to be up and running
206+
if (_started)
207+
{
208+
Console.Error.WriteLine("[{0}] Failure receiving new data: {1}",
209+
typeof(AsyncSocketListener).FullName,
210+
ex);
211+
}
212+
return;
213+
}
214+
catch (ObjectDisposedException ex)
215+
{
216+
// The listener is stopped through a Dispose() call, which in turn causes
217+
// Socket.EndReceive(...) to throw a SocketException or
218+
// ObjectDisposedException
219+
//
220+
// Since we consider such an exception normal when the listener is being
221+
// stopped, we only write a message to stderr if the listener is considered
222+
// to be up and running
223+
if (_started)
224+
{
225+
Console.Error.WriteLine("[{0}] Failure receiving new data: {1}",
226+
typeof(AsyncSocketListener).FullName,
227+
ex);
228+
}
160229
return;
161230
}
162231

@@ -165,7 +234,21 @@ private void ReadCallback(IAsyncResult ar)
165234
var bytesReceived = new byte[bytesRead];
166235
Array.Copy(state.Buffer, bytesReceived, bytesRead);
167236
SignalBytesReceived(bytesReceived, handler);
168-
handler.BeginReceive(state.Buffer, 0, state.Buffer.Length, 0, ReadCallback, state);
237+
238+
try
239+
{
240+
handler.BeginReceive(state.Buffer, 0, state.Buffer.Length, 0, ReadCallback, state);
241+
}
242+
catch (SocketException ex)
243+
{
244+
if (!_started)
245+
{
246+
throw new Exception("BeginReceive while stopping!", ex);
247+
}
248+
249+
throw new Exception("BeginReceive while started!: " + ex.SocketErrorCode + " " + _stackTrace, ex);
250+
}
251+
169252
}
170253
else
171254
{
@@ -175,8 +258,23 @@ private void ReadCallback(IAsyncResult ar)
175258
{
176259
lock (_syncLock)
177260
{
178-
handler.Shutdown(SocketShutdown.Send);
179-
handler.Close();
261+
if (!_started)
262+
{
263+
return;
264+
}
265+
try
266+
{
267+
handler.Shutdown(SocketShutdown.Send);
268+
handler.Close();
269+
}
270+
catch (SocketException ex)
271+
{
272+
throw new Exception("Exception in ReadCallback: " + ex.SocketErrorCode + " " + _stackTrace, ex);
273+
}
274+
catch (Exception ex)
275+
{
276+
throw new Exception("Exception in ReadCallback: " + _stackTrace, ex);
277+
}
180278

181279
_connectedClients.Remove(handler);
182280
}

‎src/Renci.SshNet/Connection/ProtocolVersionExchange.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ namespace Renci.SshNet.Connection
1313
/// <summary>
1414
/// Handles the SSH protocol version exchange.
1515
/// </summary>
16+
/// <remarks>
17+
/// https://tools.ietf.org/html/rfc4253#section-4.2
18+
/// </remarks>
1619
internal class ProtocolVersionExchange : IProtocolVersionExchange
1720
{
1821
private const byte Null = 0x00;
@@ -49,10 +52,14 @@ public SshIdentification Start(string clientVersion, Socket socket, TimeSpan tim
4952
{
5053
if (bytesReceived.Count == 0)
5154
{
52-
throw new SshConnectionException("Server response does not contain SSH protocol identification. Connection to remote server was closed before any data was received.", DisconnectReason.ConnectionLost);
55+
throw new SshConnectionException("The server response does not contain an SSH protocol identification. Connection to remote server was closed before any data was received.", DisconnectReason.ConnectionLost);
5356
}
5457

55-
throw new SshConnectionException(string.Format("Server response does not contain SSH protocol identification:{0}{1}", Environment.NewLine, PacketDump.Create(bytesReceived, 2)),
58+
throw new SshConnectionException(string.Format("The server response does not contain an SSH protocol identification:{0}{0}{1}{0}{0}" +
59+
"More information is available here:{0}" +
60+
"https://tools.ietf.org/html/rfc4253#section-4.2",
61+
Environment.NewLine,
62+
PacketDump.Create(bytesReceived, 2)),
5663
DisconnectReason.ProtocolError);
5764
}
5865

@@ -112,7 +119,11 @@ private static string SocketReadLine(Socket socket, TimeSpan timeout, List<byte>
112119
if (byteRead == Null)
113120
{
114121
throw new SshConnectionException(string.Format(CultureInfo.InvariantCulture,
115-
"The identification string contains a null character at position 0x{0:X8}:{1}{2}",
122+
"The server response contains a null character at position 0x{0:X8}:{1}{1}{2}{1}{1}" +
123+
"A server must not send a null character before the Protocol Version Exchange is{1}" +
124+
"complete.{1}{1}" +
125+
"More information is available here:{1}" +
126+
"https://tools.ietf.org/html/rfc4253#section-4.2",
116127
buffer.Count,
117128
Environment.NewLine,
118129
PacketDump.Create(buffer.ToArray(), 2)));

0 commit comments

Comments
 (0)