Fix: Escape control characters in XML. (fixes #6673) (#6672)

When generating XML report with control characters (like "\b" or "\n", they are sometimes used in JavaScript object keys and will cause messages.), some XML parser like SAX will crash. Escape these characters will avoid crashing.
eslint · Jul 16, 2016 · 9f96086 · 9f96086
1 parent 67c3cc2
commit 9f96086
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 37 deletions.
diff --git a/lib/formatters/checkstyle.js b/lib/formatters/checkstyle.js
@@ -4,6 +4,8 @@
  */
 "use strict";
 
+var xmlEscape = require("../util/xml-escape");
+
 //------------------------------------------------------------------------------
 // Helper Functions
 //------------------------------------------------------------------------------
@@ -22,31 +24,6 @@ function getMessageType(message) {
     }
 }
 
-/**
- * Returns the escaped value for a character
- * @param {string} s string to examine
- * @returns {string} severity level
- * @private
- */
-function xmlEscape(s) {
-    return ("" + s).replace(/[<>&"']/g, function(c) {
-        switch (c) {
-            case "<":
-                return "&lt;";
-            case ">":
-                return "&gt;";
-            case "&":
-                return "&amp;";
-            case "\"":
-                return "&quot;";
-            case "'":
-                return "&apos;";
-            default:
-                throw new Error("unreachable");
-        }
-    });
-}
-
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------

diff --git a/lib/formatters/jslint-xml.js b/lib/formatters/jslint-xml.js
@@ -4,7 +4,7 @@
  */
 "use strict";
 
-var lodash = require("lodash");
+var xmlEscape = require("../util/xml-escape");
 
 //------------------------------------------------------------------------------
 // Public Interface
