From c761a9c82e6a6fef4e715bb719a7f2e9ccc26130 Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@media.mit.edu>
Date: Thu, 7 Feb 2019 11:58:10 -0500
Subject: [PATCH] Fix issue where non-strings were getting passed in to escape
 functions for handling special characters that can appear in xml. Add tests.

---
 src/util/string-util.js  | 15 +++++++++++++-
 src/util/xml-escape.js   | 14 ++++++++++++-
 test/unit/util_string.js | 18 +++++++++++++++++
 test/unit/util_xml.js    | 43 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/src/util/string-util.js b/src/util/string-util.js
index 52a71f654..6fc99530f 100644
--- a/src/util/string-util.js
+++ b/src/util/string-util.js
@@ -1,3 +1,5 @@
+const log = require('./log');
+
 class StringUtil {
     static withoutTrailingDigits (s) {
         let i = s.length - 1;
@@ -62,10 +64,21 @@ class StringUtil {
      * in cases where we're replacing non-user facing strings (e.g. variable IDs).
      * When replacing user facing strings, the xmlEscape utility function should be used
      * instead so that the user facing string does not change how it displays.
-     * @param {!string} unsafe Unsafe string possibly containing unicode control characters.
+     * @param {!string | !Array.<string>} unsafe Unsafe string possibly containing unicode control characters.
+     * In some cases this argument may be an array (e.g. hacked inputs from 2.0)
      * @return {string} String with control characters replaced.
      */
     static replaceUnsafeChars (unsafe) {
+        if (typeof unsafe !== 'string') {
+            if (Array.isArray(unsafe)) {
+                // This happens when we have hacked blocks from 2.0
+                // See #1030
+                unsafe = String(unsafe);
+            } else {
+                log.error('Unexpected input recieved in replaceUnsafeChars');
+                return unsafe;
+            }
+        }
         return unsafe.replace(/[<>&'"]/g, c => {
             switch (c) {
             case '<': return 'lt';
diff --git a/src/util/xml-escape.js b/src/util/xml-escape.js
index 66daf83b4..eebad772f 100644
--- a/src/util/xml-escape.js
+++ b/src/util/xml-escape.js
@@ -1,12 +1,24 @@
+const log = require('./log');
+
 /**
  * Escape a string to be safe to use in XML content.
  * CC-BY-SA: hgoebl
  * https://stackoverflow.com/questions/7918868/
  * how-to-escape-xml-entities-in-javascript
- * @param {!string} unsafe Unsafe string.
+ * @param {!string | !Array.<string>} unsafe Unsafe string.
  * @return {string} XML-escaped string, for use within an XML tag.
  */
 const xmlEscape = function (unsafe) {
+    if (typeof unsafe !== 'string') {
+        if (Array.isArray(unsafe)) {
+            // This happens when we have hacked blocks from 2.0
+            // See #1030
+            unsafe = String(unsafe);
+        } else {
+            log.error('Unexpected input recieved in replaceUnsafeChars');
+            return unsafe;
+        }
+    }
     return unsafe.replace(/[<>&'"]/g, c => {
         switch (c) {
         case '<': return '&lt;';
diff --git a/test/unit/util_string.js b/test/unit/util_string.js
index 7fc88332a..199fa27cf 100644
--- a/test/unit/util_string.js
+++ b/test/unit/util_string.js
@@ -109,3 +109,21 @@ test('replaceUnsafeChars', t => {
 
     t.end();
 });
+
+test('replaceUnsafeChars should handle non strings', t => {
+    const array = ['hello', 'world'];
+    t.equal(StringUtil.replaceUnsafeChars(array), String(array));
+
+    const arrayWithSpecialChar = ['hello', '<world>'];
+    t.equal(StringUtil.replaceUnsafeChars(arrayWithSpecialChar), 'hello,ltworldgt');
+
+    const arrayWithNumbers = [1, 2, 3];
+    t.equal(StringUtil.replaceUnsafeChars(arrayWithNumbers), '1,2,3');
+
+    // Objects shouldn't get provided to replaceUnsafeChars, but in the event
+    // they do, it should just return the object (and log an error)
+    const object = {hello: 'world'};
+    t.equal(StringUtil.replaceUnsafeChars(object), object);
+
+    t.end();
+});
diff --git a/test/unit/util_xml.js b/test/unit/util_xml.js
index d22f37b61..bb7607a69 100644
--- a/test/unit/util_xml.js
+++ b/test/unit/util_xml.js
@@ -7,3 +7,46 @@ test('escape', t => {
     t.strictEqual(xml(input), output);
     t.end();
 });
+
+test('xmlEscape (more)', t => {
+    const empty = '';
+    t.equal(xml(empty), empty);
+
+    const safe = 'hello';
+    t.equal(xml(safe), safe);
+
+    const unsafe = '< > & \' "';
+    t.equal(xml(unsafe), '&lt; &gt; &amp; &apos; &quot;');
+
+    const single = '&';
+    t.equal(xml(single), '&amp;');
+
+    const mix = '<a>b& c\'def_-"';
+    t.equal(xml(mix), '&lt;a&gt;b&amp; c&apos;def_-&quot;');
+
+    const dupes = '<<&_"_"_&>>';
+    t.equal(xml(dupes), '&lt;&lt;&amp;_&quot;_&quot;_&amp;&gt;&gt;');
+
+    const emoji = '(>^_^)>';
+    t.equal(xml(emoji), '(&gt;^_^)&gt;');
+
+    t.end();
+});
+
+test('xmlEscape should handle non strings', t => {
+    const array = ['hello', 'world'];
+    t.equal(xml(array), String(array));
+
+    const arrayWithSpecialChar = ['hello', '<world>'];
+    t.equal(xml(arrayWithSpecialChar), 'hello,&lt;world&gt;');
+
+    const arrayWithNumbers = [1, 2, 3];
+    t.equal(xml(arrayWithNumbers), '1,2,3');
+
+    // Objects shouldn't get provided to replaceUnsafeChars, but in the event
+    // they do, it should just return the object (and log an error)
+    const object = {hello: 'world'};
+    t.equal(xml(object), object);
+
+    t.end();
+});