Dropdown Value Xml Escaping and unescaping Exception (#9447)

* DropdownValueXmlParsing * Use .Net methods for escape and unescape * Add Unit Tests * Update to use WebUtility.HtmlDecode and add unit tests * Address Comments
DynamoDS · Feb 15, 2019 · 66339d9 · 66339d9
1 parent 055de9d
commit 66339d9
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 9 deletions.
diff --git a/src/Libraries/CoreNodeModels/DropDown.cs b/src/Libraries/CoreNodeModels/DropDown.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
+using System.Security;
 using System.Xml;
 using CoreNodeModels.Properties;
 using Dynamo.Graph;
@@ -164,7 +165,7 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
             }
             else
             {
-                selectedString = selectedIndex > Items.Count - 1? String.Empty: GetSelectedStringFromItem(Items.ElementAt(selectedIndex));
+                selectedString = selectedIndex > Items.Count - 1 ? String.Empty : GetSelectedStringFromItem(Items.ElementAt(selectedIndex));
             }
         }
 
@@ -238,20 +239,30 @@ public static string SaveSelectedIndexImpl(int index, IList<DynamoDropDownItem>
             return string.Format("{0}:{1}", index, XmlEscape(item.Name));
         }
 
+        /// <summary>
+        /// Escape string which could contain XML forbidden chars.
+        /// </summary>
+        /// <param name="unescaped"></param>
+        /// <returns></returns>
         protected static string XmlEscape(string unescaped)
         {
-            var doc = new XmlDocument();
-            XmlNode node = doc.CreateElement("root");
-            node.InnerText = unescaped;
-            return node.InnerXml;
+            // TODO: This function can be simplified in Dynamo 3.0
+            // since it is one line wrapper
+            return SecurityElement.Escape(unescaped);
         }
 
+        /// <summary>
+        /// Unescape string which could already be escaped before,
+        /// if there is no escaped special char, return as it is
+        /// if there is special char escaped, restore them.
+        /// </summary>
+        /// <param name="escaped"></param>
+        /// <returns></returns>
         protected static string XmlUnescape(string escaped)
         {
-            var doc = new XmlDocument();
-            XmlNode node = doc.CreateElement("root");
-            node.InnerXml = escaped;
-            return node.InnerText;
+            // TODO: This function can be simplified in Dynamo 3.0
+            // since it is one line wrapper
+            return System.Web.HttpUtility.HtmlDecode(escaped);
         }
 
         /// <summary>

diff --git a/test/DynamoCoreTests/Nodes/DropDownTests.cs b/test/DynamoCoreTests/Nodes/DropDownTests.cs
@@ -266,6 +266,16 @@ public void Save_SelectedIndex()
                 DSDropDownBase.SaveSelectedIndexImpl(2, TestList()));
         }
 
+        [Test]
+        public void Save_SelectedIndexWithSpecialChar()
+        {
+            // This test makes sure that when SaveSelectedIndexImpl() is called with a index
+            // of item with Xml special char in the item.Name, the special char is escaped
+            Assert.AreEqual(
+                "2:&lt;foo&gt;",
+                DSDropDownBase.SaveSelectedIndexImpl(2, TestListWithItemWithSpecialChar()));
+        }
+
         [Test]
         public void Load_NothingSelected()
         {
@@ -296,6 +306,24 @@ public void Load_SelectionIndexNoNameMatch()
             Assert.AreEqual(-1, DSDropDownBase.ParseSelectedIndexImpl("2:foo", TestList()));
         }
 
+        [Test]
+        public void Load_SelectionIndexSpecialCharNoMatch()
+        {
+            Assert.AreEqual(-1, DSDropDownBase.ParseSelectedIndexImpl("2:<foo>", TestList()));
+        }
+
+        [Test]
+        public void Load_SelectionIndexSpecialCharMatch()
+        {
+            Assert.AreEqual(2, DSDropDownBase.ParseSelectedIndexImpl("2:<foo>", TestListWithItemWithSpecialChar()));
+        }
+
+        [Test]
+        public void Load_SelectionIndexSpecialCharMatch2()
+        {
+            Assert.AreEqual(2, DSDropDownBase.ParseSelectedIndexImpl("2:&lt;foo&gt;", TestListWithItemWithSpecialChar()));
+        }
+
         private static List<DynamoDropDownItem> TestList()
         {
             var items = new List<DynamoDropDownItem>
@@ -309,5 +337,17 @@ private static List<DynamoDropDownItem> TestList()
             return items;
         }
 
+        private static List<DynamoDropDownItem> TestListWithItemWithSpecialChar()
+        {
+            var items = new List<DynamoDropDownItem>
+            {
+                new DynamoDropDownItem("cat", "cat"),
+                new DynamoDropDownItem("dog", "dog"),
+                new DynamoDropDownItem("<foo>", "<foo>"),
+                new DynamoDropDownItem("!@#$%%%^&*()", "craziness")
+            };
+
+            return items;
+        }
     }
 }