@@ -25,8 +25,8 @@ module.exports = function(results) {
         messages.forEach(function(message) {
             output += "<issue line=\"" + message.line + "\" " +
                 "char=\"" + message.column + "\" " +
-                "evidence=\"" + lodash.escape(message.source || "") + "\" " +
-                "reason=\"" + lodash.escape(message.message || "") +
+                "evidence=\"" + xmlEscape(message.source || "") + "\" " +
+                "reason=\"" + xmlEscape(message.message || "") +
                 (message.ruleId ? " (" + message.ruleId + ")" : "") + "\" />";
         });
 

diff --git a/lib/formatters/junit.js b/lib/formatters/junit.js
@@ -4,7 +4,7 @@
  */
 "use strict";
 
-var lodash = require("lodash");
+var xmlEscape = require("../util/xml-escape");
 
 //------------------------------------------------------------------------------
 // Helper Functions
@@ -47,11 +47,11 @@ module.exports = function(results) {
             var type = message.fatal ? "error" : "failure";
 
             output += "<testcase time=\"0\" name=\"org.eslint." + (message.ruleId || "unknown") + "\">";
-            output += "<" + type + " message=\"" + lodash.escape(message.message || "") + "\">";
+            output += "<" + type + " message=\"" + xmlEscape(message.message || "") + "\">";
             output += "<![CDATA[";
             output += "line " + (message.line || 0) + ", col ";
             output += (message.column || 0) + ", " + getMessageType(message);
-            output += " - " + lodash.escape(message.message || "");
+            output += " - " + xmlEscape(message.message || "");
             output += (message.ruleId ? " (" + message.ruleId + ")" : "");
             output += "]]>";
             output += "</" + type + ">";

diff --git a/lib/util/xml-escape.js b/lib/util/xml-escape.js
@@ -0,0 +1,34 @@
+/**
+ * @fileoverview XML character escaper
+ * @author George Chung
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Public Interface
+//------------------------------------------------------------------------------
+
+/**
+ * Returns the escaped value for a character
+ * @param {string} s string to examine
+ * @returns {string} severity level
+ * @private
+ */
+module.exports = function(s) {
+    return ("" + s).replace(/[<>&"'\x00-\x1F\x7F\u0080-\uFFFF]/g, function(c) { // eslint-disable-line no-control-regex
+        switch (c) {
+            case "<":
+                return "&lt;";
+            case ">":
+                return "&gt;";
+            case "&":
+                return "&amp;";
+            case "\"":
+                return "&quot;";
+            case "'":
+                return "&apos;";
+            default:
+                return "&#" + c.charCodeAt(0) + ";";
+        }
+    });
+};
diff --git a/tests/lib/formatters/checkstyle.js b/tests/lib/formatters/checkstyle.js
@@ -48,7 +48,7 @@ describe("formatter:checkstyle", function() {
             filePath: "<>&\"'.js",
             messages: [{
                 fatal: true,
-                message: "Unexpected <>&\"'.",
+                message: "Unexpected <>&\"'\b\t\n\f\r牛逼.",
                 line: "<",
                 column: ">",
                 ruleId: "foo"
@@ -58,7 +58,7 @@ describe("formatter:checkstyle", function() {
         it("should return a string in the format filename: line x, col y, Error - z", function() {
             var result = formatter(code);
 
-            assert.equal(result, "<?xml version=\"1.0\" encoding=\"utf-8\"?><checkstyle version=\"4.3\"><file name=\"&lt;&gt;&amp;&quot;&apos;.js\"><error line=\"&lt;\" column=\"&gt;\" severity=\"error\" message=\"Unexpected &lt;&gt;&amp;&quot;&apos;. (foo)\" source=\"eslint.rules.foo\" /></file></checkstyle>");
+            assert.equal(result, "<?xml version=\"1.0\" encoding=\"utf-8\"?><checkstyle version=\"4.3\"><file name=\"&lt;&gt;&amp;&quot;&apos;.js\"><error line=\"&lt;\" column=\"&gt;\" severity=\"error\" message=\"Unexpected &lt;&gt;&amp;&quot;&apos;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;. (foo)\" source=\"eslint.rules.foo\" /></file></checkstyle>");
         });
     });
 

diff --git a/tests/lib/formatters/jslint-xml.js b/tests/lib/formatters/jslint-xml.js
@@ -121,7 +121,7 @@ describe("formatter:jslint-xml", function() {
         var code = [{
             filePath: "foo.js",
             messages: [{
-                message: "Unexpected <&\"'> foo.",
+                message: "Unexpected <&\"'>\b\t\n\f\r牛逼 foo.",
                 severity: 2,
                 line: 5,
                 column: 10,
@@ -133,7 +133,7 @@ describe("formatter:jslint-xml", function() {
         it("should return a string in JSLint XML format with 1 issue in 1 file", function() {
             var result = formatter(code);
 
-            assert.equal(result, "<?xml version=\"1.0\" encoding=\"utf-8\"?><jslint><file name=\"foo.js\"><issue line=\"5\" char=\"10\" evidence=\"foo\" reason=\"Unexpected &lt;&amp;&quot;&#39;&gt; foo. (foo)\" /></file></jslint>");
+            assert.equal(result, "<?xml version=\"1.0\" encoding=\"utf-8\"?><jslint><file name=\"foo.js\"><issue line=\"5\" char=\"10\" evidence=\"foo\" reason=\"Unexpected &lt;&amp;&quot;&apos;&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924; foo. (foo)\" /></file></jslint>");
         });
     });
 

diff --git a/tests/lib/formatters/junit.js b/tests/lib/formatters/junit.js
@@ -134,7 +134,7 @@ describe("formatter:junit", function() {
         var code = [{
             filePath: "foo.js",
             messages: [{
-                message: "Unexpected <foo></foo>.",
+                message: "Unexpected <foo></foo>\b\t\n\f\r牛逼.",
                 severity: 1,
                 line: 5,
                 column: 10,
@@ -145,7 +145,7 @@ describe("formatter:junit", function() {
         it("should make them go away", function() {
             var result = formatter(code);
 
-            assert.equal(result.replace(/\n/g, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\"><failure message=\"Unexpected &lt;foo&gt;&lt;/foo&gt;.\"><![CDATA[line 5, col 10, Warning - Unexpected &lt;foo&gt;&lt;/foo&gt;. (foo)]]></failure></testcase></testsuite></testsuites>");
+            assert.equal(result.replace(/\n/g, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\"><failure message=\"Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;.\"><![CDATA[line 5, col 10, Warning - Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;. (foo)]]></failure></testcase></testsuite></testsuites>");
         });
     });