Skip to content

Commit

Permalink
Harden test-cases and implementation to align with other implementations
Browse files Browse the repository at this point in the history
This commit refactors and adds more tests to the `parse()` implementation
to align with similar libraries's behaviour, e.g.:
- https://github.com/Microsoft/node-fast-plist
- https://docs.python.org/3/library/plistlib.html

With the new tests a few of the tests were failing:
```
31 passing (177ms)
10 failing

1) parse() real should throw an Error when parsing an empty real:
    AssertionError: Missing expected exception..

2) parse() string should parse a self closing string:
    AssertionError: null === ''

3) parse() string should parse an empty string:
    AssertionError: null === ''

4) parse() string should parse a string with comments:
    AssertionError: 'a comment  string' === 'a string'

5) parse() array should parse empty elements inside an array:
    AssertionError: [ false ] deepEqual [ '', false ]

6) parse() dict should throw if key is missing:
    AssertionError: Missing expected exception..

7) parse() dict should throw if two keys follow each other:
    AssertionError: Missing expected exception..

8) parse() dict should throw if value is missing:
    AssertionError: Missing expected exception..

9) parse() dict should parse an empty key:
    AssertionError: {} deepEqual { '': '1' }

10) parse() dict should parse an empty value:
    AssertionError: { a: null } deepEqual { a: '' }
```

This commit also refactors the `lib/parse.js` to pass these tests.
When executing the new implementation of `lib/parse.js` agains the old
tests the following tests were failing:
```
25 passing (143ms)
3 failing

1) plist parse() should parse an empty <key/> in a dictionary:
    AssertionError: false == true

2) plist parse() should parse an empty <key></key> in a dictionary:
    AssertionError: false == true

3) plist parse() should parse an empty <key></key> and <string></string> in dictionary with more data:
    AssertionError: { '': '', UIRequiredDeviceCapabilities: [ 'armv7' ] } deepEqual { UIRequiredDeviceCapabilities: [ 'armv7' ] }
```

As the output above shows, most of the issue are with the handling of
empty strings and empty dictionary keys.
Added with the new implementation are custom Errors for unexpected
input, better handling of comments inside tags and a more aligned
handling of empty keys/strings.
  • Loading branch information
ZauberNerd committed Feb 12, 2017
1 parent c71d9f7 commit 85d11c4
Show file tree
Hide file tree
Showing 2 changed files with 229 additions and 199 deletions.
101 changes: 69 additions & 32 deletions lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ function isEmptyNode(node){
}
}

function invariant(test, message) {
if (!test) {
throw new Error(message);
}
}

/**
* Parses a Plist XML string. Returns an Object.
*
Expand All @@ -53,9 +59,10 @@ function isEmptyNode(node){

function parse (xml) {
var doc = new DOMParser().parseFromString(xml);
if (doc.documentElement.nodeName !== 'plist') {
throw new Error('malformed document. First element should be <plist>');
}
invariant(
doc.documentElement.nodeName === 'plist',
'malformed document. First element should be <plist>'
);
var plist = parsePlistXML(doc.documentElement);

// the root <plist> node gets interpreted as an Array,
Expand All @@ -74,44 +81,60 @@ function parse (xml) {
*/

