Handle clients with special characters in their names (#682)

Azure · Jan 4, 2019 · 82ce72e · 82ce72e
1 parent 0dac12c
commit 82ce72e
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 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
@@ -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
@@ -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<string, string> boundVariables)
+        internal IIdentity GetIdentity(IDictionary<string, string> 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
@@ -312,7 +312,7 @@ DeviceConnectionStatus GetDeviceConnectionStatus()
 
                 var connectedDevices = new Dictionary<string, DeviceConnectionStatus>
                 {
-                    [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
@@ -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<TwinManager>();

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
@@ -124,6 +124,14 @@ public void ParseIdsTest()
             audience = "edgehubtest1.azure-devices.net/device/device1/module/mod1";
             // Act/Assert
             Assert.Throws<InvalidOperationException>(() => 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
@@ -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<string, string> boundVariables, string expectedDeviceId, string expectedModuleId)
+        {
+            // Arrange
+            var messageConverter = Mock.Of<IMessageConverter<AmqpMessage>>();
+            var twinMessageConverter = Mock.Of<IMessageConverter<AmqpMessage>>();
+            var methodMessageConverter = Mock.Of<IMessageConverter<AmqpMessage>>();
+            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<DeviceIdentity>(identity);
+                Assert.Equal(expectedDeviceId, identity.Id);
+            }
+            else
+            {
+                Assert.IsType<ModuleIdentity>(identity);
+                Assert.Equal(expectedDeviceId, ((ModuleIdentity)identity).DeviceId);
+                Assert.Equal(expectedModuleId, ((ModuleIdentity)identity).ModuleId);
+            }
+        }
+
+        static IEnumerable<object[]> GetIdentityFromBoundVariablesTestData()
+        {
+            yield return new object[]
+            {
+                new Dictionary<string, string>
+                {
+                    ["deviceid"] = "d1"
+                },
+                "d1",
+                string.Empty
+            };
+
+            yield return new object[]
+            {
+                new Dictionary<string, string>
+                {
+                    ["deviceid"] = "d1",
+                    ["moduleid"] = "m1"
+                },
+                "d1",
+                "m1"
+            };
+
+            yield return new object[]
+            {
+                new Dictionary<string, string>
+                {
+                    ["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
@@ -1325,5 +1325,24 @@ public void ValidateTwinPropertiesSuccess()
 
             Assert.Throws<InvalidOperationException>(() => 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<object[]> 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" };
+        }
     }
 }