[Caja] Bug 714: el.innerHTML = undefined;

[mikesamuel]

Reviewers: felix8a,
Description:
http://code.google.com/p/google-caja/issues/detail?id=714

    in browsers, that stringifies undefined and you see 'undefined'.
    in caja, domita.js setInnerHTML says
      sanitizeHtml(String(htmlFragment || ''))
    which turns undefined into '' instead.

    this is mostly an issue of developer-friendliness.  eg, someone
    says
      el.innerHTML = foo();
    and expects that to show something helpful, even if foo() is
    buggy.  instead, it shows nothing, and it's unclear whether the
    call to foo() happened at all.

Fixed setInnerHtml in DOMita and added tests.

Please review this at http://codereview.appspot.com/124051

Affected files:
  M     src/com/google/caja/plugin/domita.js
  M     tests/com/google/caja/plugin/domita_test_untrusted.html


Index: tests/com/google/caja/plugin/domita_test_untrusted.html
===================================================================
--- tests/com/google/caja/plugin/domita_test_untrusted.html     (revision 3752)
+++ tests/com/google/caja/plugin/domita_test_untrusted.html     (working copy)
@@ -1177,6 +1177,21 @@
       + '?url=http%3A%2F%2Fbar.com%2Fbaz&mimeType=*%2F*"  
id="foo-xyz___"'
       + ' target="_blank" title="A link">A &amp; B &amp; C&lt;</a>',
       canonInnerHtml(directAccess.getInnerHTML(container)));
+
+  var span = document.createElement('SPAN');
+  container.appendChild(span);
+  // See Issue 714 for the derivation of these tests.
+  var inputsAndGoldens = [
+      [null, ''],
+      [undefined, 'undefined'],
+      [4, '4'],
+      [{}, '[object Object]'],
+      ['my_string', 'my_string']];
+  for (var i = 0; i < inputsAndGoldens.length; ++i) {
+    var pair = inputsAndGoldens[i];
+    span.innerHTML = pair[0];
+    assertEquals(pair[1], span.innerHTML);
+  }

   pass('test-inner-html');
 });
Index: src/com/google/caja/plugin/domita.js
===================================================================
--- src/com/google/caja/plugin/domita.js        (revision 3752)
+++ src/com/google/caja/plugin/domita.js        (working copy)
@@ -2254,13 +2254,20 @@
       if (!html4.ELEMENTS.hasOwnProperty(tagName)) { throw new Error(); }
       var flags = html4.ELEMENTS[tagName];
       if (flags & html4.eflags.UNSAFE) { throw new Error(); }
+      var isRcData = flags & html4.eflags.RCDATA;
+      var htmlFragmentString;
+      if (!isRcData && htmlFragment instanceof Html) {
+        htmlFragmentString = '' + safeHtml(htmlFragment);
+      } else if (htmlFragment === null) {
+        htmlFragmentString = '';
+      } else {
+        htmlFragmentString += '';
+      }
       var sanitizedHtml;
-      if (flags & html4.eflags.RCDATA) {
-        sanitizedHtml = html.normalizeRCData(String(htmlFragment || ''));
+      if (rcData) {
+        sanitizedHtml = html.normalizeRCData(htmlFragmentString);
       } else {
-        sanitizedHtml = (htmlFragment instanceof Html
-                        ? safeHtml(htmlFragment)
-                        : sanitizeHtml(String(htmlFragment || '')));
+        sanitizedHtml = sanitizeHtml(htmlFragmentString);
       }
       this.node___.innerHTML = sanitizedHtml;
       return htmlFragment;