diff --git a/src/Renci.SshNet/SshMessageFactory.cs b/src/Renci.SshNet/SshMessageFactory.cs index 996edb43..61b0bb62 100644 --- a/src/Renci.SshNet/SshMessageFactory.cs +++ b/src/Renci.SshNet/SshMessageFactory.cs @@ -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 MessagesByName; + /// + /// Defines the highest message number that is currently supported. + /// + internal const byte HighestMessageNumber = 100; + + /// + /// Defines the total number of supported messages. + /// + internal const int TotalMessageCount = 31; + static SshMessageFactory() { AllMessages = new MessageMetadata[] @@ -54,15 +65,15 @@ namespace Renci.SshNet new MessageMetadata (30, "SSH_MSG_KEX_DH_GEX_REPLY", 33) }; - MessagesByName = new Dictionary(31); + MessagesByName = new Dictionary(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]; } /// @@ -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 : MessageMetadata where T : Message, new() + internal class MessageMetadata : MessageMetadata where T : Message, new() { public MessageMetadata(byte id, string name, byte number) : base(id, name, number) diff --git a/test/Renci.SshNet.Shared.Tests/SshMessageFactoryTest.cs b/test/Renci.SshNet.Shared.Tests/SshMessageFactoryTest.cs index 3b8eddac..707b0203 100644 --- a/test/Renci.SshNet.Shared.Tests/SshMessageFactoryTest.cs +++ b/test/Renci.SshNet.Shared.Tests/SshMessageFactoryTest.cs @@ -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]