From 82ce72e49a20bdd4feec417c2f7c021af8fc55c4 Mon Sep 17 00:00:00 2001 From: Varun Puranik Date: Fri, 4 Jan 2019 12:17:54 -0800 Subject: [PATCH] Handle clients with special characters in their names (#682) --- .../CbsNode.cs | 4 +- .../linkhandlers/LinkHandlerProvider.cs | 6 +- .../EdgeHubConnection.cs | 2 +- .../TwinManager.cs | 30 +++++++++ .../CbsNodeTest.cs | 8 +++ .../LinkHandlerProviderTest.cs | 64 +++++++++++++++++++ .../TwinManagerTest.cs | 19 ++++++ 7 files changed, 129 insertions(+), 4 deletions(-) diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/CbsNode.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/CbsNode.cs index a1893654db4..f3d97193ee3 100644 --- a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/CbsNode.cs +++ b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/CbsNode.cs @@ -5,6 +5,7 @@ namespace Microsoft.Azure.Devices.Edge.Hub.Amqp using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; + using System.Net; using System.Threading; using System.Threading.Tasks; using Microsoft.Azure.Amqp; @@ -243,7 +244,8 @@ static AmqpMessage GetAmqpResponse(AmqpMessage requestMessage, AmqpResponseStatu internal static (string deviceId, string moduleId) ParseIds(string audience) { - string audienceUri = audience.StartsWith("amqps://", StringComparison.CurrentCultureIgnoreCase) ? audience : "amqps://" + audience; + string decodedAudience = WebUtility.UrlDecode(audience); + string audienceUri = decodedAudience.StartsWith("amqps://", StringComparison.CurrentCultureIgnoreCase) ? decodedAudience : "amqps://" + decodedAudience; foreach (UriPathTemplate template in ResourceTemplates) { diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/linkhandlers/LinkHandlerProvider.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/linkhandlers/LinkHandlerProvider.cs index db3c41e8238..4c581ff8651 100644 --- a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/linkhandlers/LinkHandlerProvider.cs +++ b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Amqp/linkhandlers/LinkHandlerProvider.cs @@ -3,6 +3,7 @@ namespace Microsoft.Azure.Devices.Edge.Hub.Amqp.LinkHandlers { using System; using System.Collections.Generic; + using System.Net; using Microsoft.Azure.Amqp; using Microsoft.Azure.Devices.Edge.Hub.Core; using Microsoft.Azure.Devices.Edge.Hub.Core.Identity; @@ -118,15 +119,16 @@ IConnectionHandler GetConnectionHandler(IAmqpLink link, IIdentity identity) return amqpClientConnectionsHandler.GetConnectionHandler(identity); } - IIdentity GetIdentity(IDictionary boundVariables) + internal IIdentity GetIdentity(IDictionary boundVariables) { if (!boundVariables.TryGetValue(Templates.DeviceIdTemplateParameterName, out string deviceId)) { throw new InvalidOperationException("Link should contain a device Id"); } + deviceId = WebUtility.UrlDecode(deviceId); return boundVariables.TryGetValue(Templates.ModuleIdTemplateParameterName, out string moduleId) - ? this.identityProvider.Create(deviceId, moduleId) + ? this.identityProvider.Create(deviceId, WebUtility.UrlDecode(moduleId)) : this.identityProvider.Create(deviceId); } diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/EdgeHubConnection.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/EdgeHubConnection.cs index 76496cba18c..bf840ae33ef 100644 --- a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/EdgeHubConnection.cs +++ b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/EdgeHubConnection.cs @@ -312,7 +312,7 @@ DeviceConnectionStatus GetDeviceConnectionStatus() var connectedDevices = new Dictionary { - [client.Id] = GetDeviceConnectionStatus() + [TwinManager.EncodeTwinKey(client.Id)] = GetDeviceConnectionStatus() }; var edgeHubReportedProperties = new ReportedProperties(this.versionInfo, connectedDevices); var twinCollection = new TwinCollection(JsonConvert.SerializeObject(edgeHubReportedProperties)); diff --git a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs index a8e2466a495..5ef6fa167bb 100644 --- a/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs +++ b/edge-hub/src/Microsoft.Azure.Devices.Edge.Hub.Core/TwinManager.cs @@ -604,6 +604,36 @@ static void ValidateTwinProperties(JToken properties, int currentDepth) } } + // TODO: Move to a Twin helper class (along with Twin manager update). + internal static string EncodeTwinKey(string key) + { + Preconditions.CheckNonWhiteSpace(key, nameof(key)); + var sb = new StringBuilder(); + foreach (char ch in key) + { + switch (ch) + { + case '.': + sb.Append("%2E"); + break; + + case '$': + sb.Append("%24"); + break; + + case ' ': + sb.Append("%20"); + break; + + default: + sb.Append(ch); + break; + } + } + + return sb.ToString(); + } + static class Events { static readonly ILogger Log = Logger.Factory.CreateLogger(); diff --git a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/CbsNodeTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/CbsNodeTest.cs index 2680e623f67..8a2c6af1395 100644 --- a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/CbsNodeTest.cs +++ b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/CbsNodeTest.cs @@ -124,6 +124,14 @@ public void ParseIdsTest() audience = "edgehubtest1.azure-devices.net/device/device1/module/mod1"; // Act/Assert Assert.Throws(() => CbsNode.ParseIds(audience)); + + // Arrange + audience = "edgehubtest1.azure-devices.net/devices/d%40n.f/modules/m%40n.p"; + // Act + (deviceId, moduleId) = CbsNode.ParseIds(audience); + // Assert + Assert.Equal("d@n.f", deviceId); + Assert.Equal("m@n.p", moduleId); } [Fact] diff --git a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/LinkHandlerProviderTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/LinkHandlerProviderTest.cs index af5660d6611..4188f493a9e 100644 --- a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/LinkHandlerProviderTest.cs +++ b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Amqp.Test/LinkHandlerProviderTest.cs @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Devices.Edge.Hub.Amqp.Test using Microsoft.Azure.Amqp; using Microsoft.Azure.Devices.Edge.Hub.Amqp.LinkHandlers; using Microsoft.Azure.Devices.Edge.Hub.Core; + using Microsoft.Azure.Devices.Edge.Hub.Core.Device; using Microsoft.Azure.Devices.Edge.Hub.Core.Identity; using Microsoft.Azure.Devices.Edge.Util.Test.Common; using Moq; @@ -174,5 +175,68 @@ public void GetLinkHandlerTest(string url, bool isReceiver, Type expectedLinkHan Assert.NotNull(linkHandler); Assert.IsType(expectedLinkHandlerType, linkHandler); } + + [Theory] + [MemberData(nameof(GetIdentityFromBoundVariablesTestData))] + public void GetIdentityFromBoundVariablesTest(IDictionary boundVariables, string expectedDeviceId, string expectedModuleId) + { + // Arrange + var messageConverter = Mock.Of>(); + var twinMessageConverter = Mock.Of>(); + var methodMessageConverter = Mock.Of>(); + var identityProvider = new IdentityProvider("foo.azure-device.net"); + var linkHandlerProvider = new LinkHandlerProvider(messageConverter, twinMessageConverter, methodMessageConverter, identityProvider); + + // Act + IIdentity identity = linkHandlerProvider.GetIdentity(boundVariables); + + // Assert + if (string.IsNullOrEmpty(expectedModuleId)) + { + Assert.IsType(identity); + Assert.Equal(expectedDeviceId, identity.Id); + } + else + { + Assert.IsType(identity); + Assert.Equal(expectedDeviceId, ((ModuleIdentity)identity).DeviceId); + Assert.Equal(expectedModuleId, ((ModuleIdentity)identity).ModuleId); + } + } + + static IEnumerable GetIdentityFromBoundVariablesTestData() + { + yield return new object[] + { + new Dictionary + { + ["deviceid"] = "d1" + }, + "d1", + string.Empty + }; + + yield return new object[] + { + new Dictionary + { + ["deviceid"] = "d1", + ["moduleid"] = "m1" + }, + "d1", + "m1" + }; + + yield return new object[] + { + new Dictionary + { + ["deviceid"] = "d%40n.f", + ["moduleid"] = "m%40n.p" + }, + "d@n.f", + "m@n.p" + }; + } } } diff --git a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs index a46e4f60e1a..40e11380d30 100644 --- a/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs +++ b/edge-hub/test/Microsoft.Azure.Devices.Edge.Hub.Core.Test/TwinManagerTest.cs @@ -1325,5 +1325,24 @@ public void ValidateTwinPropertiesSuccess() Assert.Throws(() => TwinManager.ValidateTwinProperties(JToken.FromObject(reported6))); } + + [Theory] + [MemberData(nameof(GetTwinKeyData))] + public void EncodeTwinKeyTest(string input, string expectedResult) + { + string result = TwinManager.EncodeTwinKey(input); + Assert.Equal(expectedResult, result); + } + + static IEnumerable GetTwinKeyData() + { + yield return new object[] { "key1", "key1" }; + + yield return new object[] { "123", "123" }; + + yield return new object[] { "a.b$c d", "a%2Eb%24c%20d" }; + + yield return new object[] { "a.b.c.d", "a%2Eb%2Ec%2Ed" }; + } } }