CB-11636 Handle attributes with quotes correctly
authorVladimir Kotikov <v-vlkoti@microsoft.com>
Thu, 28 Jul 2016 09:44:59 +0000 (12:44 +0300)
committerSteve Gill <stevengill97@gmail.com>
Tue, 2 Aug 2016 00:25:08 +0000 (17:25 -0700)
Avoid using complex queries in et.findall to prevent
issues w/ quoted values. Previous approach w/ crafting
complex query from attributes names and values was faulty
when any of the attributes had double quotes or &quot;

 This closes #470

spec/util/xml-helpers.spec.js
src/util/xml-helpers.js

index 24c6553..0becf72 100644 (file)
@@ -294,6 +294,7 @@ describe('xml-helpers', function(){
         beforeEach(function() {
             dstXml = et.XML(TEST_XML);
         });
+
         it('should merge attributes and text of the root element without clobbering', function () {
             var testXml = et.XML('<widget foo="bar" id="NOTANID">TEXT</widget>');
             xml_helpers.mergeXml(testXml, dstXml);
@@ -310,6 +311,15 @@ describe('xml-helpers', function(){
             expect(dstXml.text).toEqual('TEXT');
         });
 
+        it('should handle attributes values with quotes correctly', function () {
+            var testXml = et.XML('<widget><quote foo="some \'quoted\' string" bar="another &quot;quoted&quot; string" baz="&quot;mixed&quot; \'quotes\'" /></widget>');
+            xml_helpers.mergeXml(testXml, dstXml);
+            expect(dstXml.find('quote')).toBeDefined();
+            expect(dstXml.find('quote').attrib.foo).toEqual('some \'quoted\' string');
+            expect(dstXml.find('quote').attrib.bar).toEqual('another "quoted" string');
+            expect(dstXml.find('quote').attrib.baz).toEqual('"mixed" \'quotes\'');
+        });
+
         it('should not merge platform tags with the wrong platform', function () {
             var testXml = et.XML('<widget><platform name="bar"><testElement testAttrib="value">testTEXT</testElement></platform></widget>'),
                 origCfg = et.tostring(dstXml);
index f16eaaf..4b630fa 100644 (file)
@@ -44,23 +44,9 @@ module.exports = {
             return false;
         }
 
-        var oneAttribKeys = Object.keys(one.attrib),
-            twoAttribKeys = Object.keys(two.attrib),
-            i = 0, attribName;
+        if (!attribMatch(one, two)) return false;
 
-        if (oneAttribKeys.length != twoAttribKeys.length) {
-            return false;
-        }
-
-        for (i; i < oneAttribKeys.length; i++) {
-            attribName = oneAttribKeys[i];
-
-            if (one.attrib[attribName] != two.attrib[attribName]) {
-                return false;
-            }
-        }
-
-        for (i; i < one._children.length; i++) {
+        for (var i = 0; i < one._children.length; i++) {
             if (!module.exports.equalNodes(one._children[i], two._children[i])) {
                 return false;
             }
@@ -287,33 +273,30 @@ function mergeXml(src, dest, platform, clobber) {
             query = srcTag + '',
             shouldMerge = true;
 
-        if (BLACKLIST.indexOf(srcTag) === -1) {
-            if (SINGLETONS.indexOf(srcTag) !== -1) {
-                foundChild = dest.find(query);
-                if (foundChild) {
-                    destChild = foundChild;
-                    dest.remove(destChild);
-                }
-            } else {
-                //Check for an exact match and if you find one don't add
-                Object.getOwnPropertyNames(srcChild.attrib).forEach(function (attribute) {
-                    query += '[@' + attribute + '="' + srcChild.attrib[attribute] + '"]';
-                });
-                var foundChildren = dest.findall(query);
-                for(var i = 0; i < foundChildren.length; i++) {
-                    foundChild = foundChildren[i];
-                    if (foundChild && textMatch(srcChild, foundChild) && (Object.keys(srcChild.attrib).length==Object.keys(foundChild.attrib).length)) {
-                        destChild = foundChild;
-                        dest.remove(destChild);
-                        shouldMerge = false;
-                        break;
-                    }
-                }
-            }
+        if (BLACKLIST.indexOf(srcTag) !== -1) return;
 
-            mergeXml(srcChild, destChild, platform, clobber && shouldMerge);
-            dest.append(destChild);
+        if (SINGLETONS.indexOf(srcTag) !== -1) {
+            foundChild = dest.find(query);
+            if (foundChild) {
+                destChild = foundChild;
+                dest.remove(destChild);
+            }
+        } else {
+            //Check for an exact match and if you find one don't add
+            var mergeCandidates = dest.findall(query)
+            .filter(function (foundChild) {
+                return foundChild && textMatch(srcChild, foundChild) && attribMatch(srcChild, foundChild);
+            });
+
+            if (mergeCandidates.length > 0) {
+                destChild = mergeCandidates[0];
+                dest.remove(destChild);
+                shouldMerge = false;
+            }
         }
+
+        mergeXml(srcChild, destChild, platform, clobber && shouldMerge);
+        dest.append(destChild);
     }
 
     function removeDuplicatePreferences(xml) {
@@ -345,3 +328,22 @@ function textMatch(elm1, elm2) {
         text2 = elm2.text ? elm2.text.replace(/\s+/, '') : '';
     return (text1 === '' || text1 === text2);
 }
+
+function attribMatch(one, two) {
+    var oneAttribKeys = Object.keys(one.attrib);
+    var twoAttribKeys = Object.keys(two.attrib);
+
+    if (oneAttribKeys.length != twoAttribKeys.length) {
+        return false;
+    }
+
+    for (var i = 0; i < oneAttribKeys.length; i++) {
+        var attribName = oneAttribKeys[i];
+
+        if (one.attrib[attribName] != two.attrib[attribName]) {
+            return false;
+        }
+    }
+
+    return true;
+}