Reviewers: felix8a,

Description:
Fixes <https://code.google.com/p/google-caja/issues/detail?id=1579>.

Domado's uriFetch now always invokes its callback, one way or another;
particularly if the URI policy object contains no 'fetch' method.

Please review this at https://codereview.appspot.com/14228045/

Affected files (+58, -33 lines):
  M     src/com/google/caja/plugin/domado.js
  M     tests/com/google/caja/plugin/test-cajajs-invocation.js


Index: tests/com/google/caja/plugin/test-cajajs-invocation.js
===================================================================
--- tests/com/google/caja/plugin/test-cajajs-invocation.js      (revision 5607)
+++ tests/com/google/caja/plugin/test-cajajs-invocation.js      (working copy)
@@ -312,6 +312,25 @@
     });
   });

+  var urlSrcAndXHRTestCase =
+      '<a href="http://fake1.url/foo";>fake1</a>' +
+      '<a href="http://fake2.url/foo";>fake2</a>' +
+      // script should not stall execution, just not load.
+      '<script src="http://bogus.invalid/foo.js";></script>' +
+      // xhr should indicate error response
+      '<script>' +
+      'r("init");' +
+      'var xhr = new XMLHttpRequest();' +
+      'try {' +
+      '  xhr.open("GET", "' + location.protocol + '//' + location.host +
+          '/nonexistent");' +
+      '} catch (e) { r("" + e); }' +
+      'xhr.onreadystatechange = function() {' +
+      '  r(xhr.readyState + xhr.responseText);' +
+      '};' +
+      'xhr.send();' +
+      '</script>';
+
jsunitRegister('testBuilderApiNetNone', function testBuilderApiNetNone() {
     var div = createDiv();
caja.load(div, caja.policy.net.NO_NETWORK, jsunitCallback(function(frame) {
@@ -319,21 +338,7 @@
       frame.code(
           location.protocol + '//' + location.host + '/',
           'text/html',
-          '<a href="http://fake1.url/foo";>fake1</a>' +
-          '<a href="http://fake2.url/foo";>fake2</a>' +
-          // script should not stall execution, just not load.
-          '<script src="http://bogus.invalid/foo.js";></script>' +
-          // xhr should indicate error response
-          '<script>' +
-          'r("init");' +
-          'var xhr = new XMLHttpRequest();' +
- 'try { xhr.open("http://localhost/";); } catch (e) { r("" + e); }' +
-          'xhr.onreadystatechange = function() {' +
-          '  r(xhr.readyState + xhr.responseText);' +
-          '};' +
-          'xhr.send();' +
-          '</script>'
-          )
+          urlSrcAndXHRTestCase)
         .api({r: frame.tame(frame.markFunction(
             function(val) { xhrRes.push(val); }))})
         .run(jsunitCallback(function(result) {
@@ -348,6 +353,30 @@
     }));
   });

+ jsunitRegister('testBuilderApiNetNoFetch', function testBuilderApiNetNone() {
+    var div = createDiv();
+    caja.load(
+        div,
+        { rewrite: caja.policy.net.ALL.rewrite /* but no 'fetch' */ },
+        jsunitCallback(function(frame) {
+      var xhrRes = [];
+      frame.code(
+          location.protocol + '//' + location.host + '/',
+          'text/html',
+          urlSrcAndXHRTestCase)
+        .api({r: frame.tame(frame.markFunction(
+            function(val) { xhrRes.push(val); }))})
+        .run(jsunitCallback(function(result) {
+          assertStringContains('http://fake1.url/foo', div.innerHTML);
+          assertStringContains('http://fake2.url/foo', div.innerHTML);
+          // TODO(kpreid): verify script did not load, as expected
+          // XHR is independent of fetcher
+          assertEquals('init,4<html>\n', String(xhrRes).substr(0, 13));
+          jsunitPass('testBuilderApiNetNoFetch');
+        }));
+    }));
+  });
+
   jsunitRegister('testBuilderApiNetAll', function testBuilderApiNetAll() {
     var div = createDiv();
     caja.load(div, caja.policy.net.ALL, function (frame) {
Index: src/com/google/caja/plugin/domado.js
===================================================================
--- src/com/google/caja/plugin/domado.js        (revision 5607)
+++ src/com/google/caja/plugin/domado.js        (working copy)
@@ -83,29 +83,22 @@
     return (uri.hasScheme() && ALLOWED_URI_SCHEMES.test(uri.getScheme()));
   }

-  function ifThrowsThenNull(uri, func) {
-    try {
-      return func();
-    } catch (e) {
-      console.log('Rejecting url ' + uri + ' because ' + e);
-      return null;
-    }
-  }
-
   function uriFetch(naiveUriPolicy, uri, mime, callback) {
-    if (!naiveUriPolicy || !callback
-      || 'function' !== typeof naiveUriPolicy.fetch) {
-      return;
-    }
     uri = '' + uri;
     var parsed = URI.parse(uri);
-    ifThrowsThenNull(uri, function() {
-      if (allowedUriScheme(parsed)) {
+    try {
+      if (!naiveUriPolicy || !callback
+          || 'function' !== typeof naiveUriPolicy.fetch) {
+        window.setTimeout(function() { callback({}); }, 0);
+      } else if (allowedUriScheme(parsed)) {
         naiveUriPolicy.fetch(parsed, mime, callback);
       } else {
         naiveUriPolicy.fetch(undefined, mime, callback);
       }
-    });
+    } catch (e) {
+      console.log('Rejecting url ' + uri + ' because ' + e);
+      window.setTimeout(function() { callback({}); }, 0);
+    }
   }

   function uriRewrite(naiveUriPolicy, uri, effects, ltype, hints) {
@@ -114,14 +107,17 @@
     }
     uri = '' + uri;
     var parsed = URI.parse(uri);
-    return ifThrowsThenNull(uri, function() {
+    try {
       if (allowedUriScheme(parsed)) {
var safeUri = naiveUriPolicy.rewrite(parsed, effects, ltype, hints);
         return safeUri ? safeUri.toString() : null;
       } else {
         return null;
       }
-    });
+    } catch (e) {
+      console.log('Rejecting url ' + uri + ' because ' + e);
+      return null;
+    }
   }

   var proxiesAvailable = typeof Proxy !== 'undefined';


--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to