Improved exception messages.

Avoid magic numbers.
Improve test coverage.
This commit is contained in:
drieseng
2016-06-17 12:01:00 +02:00
parent a1f46d40c2
commit f5b5542684
2 changed files with 448 additions and 48 deletions
+76 -24
View File
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Renci.SshNet.Common;
using Renci.SshNet.Messages;
using Renci.SshNet.Messages.Authentication;
@@ -14,9 +15,19 @@ namespace Renci.SshNet
private readonly MessageMetadata[] _enabledMessagesByNumber;
private readonly bool[] _activatedMessagesById;
private static readonly MessageMetadata[] AllMessages;
internal static readonly MessageMetadata[] AllMessages;
private static readonly IDictionary<string, MessageMetadata> MessagesByName;
/// <summary>
/// Defines the highest message number that is currently supported.
/// </summary>
internal const byte HighestMessageNumber = 100;
/// <summary>
/// Defines the total number of supported messages.
/// </summary>
internal const int TotalMessageCount = 31;
static SshMessageFactory()
{
AllMessages = new MessageMetadata[]
@@ -54,15 +65,15 @@ namespace Renci.SshNet
new MessageMetadata<KeyExchangeDhGroupExchangeReply> (30, "SSH_MSG_KEX_DH_GEX_REPLY", 33)
};
MessagesByName = new Dictionary<string, MessageMetadata>(31);
MessagesByName = new Dictionary<string, MessageMetadata>(AllMessages.Length);
foreach (var messageMetadata in AllMessages)
MessagesByName.Add(messageMetadata.Name, messageMetadata);
}
public SshMessageFactory()
{
_activatedMessagesById = new bool[31];
_enabledMessagesByNumber = new MessageMetadata[101];
_activatedMessagesById = new bool[TotalMessageCount];
_enabledMessagesByNumber = new MessageMetadata[HighestMessageNumber + 1];
}
/// <summary>
@@ -76,14 +87,24 @@ namespace Renci.SshNet
public Message Create(byte messageNumber)
{
var messageMetadata = _enabledMessagesByNumber[messageNumber];
if (messageMetadata == null)
if (messageNumber > HighestMessageNumber)
{
throw new SshException(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid.", messageNumber));
throw CreateMessageTypeNotSupportedException(messageNumber);
}
return messageMetadata.Create();
var enabledMessageMetadata = _enabledMessagesByNumber[messageNumber];
if (enabledMessageMetadata == null)
{
var definedMessageMetadata = AllMessages.FirstOrDefault(p => p.Number == messageNumber);
if (definedMessageMetadata == null)
{
throw CreateMessageTypeNotSupportedException(messageNumber);
}
throw new SshException(string.Format(CultureInfo.InvariantCulture, "Message type {0} is not valid in the current context.", messageNumber));
}
return enabledMessageMetadata.Create();
}
public void DisableNonKeyExchangeMessages()
@@ -106,10 +127,12 @@ namespace Renci.SshNet
if (!_activatedMessagesById[messageMetadata.Id])
continue;
var enabledMessage = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMessage != null && enabledMessage != messageMetadata)
var enabledMessageMetadata = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMessageMetadata != null && enabledMessageMetadata != messageMetadata)
{
throw new Exception("Message X is already enabled for Y");
throw CreateMessageTypeAlreadyEnabledForOtherMessageException(messageMetadata.Number,
messageMetadata.Name,
enabledMessageMetadata.Name);
}
_enabledMessagesByNumber[messageMetadata.Number] = messageMetadata;
}
@@ -117,47 +140,76 @@ namespace Renci.SshNet
public void EnableAndActivateMessage(string messageName)
{
if (messageName == null)
throw new ArgumentNullException("messageName");
lock (this)
{
MessageMetadata messageMetadata;
if (!MessagesByName.TryGetValue(messageName, out messageMetadata))
{
throw new Exception("TODO");
throw CreateMessageNotSupportedException(messageName);
}
var enabledMessage = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMessage != null && enabledMessage != messageMetadata)
var enabledMessageMetadata = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMessageMetadata != null && enabledMessageMetadata != messageMetadata)
{
throw new Exception("Message X is already enabled for Y");
throw CreateMessageTypeAlreadyEnabledForOtherMessageException(messageMetadata.Number,
messageMetadata.Name,
enabledMessageMetadata.Name);
}
_enabledMessagesByNumber[messageMetadata.Number] = messageMetadata;
_enabledMessagesByNumber[messageMetadata.Number] = messageMetadata;
_activatedMessagesById[messageMetadata.Id] = true;
}
}
public void DisableAndDeactivateMessage(string messageName)
{
if (messageName == null)
throw new ArgumentNullException("messageName");
lock (this)
{
MessageMetadata messageMetadata;
if (!MessagesByName.TryGetValue(messageName, out messageMetadata))
{
throw new Exception("TODO");
throw CreateMessageNotSupportedException(messageName);
}
var enabledMessageMetadata = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMessageMetadata != null && enabledMessageMetadata != messageMetadata)
{
throw CreateMessageTypeAlreadyEnabledForOtherMessageException(messageMetadata.Number,
messageMetadata.Name,
enabledMessageMetadata.Name);
}
_activatedMessagesById[messageMetadata.Id] = false;
var enabledMetadata = _enabledMessagesByNumber[messageMetadata.Number];
if (enabledMetadata != null && enabledMetadata != messageMetadata)
throw new Exception();
_enabledMessagesByNumber[messageMetadata.Number] = null;
}
}
private abstract class MessageMetadata
private SshException CreateMessageTypeNotSupportedException(byte messageNumber)
{
throw new SshException(string.Format(CultureInfo.InvariantCulture, "Message type {0} is not supported.", messageNumber));
}
private SshException CreateMessageNotSupportedException(string messageName)
{
throw new SshException(string.Format(CultureInfo.InvariantCulture, "Message '{0}' is not supported.", messageName));
}
private SshException CreateMessageTypeAlreadyEnabledForOtherMessageException(byte messageNumber, string messageName, string currentEnabledForMessageName)
{
throw new SshException(string.Format(CultureInfo.InvariantCulture,
"Cannot enable message '{0}'. Message type {1} is already enabled for '{2}'.",
messageName, messageNumber, currentEnabledForMessageName));
}
internal abstract class MessageMetadata
{
protected MessageMetadata(byte id, string name, byte number)
{
@@ -175,7 +227,7 @@ namespace Renci.SshNet
public abstract Message Create();
}
private class MessageMetadata<T> : MessageMetadata where T : Message, new()
internal class MessageMetadata<T> : MessageMetadata where T : Message, new()
{
public MessageMetadata(byte id, string name, byte number)
: base(id, name, number)
@@ -30,56 +30,404 @@ namespace Renci.SshNet.Tests
}
[TestMethod]
public void EnableInformationRequestMessage()
public void CreateShouldThrowSshExceptionWhenMessageIsNotEnabled()
{
const byte messageNumber = 60;
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void CreateShouldThrowSshExceptionWhenMessageDoesNotExist_OutsideOfMessageNumberRange()
{
const byte messageNumber = 255;
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not supported.", messageNumber), ex.Message);
}
}
[TestMethod]
public void CreateShouldThrowSshExceptionWhenMessageDoesNotExist_WithinMessageNumberRange()
{
const byte messageNumber = 5;
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not supported.", messageNumber), ex.Message);
}
}
[TestMethod]
public void CreateShouldThrowSshExceptionWhenMessageIsNotActivated()
{
const byte messageNumber = 60;
const string messageName = "SSH_MSG_USERAUTH_PASSWD_CHANGEREQ";
_sshMessageFactory.EnableAndActivateMessage(messageName);
_sshMessageFactory.DisableAndDeactivateMessage(messageName);
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void CreateShouldReturnMessageInstanceCorrespondingToMessageNumberWhenMessageIsEnabledAndActivated()
{
const byte messageNumber = 60;
const string messageName = "SSH_MSG_USERAUTH_PASSWD_CHANGEREQ";
_sshMessageFactory.EnableAndActivateMessage(messageName);
var actual = _sshMessageFactory.Create(messageNumber);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof (PasswordChangeRequiredMessage), actual.GetType());
_sshMessageFactory.DisableAndDeactivateMessage(messageName);
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
var message = _sshMessageFactory.Create(60);
Assert.AreEqual(typeof(InformationRequestMessage), message.GetType());
actual = _sshMessageFactory.Create(messageNumber);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(InformationRequestMessage), actual.GetType());
}
[TestMethod]
public void EnablePasswordChangeRequiredMessage()
public void DisableAndDeactivateMessageShouldThrowSshExceptionWhenAnotherMessageWithSameMessageNumberIsEnabled()
{
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
var message = _sshMessageFactory.Create(60);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), message.GetType());
try
{
_sshMessageFactory.DisableAndDeactivateMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual("Cannot enable message 'SSH_MSG_USERAUTH_INFO_REQUEST'. Message type 60 is already enabled for 'SSH_MSG_USERAUTH_PASSWD_CHANGEREQ'.", ex.Message);
}
// verify that the original message remains enabled
var actual = _sshMessageFactory.Create(60);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), actual.GetType());
}
[TestMethod]
public void Di()
public void DisableAndDeactivateMessageShouldNotThrowExceptionWhenMessageIsAlreadyDisabled()
{
const byte messageNumber = 60;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableAndDeactivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableAndDeactivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
// verify that message remains disabled
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void DisableAndDeactivateMessageShouldNotThrowExceptionWhenMessageWasNeverEnabled()
{
const byte messageNumber = 60;
_sshMessageFactory.DisableAndDeactivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
// verify that message is disabled
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void DisableAndDeactivateMessageShouldThrowSshExceptionWhenMessageIsNotSupported()
{
const string messageName = "WHATEVER";
try
{
_sshMessageFactory.DisableAndDeactivateMessage("WHATEVER");
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format("Message '{0}' is not supported.", messageName), ex.Message);
}
}
[TestMethod]
public void DisableAndDeactivateMessageShouldThrowArgumentNullExceptionWhenMessageNameIsNull()
{
const string messageName = null;
try
{
_sshMessageFactory.DisableAndDeactivateMessage(messageName);
Assert.Fail();
}
catch (ArgumentNullException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual("messageName", ex.ParamName);
}
}
[TestMethod]
public void EnableAndActivateMessageShouldThrowSshExceptionWhenAnotherMessageWithSameMessageNumberIsAlreadyEnabled()
{
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_SERVICE_ACCEPT");
try
{
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual("Cannot enable message 'SSH_MSG_USERAUTH_INFO_REQUEST'. Message type 60 is already enabled for 'SSH_MSG_USERAUTH_PASSWD_CHANGEREQ'.", ex.Message);
}
// verify that the original message remains enabled
var actual = _sshMessageFactory.Create(60);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), actual.GetType());
}
[TestMethod]
public void EnableAndActivateMessageShouldNotThrowExceptionWhenMessageIsAlreadyEnabled()
{
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
var actual = _sshMessageFactory.Create(60);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), actual.GetType());
}
[TestMethod]
public void EnableAndActivateMessageShouldThrowSshExceptionWhenMessageIsNotSupported()
{
const string messageName = "WHATEVER";
try
{
_sshMessageFactory.EnableAndActivateMessage("WHATEVER");
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format("Message '{0}' is not supported.", messageName), ex.Message);
}
}
[TestMethod]
public void EnableAndActivateMessageShouldThrowArgumentNullExceptionWhenMessageNameIsNull()
{
const string messageName = null;
try
{
_sshMessageFactory.EnableAndActivateMessage(messageName);
Assert.Fail();
}
catch (ArgumentNullException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual("messageName", ex.ParamName);
}
}
[TestMethod]
public void DisableNonKeyExchangeMessagesShouldDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableNonKeyExchangeMessages();
// verify that message is disabled
try
{
_sshMessageFactory.Create(6); // SSH_MSG_SERVICE_ACCEPT
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException)
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
try
{
_sshMessageFactory.Create(60);
// SSH_MSG_USERAUTH_PASSWD_CHANGEREQ or SSH_MSG_USERAUTH_INFO_REQUEST or SSH_MSG_USERAUTH_PK_OK
Assert.Fail();
}
catch (SshException)
{
}
[TestMethod]
public void DisableNonKeyExchangeMessagesShouldNotDisableKeyExchangeMessages()
{
const byte messageNumber = 21;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
var message = _sshMessageFactory.Create(60);
Assert.AreEqual(typeof(InformationRequestMessage), message.GetType());
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_NEWKEYS");
_sshMessageFactory.DisableNonKeyExchangeMessages();
// verify that message remains enabled
var actual = _sshMessageFactory.Create(messageNumber);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof (NewKeysMessage), actual.GetType());
}
[TestMethod]
public void EnableActivatedMessagesShouldEnableMessagesThatWereEnabledPriorToInvokingDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableNonKeyExchangeMessages();
_sshMessageFactory.EnableActivatedMessages();
var message2 = _sshMessageFactory.Create(60);
Assert.AreEqual(typeof(InformationRequestMessage), message2.GetType());
var actual = _sshMessageFactory.Create(messageNumber);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), actual.GetType());
}
[TestMethod]
public void EnableActivatedMessagesShouldNotEnableMessagesThatWereDisabledPriorToInvokingDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.DisableNonKeyExchangeMessages();
_sshMessageFactory.EnableActivatedMessages();
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void EnableActivatedMessagesShouldNotEnableMessagesThatWereDisabledAfterInvokingDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableNonKeyExchangeMessages();
_sshMessageFactory.DisableAndDeactivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.EnableActivatedMessages();
try
{
_sshMessageFactory.Create(messageNumber);
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Message type {0} is not valid in the current context.", messageNumber), ex.Message);
}
}
[TestMethod]
public void EnableActivatedMessagesShouldThrowSshExceptionWhenAnothersMessageWithSameMessageNumberWasEnabledAfterInvokingDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.DisableNonKeyExchangeMessages();
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
try
{
_sshMessageFactory.EnableActivatedMessages();
Assert.Fail();
}
catch (SshException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual("Cannot enable message 'SSH_MSG_USERAUTH_PASSWD_CHANGEREQ'. Message type 60 is already enabled for 'SSH_MSG_USERAUTH_INFO_REQUEST'.", ex.Message);
}
}
[TestMethod]
public void EnableActivatedMessagesShouldLeaveMessagesEnabledThatWereEnabledAfterInvokingDisableNonKeyExchangeMessages()
{
const byte messageNumber = 60;
_sshMessageFactory.DisableNonKeyExchangeMessages();
_sshMessageFactory.EnableAndActivateMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
_sshMessageFactory.EnableActivatedMessages();
var actual = _sshMessageFactory.Create(messageNumber);
Assert.IsNotNull(actual);
Assert.AreEqual(typeof(PasswordChangeRequiredMessage), actual.GetType());
}
[TestMethod]
public void HighestMessageNumberShouldCorrespondWithHighestSupportedMessageNumber()
{
var highestSupportMessageNumber = SshMessageFactory.AllMessages.Max(m => m.Number);
Assert.AreEqual(highestSupportMessageNumber, SshMessageFactory.HighestMessageNumber);
}
[TestMethod]
public void TotalMessageCountShouldBeTotalNumberOfSupportedMessages()
{
var totalNumberOfSupportedMessages = SshMessageFactory.AllMessages.Length;
Assert.AreEqual(totalNumberOfSupportedMessages, SshMessageFactory.TotalMessageCount);
}
[TestMethod]