From f94da0b36af1d0ee46ec3d15b265cd7191f69a76 Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Thu, 10 Aug 2017 17:58:09 -0700 Subject: [PATCH 1/2] Add null checks for MAC address functionality, add initial test for empty MAC address --- src/Common/Commands.Common/MetricHelper.cs | 11 +++++--- .../AzureRMCmdlet.cs | 2 +- .../Commands.Profile.Test/TelemetryTests.cs | 28 +++++++++++++++++-- .../AzureSMCmdlet.cs | 2 +- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/Common/Commands.Common/MetricHelper.cs b/src/Common/Commands.Common/MetricHelper.cs index c70b885b188c..8772c418129a 100644 --- a/src/Common/Commands.Common/MetricHelper.cs +++ b/src/Common/Commands.Common/MetricHelper.cs @@ -62,10 +62,13 @@ private static string HashMacAddress { if (_hashMacAddress == string.Empty) { - var macAddress = NetworkInterface.GetAllNetworkInterfaces() - .FirstOrDefault(nic => nic.OperationalStatus == OperationalStatus.Up)? + var macAddress = NetworkInterface.GetAllNetworkInterfaces()? + .FirstOrDefault(nic => nic != null && + nic.OperationalStatus == OperationalStatus.Up && + nic.GetPhysicalAddress() != null && + !string.IsNullOrWhiteSpace(nic.GetPhysicalAddress().ToString()))? .GetPhysicalAddress()?.ToString(); - _hashMacAddress = macAddress == null ? null : GenerateSha256HashString(macAddress).Replace("-", string.Empty).ToLowerInvariant(); + _hashMacAddress = string.IsNullOrWhiteSpace(macAddress) ? null : GenerateSha256HashString(macAddress).Replace("-", string.Empty).ToLowerInvariant(); } return _hashMacAddress; @@ -246,7 +249,7 @@ public static string GenerateSha256HashString(string originInput) { if (string.IsNullOrWhiteSpace(originInput)) { - throw new ArgumentNullException("originInput"); + return null; } using (var sha256 = new SHA256CryptoServiceProvider()) diff --git a/src/ResourceManager/Common/Commands.ResourceManager.Common/AzureRMCmdlet.cs b/src/ResourceManager/Common/Commands.ResourceManager.Common/AzureRMCmdlet.cs index c1fe9d21c0f3..0c78e8013254 100644 --- a/src/ResourceManager/Common/Commands.ResourceManager.Common/AzureRMCmdlet.cs +++ b/src/ResourceManager/Common/Commands.ResourceManager.Common/AzureRMCmdlet.cs @@ -225,7 +225,7 @@ protected override void InitializeQosEvent() if (this.DefaultProfile != null && this.DefaultProfile.DefaultContext != null && this.DefaultProfile.DefaultContext.Account != null && - this.DefaultProfile.DefaultContext.Account.Id != null) + !string.IsNullOrWhiteSpace(this.DefaultProfile.DefaultContext.Account.Id)) { _qosEvent.Uid = MetricHelper.GenerateSha256HashString( this.DefaultProfile.DefaultContext.Account.Id.ToString()); diff --git a/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs b/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs index 1a4a9db2817a..152ca2c671b9 100644 --- a/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs +++ b/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs @@ -14,7 +14,9 @@ using Microsoft.WindowsAzure.Commands.Common; using Microsoft.WindowsAzure.Commands.ScenarioTest; +using Moq; using System; +using System.Net.NetworkInformation; using Xunit; namespace Microsoft.Azure.Commands.Profile.Test @@ -23,9 +25,11 @@ public class TelemetryTests { [Fact] [Trait(Category.AcceptanceType, Category.CheckIn)] - public void HashOfNullValueThrowsAppropriateException() + public void HashOfNullOrWhitespaceValueReturnsNull() { - Assert.Throws(() => MetricHelper.GenerateSha256HashString(null)); + Assert.Null(MetricHelper.GenerateSha256HashString(null)); + Assert.Null(MetricHelper.GenerateSha256HashString(string.Empty)); + Assert.Null(MetricHelper.GenerateSha256HashString(" ")); } [Fact] @@ -38,5 +42,25 @@ public void HashOfValidValueSucceeds() Assert.True(hash.Length > 0); Assert.NotEqual(inputValue, hash, StringComparer.OrdinalIgnoreCase); } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void NetworkInterfaceWithEmptyAddressReturnsNull() + { + var networkInterfaceMock = new Mock(); + networkInterfaceMock.Setup(t => t.GetPhysicalAddress()) + .Returns( + () => + { + return new PhysicalAddress(new byte[] { }); + } + ); + + var address = networkInterfaceMock.Object.GetPhysicalAddress(); + Assert.NotNull(address); + Assert.Equal(string.Empty, address.ToString()); + var hashAddress = MetricHelper.GenerateSha256HashString(address.ToString()); + Assert.Null(hashAddress); + } } } diff --git a/src/ServiceManagement/Common/Commands.ServiceManagement.Common/AzureSMCmdlet.cs b/src/ServiceManagement/Common/Commands.ServiceManagement.Common/AzureSMCmdlet.cs index 7ab14a80122e..615c4f083399 100644 --- a/src/ServiceManagement/Common/Commands.ServiceManagement.Common/AzureSMCmdlet.cs +++ b/src/ServiceManagement/Common/Commands.ServiceManagement.Common/AzureSMCmdlet.cs @@ -111,7 +111,7 @@ protected override void InitializeQosEvent() if (this.DefaultContext != null && this.DefaultContext.Account != null && - this.DefaultContext.Account.Id != null) + !string.IsNullOrEmpty(this.DefaultContext.Account.Id)) { _qosEvent.Uid = MetricHelper.GenerateSha256HashString( this.DefaultContext.Account.Id.ToString()); From 6a1ba6a7f2ec96e6aca3d6e5b116fcca3bc5bd9e Mon Sep 17 00:00:00 2001 From: cormacpayne Date: Fri, 11 Aug 2017 09:21:13 -0700 Subject: [PATCH 2/2] Resolve cod review feedback --- src/Common/Commands.Common/MetricHelper.cs | 4 ++-- .../Commands.Profile.Test/TelemetryTests.cs | 24 ++++++------------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/Common/Commands.Common/MetricHelper.cs b/src/Common/Commands.Common/MetricHelper.cs index 8772c418129a..eef92a734cb2 100644 --- a/src/Common/Commands.Common/MetricHelper.cs +++ b/src/Common/Commands.Common/MetricHelper.cs @@ -68,7 +68,7 @@ private static string HashMacAddress nic.GetPhysicalAddress() != null && !string.IsNullOrWhiteSpace(nic.GetPhysicalAddress().ToString()))? .GetPhysicalAddress()?.ToString(); - _hashMacAddress = string.IsNullOrWhiteSpace(macAddress) ? null : GenerateSha256HashString(macAddress).Replace("-", string.Empty).ToLowerInvariant(); + _hashMacAddress = string.IsNullOrWhiteSpace(macAddress) ? null : GenerateSha256HashString(macAddress)?.Replace("-", string.Empty)?.ToLowerInvariant(); } return _hashMacAddress; @@ -249,7 +249,7 @@ public static string GenerateSha256HashString(string originInput) { if (string.IsNullOrWhiteSpace(originInput)) { - return null; + return string.Empty; } using (var sha256 = new SHA256CryptoServiceProvider()) diff --git a/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs b/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs index 152ca2c671b9..392b332e22c9 100644 --- a/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs +++ b/src/ResourceManager/Profile/Commands.Profile.Test/TelemetryTests.cs @@ -25,11 +25,11 @@ public class TelemetryTests { [Fact] [Trait(Category.AcceptanceType, Category.CheckIn)] - public void HashOfNullOrWhitespaceValueReturnsNull() + public void HashOfNullOrWhitespaceValueReturnsEmptyString() { - Assert.Null(MetricHelper.GenerateSha256HashString(null)); - Assert.Null(MetricHelper.GenerateSha256HashString(string.Empty)); - Assert.Null(MetricHelper.GenerateSha256HashString(" ")); + Assert.Equal(string.Empty, MetricHelper.GenerateSha256HashString(null)); + Assert.Equal(string.Empty, MetricHelper.GenerateSha256HashString(string.Empty)); + Assert.Equal(string.Empty, MetricHelper.GenerateSha256HashString(" ")); } [Fact] @@ -45,22 +45,12 @@ public void HashOfValidValueSucceeds() [Fact] [Trait(Category.AcceptanceType, Category.CheckIn)] - public void NetworkInterfaceWithEmptyAddressReturnsNull() + public void NetworkInterfaceWithEmptyAddressReturnsEmptyString() { - var networkInterfaceMock = new Mock(); - networkInterfaceMock.Setup(t => t.GetPhysicalAddress()) - .Returns( - () => - { - return new PhysicalAddress(new byte[] { }); - } - ); - - var address = networkInterfaceMock.Object.GetPhysicalAddress(); - Assert.NotNull(address); + var address = new PhysicalAddress(new byte[] { } ); Assert.Equal(string.Empty, address.ToString()); var hashAddress = MetricHelper.GenerateSha256HashString(address.ToString()); - Assert.Null(hashAddress); + Assert.Equal(string.Empty, hashAddress); } } }