function parsePlistXML (node) {
var i, new_obj, key, val, new_arr, res, d;
var i, new_obj, key, val, new_arr, res, counter, type;

if (!node)
return null;

if (node.nodeName === 'plist') {
new_arr = [];
for (i=0; i < node.childNodes.length; i++) {
// ignore comment nodes (text)
if (!shouldIgnoreNode(node.childNodes[i])) {
new_arr.push( parsePlistXML(node.childNodes[i]));
if (!isEmptyNode(node)) {
for (i=0; i < node.childNodes.length; i++) {
if (!shouldIgnoreNode(node.childNodes[i])) {
new_arr.push( parsePlistXML(node.childNodes[i]));
}
}
}
return new_arr;

} else if (node.nodeName === 'dict') {
new_obj = {};
key = null;
for (i=0; i < node.childNodes.length; i++) {
// ignore comment nodes (text)
if (!shouldIgnoreNode(node.childNodes[i])) {
if (key === null) {
counter = 0;
if (!isEmptyNode(node)) {
for (i=0; i < node.childNodes.length; i++) {
if (shouldIgnoreNode(node.childNodes[i])) continue;
if (counter % 2 === 0) {
invariant(
node.childNodes[i].nodeName === 'key',
'Missing key while parsing <dict/>.'
);
key = parsePlistXML(node.childNodes[i]);
} else {
invariant(
node.childNodes[i].nodeName !== 'key',
'Unexpected key "'
+ parsePlistXML(node.childNodes[i])
+ '" while parsing <dict/>.'
);
new_obj[key] = parsePlistXML(node.childNodes[i]);
key = null;
}
counter += 1;
}
}
if (counter % 2 === 1) {
throw new Error('Missing value for "' + key + '" while parsing <dict/>');
}
return new_obj;

} else if (node.nodeName === 'array') {
new_arr = [];
for (i=0; i < node.childNodes.length; i++) {
// ignore comment nodes (text)
if (!shouldIgnoreNode(node.childNodes[i])) {
res = parsePlistXML(node.childNodes[i]);
if (null != res) new_arr.push(res);
if (!isEmptyNode(node)) {
for (i=0; i < node.childNodes.length; i++) {
if (!shouldIgnoreNode(node.childNodes[i])) {
res = parsePlistXML(node.childNodes[i]);
if (null != res) new_arr.push(res);
}
}
}
return new_arr;
Expand All @@ -120,45 +143,59 @@ function parsePlistXML (node) {
// TODO: what should we do with text types? (CDATA sections)

} else if (node.nodeName === 'key') {
if(isEmptyNode(node)) return null;
res = '';
if(node.childNodes[0]) {
if(!isEmptyNode(node)) {
res = node.childNodes[0].nodeValue;
}
return res;
} else if (node.nodeName === 'string') {
res = '';
if(isEmptyNode(node)) return null;
for (d=0; d < node.childNodes.length; d++) {
res += node.childNodes[d].nodeValue;
if(!isEmptyNode(node)) {
for (i=0; i < node.childNodes.length; i++) {
var type = node.childNodes[i].nodeType;
if (type === 3 || type === 4) {
res += node.childNodes[i].nodeValue;
}
}
}
return res;

} else if (node.nodeName === 'integer') {
// parse as base 10 integer
invariant(
!isEmptyNode(node),
'Cannot parse "" as integer.'
);
return parseInt(node.childNodes[0].nodeValue, 10);

} else if (node.nodeName === 'real') {
invariant(
!isEmptyNode(node),
'Cannot parse "" as real.'
);
res = '';
for (d=0; d < node.childNodes.length; d++) {
if (node.childNodes[d].nodeType === 3) {
res += node.childNodes[d].nodeValue;
for (i=0; i < node.childNodes.length; i++) {
if (node.childNodes[i].nodeType === 3) {
res += node.childNodes[i].nodeValue;
}
}
return parseFloat(res);

} else if (node.nodeName === 'data') {
res = '';
for (d=0; d < node.childNodes.length; d++) {
if (node.childNodes[d].nodeType === 3) {
res += node.childNodes[d].nodeValue.replace(/\s+/g, '');
if (!isEmptyNode(node)) {
for (i=0; i < node.childNodes.length; i++) {
if (node.childNodes[i].nodeType === 3) {
res += node.childNodes[i].nodeValue.replace(/\s+/g, '');
}
}
}

// decode base64 data to a Buffer instance
return new Buffer(res, 'base64');

} else if (node.nodeName === 'date') {
invariant(
!isEmptyNode(node),
'Cannot parse "" as Date.'
)
return new Date(node.childNodes[0].nodeValue);

} else if (node.nodeName === 'true') {
Expand Down
Loading

0 comments on commit 85d11c4

Please sign in to comment.