Merge pull request #28 from buglabs/html-sanitizer

Adds basic sanitization and re-rewrites coercion and trimming

Author: c4milo

lib/xml2json.js

@@ -12,12 +12,11 @@ function startElement(name, attrs) {
     currentElementName = name;
     if(options.coerce) {
         // Looping here in stead of making coerce generic as object walk is unnecessary
-        for (var key in attrs) {
-            if (attrs.hasOwnProperty(key)) {
-                attrs[key] = coerce(attrs[key]);
-            }
-        }
+        Object.keys(attrs).forEach(function(key) {
+            attrs[key] = coerce(attrs[key]);
+        });
     }
+
     if (! (name in currentObject)) {
         currentObject[name] = attrs;
     } else if (! (currentObject[name] instanceof Array)) {
@@ -46,17 +45,24 @@ function startElement(name, attrs) {
 }
 
 function text(data) {
-    if (!options.space) {
+    //console.log('->' + data + '<-');
+    /*if (!data.trim().length) {
+        return;
+    }*/
+    
+    if (options.trim) {
         data = data.trim();
-        if (!data.length) {
-            return;
-        }
     }
-    currentObject['$t'] = coerce((currentObject['$t'] || "") + data);
+
+    if (options.sanitize) {
+        data = sanitize(data);
+    }
+
+    currentObject['$t'] = coerce((currentObject['$t'] || '') + data);
 }
 
 function endElement(name) {
-    if (options.space && currentElementName !== name) {
+    if (currentElementName !== name) {
         delete currentObject['$t'];
     }
     // This should check to make sure that the name we're ending 
@@ -75,25 +81,80 @@ function endElement(name) {
     currentObject = ancestor;
 }
 
-function coerce(val) {
+function coerce(value) {
     if (!options.coerce) {
-        return val;
+        return value;
     }
-    var num = Number(val);
+    
+    var num = Number(value);
     if (!isNaN(num)) {
         return num;
     }
-    switch (val.toLowerCase()){
-        case 'true':
-        case 'yes':
-            return true;
-        case 'false':
-        case 'no':
-            return false;
-        default: return val;
+
+    var _value = value.toLowerCase();
+    
+    if (_value == 'true' || _value == 'yes') {
+        return true;
+    }
+
+    if (_value == 'false' || _value == 'no') {
+        return false;
     }
+
+    return value;
 }
 
+
+/**
+ * Simple sanitization. It is not intended to sanitize 
+ * malicious element values.
+ *
+ * character | escaped
+ *      <       &lt;
+ *      >       &gt;
+ *      (       &#40;
+ *      )       &#41;
+ *      #       &#35;
+ *      &       &amp;
+ *      "       &quot;
+ *      '       &apos;
+ */
+var chars =  {  '<': '&lt;',
+                '>': '&gt;',
+                '(': '&#40;',
+                ')': '&#41;',
+                '#': '&#35;',
+                '&': '&amp;',
+                '"': '&quot;',
+                "'": '&apos;' };
+
+function sanitize(value) {
+    if (typeof value !== 'string') {
+        return value;
+    }
+
+    Object.keys(chars).forEach(function(key) {
+        value = value.replace(key, chars[key]);
+    });
+
+    return value;
+}
+
+/**
+ * Parses xml to json using node-expat.
+ * @param {String|Buffer} xml The xml to be parsed to json.
+ * @param {Object} _options An object with options provided by the user.
+ * The available options are:
+ *  - object: If true, the parser returns a Javascript object instead of
+ *            a JSON string.
+ *  - reversible: If true, the parser generates a reversible JSON, mainly
+ *                characterized by the presence of the property $t.
+ *  - sanitize_values: If true, the parser escapes any element value in the xml
+ * that has any of the following characters: <, >, (, ), #, #, &, ", '.
+ * 
+ * @return {String|Object} A String or an Object with the JSON representation
+ * of the XML.
+ */
 module.exports = function(xml, _options) {
     var parser = new expat.Parser('UTF-8');
 
@@ -108,7 +169,9 @@ module.exports = function(xml, _options) {
     options = {
         object: false,
         reversible: false,
-        space: false, // keep space or not
+        coerce: true,
+        sanitize: true,
+        trim: true
     };
 
     for (var opt in _options) {
@@ -123,6 +186,11 @@ module.exports = function(xml, _options) {
         return obj;
     }
 
-    return JSON.stringify(obj);
+    var json = JSON.stringify(obj);
+
+    //See: http://timelessrepo.com/json-isnt-a-javascript-subset
+    json = json.replace(/\u2028/g, '\\u2028').replace(/\u2029/g, '\\u2029');
+    
+    return json;
 };
 

test/.gitignore

@@ -0,0 +1 @@
+*.DS_Store

test/test-coerce.js

@@ -8,13 +8,14 @@ var data = fs.readFileSync(file);
 
 // With coercion
 var result = parser.toJson(data, {reversible: true, coerce: true, object: true});
+console.log(result.itemRecord.value);
 assert.strictEqual(result.itemRecord.value[0].longValue['$t'], 12345);
 assert.strictEqual(result.itemRecord.value[1].stringValue.number, false);
 assert.strictEqual(result.itemRecord.value[2].moneyValue.number, true);
 assert.strictEqual(result.itemRecord.value[2].moneyValue['$t'], 104.95);
 
 // Without coercion
-result = parser.toJson(data, {reversible: true, object: true});
+result = parser.toJson(data, {reversible: true, coerce: false, object: true});
 assert.strictEqual(result.itemRecord.value[0].longValue['$t'], '12345');
 assert.strictEqual(result.itemRecord.value[1].stringValue.number, 'false');
 assert.strictEqual(result.itemRecord.value[2].moneyValue.number, 'true');

test/test.js

@@ -18,9 +18,15 @@ fs.readdir(fixturesPath, function(err, files) {
 
             var  data2 =  fs.readFileSync(fixturesPath + '/' + file);
             if (file.indexOf('spacetext') >= 0) {
-                result = parser.toJson(data2, {space: true});
+                result = parser.toJson(data2, {trim: false, coerce: false});
+            } else if (file.indexOf('coerce') >= 0) {
+                result = parser.toJson(data2, {coerce: false});
+            } else if (file.indexOf('domain') >= 0) {
+                result = parser.toJson(data2, {coerce: false});
+            } else if (file.indexOf('large') >= 0) {
+                result = parser.toJson(data2, {coerce: false, trim: true, sanitize: false});
             } else {
-                result = parser.toJson(data2);
+                result = parser.toJson(data2, {trim: false});
             }
 
             var jsonFile = basename + '.json';
@@ -29,6 +35,9 @@ fs.readdir(fixturesPath, function(err, files) {
             if (expected) {
                 expected = expected.trim();
             }
+            /*console.log(result);
+            console.log('============ Expected ===============');
+            console.log(expected)*/
             assert.deepEqual(result, expected, jsonFile + ' and ' + file + ' are different');
             console.log('[xml2json: ' + file + '->' + jsonFile + '] passed!');
         } else if( ext == '.json') {