fix(plist): only read direct-child keys when parsing a dict (#295)

getElementsByTagName('key') is recursive, so keys from nested dicts leaked
into the parent object and a nested key could overwrite a parent key of the
same name. Walk direct children instead and skip past each parsed value.

Author: harsha509

src/lib/plist/plist-parser.ts

@@ -111,21 +111,32 @@ export function parsePlist(xmlData: string | Buffer): PlistDictionary {
 
   /**
    * Parse a plist `<dict>` element into a JavaScript object.
+   * Only direct-child `<key>` elements are considered: getElementsByTagName
+   * is recursive and would flatten keys of nested dicts into the parent.
    */
   function parseDict(dictNode: Element): PlistDictionary {
     const obj: PlistDictionary = {};
-    const keys = dictNode.getElementsByTagName('key');
+    let childNode = dictNode.firstChild;
 
-    for (let i = 0; i < keys.length; i++) {
-      const keyName = keys[i].textContent || '';
-      let valueNode = keys[i].nextSibling;
+    while (childNode) {
+      const keyNode = childNode;
+      childNode = childNode.nextSibling;
+
+      if (keyNode.nodeType !== Node.ELEMENT_NODE || keyNode.nodeName !== 'key') {
+        continue;
+      }
+
+      const keyName = keyNode.textContent || '';
+      let valueNode = keyNode.nextSibling;
 
       while (valueNode && valueNode.nodeType !== Node.ELEMENT_NODE) {
         valueNode = valueNode.nextSibling;
       }
 
       if (valueNode) {
         obj[keyName] = parseNode(valueNode as Element);
+        // Skip ahead of the parsed value so the loop doesn't re-visit it
+        childNode = valueNode.nextSibling;
       }
     }
 

test/unit/plist/plist-parser.spec.ts

@@ -133,6 +133,71 @@ describe('Plist Parser', function () {
 
       const level2 = level1.level2 as PlistDictionary;
       assert.strictEqual(level2.level3, 'deep value');
+
+      // Nested keys must not leak into ancestor dicts
+      assert.deepStrictEqual(Object.keys(result), ['level1']);
+      assert.deepStrictEqual(Object.keys(level1), ['level2']);
+      assert.deepStrictEqual(Object.keys(level2), ['level3']);
+    });
+
+    it('should not let a nested key overwrite an outer key with the same name', function () {
+      const xml = `
+        <?xml version="1.0" encoding="UTF-8"?>
+        <plist version="1.0">
+        <dict>
+          <key>Result</key>
+          <string>ok</string>
+          <key>Nested</key>
+          <dict>
+            <key>Result</key>
+            <integer>1</integer>
+            <key>Inner</key>
+            <string>x</string>
+          </dict>
+          <key>Name</key>
+          <string>dev</string>
+        </dict>
+        </plist>
+      `;
+
+      const result = parseXmlPlist(xml);
+      assert.strictEqual(result.Result, 'ok');
+      assert.strictEqual(result.Name, 'dev');
+      assert.ok(!('Inner' in result));
+
+      const nested = result.Nested as PlistDictionary;
+      assert.strictEqual(nested.Result, 1);
+      assert.strictEqual(nested.Inner, 'x');
+    });
+
+    it('should not leak keys from dicts nested inside arrays', function () {
+      const xml = `
+        <?xml version="1.0" encoding="UTF-8"?>
+        <plist version="1.0">
+        <dict>
+          <key>Status</key>
+          <string>Complete</string>
+          <key>Items</key>
+          <array>
+            <dict>
+              <key>Status</key>
+              <string>Pending</string>
+              <key>Id</key>
+              <integer>7</integer>
+            </dict>
+          </array>
+        </dict>
+        </plist>
+      `;
+
+      const result = parseXmlPlist(xml);
+      assert.strictEqual(result.Status, 'Complete');
+      assert.deepStrictEqual(Object.keys(result), ['Status', 'Items']);
+
+      const items = result.Items as PlistArray;
+      const item = items[0] as PlistDictionary;
+      assert.strictEqual(item.Status, 'Pending');
+      assert.strictEqual(item.Id, 7);
     });
 
     it('should parse mixed arrays and dictionaries', function () {