Revision: 3762
Author: [email protected]
Date: Sun Sep 27 22:20:07 2009
Log: Bug 541: Failure to load an external script is not fatal
http://codereview.appspot.com/124045

http://code.google.com/p/google-caja/issues/detail?id=541
     if <script src="..."> doesn't NPE ( issue 540 )
     and the external js can't be loaded,
     caja emits PluginMessageType.FAILED_TO_LOAD_EXTERNAL_URL,
     which is a warning.

Changed RewriteHtmlStage to throw an exception instead so that a
user installed error handler can deal with it.

[email protected]

http://code.google.com/p/google-caja/source/detail?r=3762

Modified:
  /trunk/src/com/google/caja/plugin/stages/RewriteHtmlStage.java
  /trunk/tests/com/google/caja/opensocial/example-rewritten.xml
  /trunk/tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java

=======================================
--- /trunk/src/com/google/caja/plugin/stages/RewriteHtmlStage.java      Fri Sep 
 
25 14:52:42 2009
+++ /trunk/src/com/google/caja/plugin/stages/RewriteHtmlStage.java      Sun Sep 
 
27 22:20:07 2009
@@ -32,6 +32,7 @@
  import com.google.caja.parser.html.Nodes;
  import com.google.caja.parser.js.Block;
  import com.google.caja.parser.js.Parser;
+import com.google.caja.parser.js.StringLiteral;
  import com.google.caja.plugin.Dom;
  import com.google.caja.plugin.ExtractedHtmlContent;
  import com.google.caja.plugin.Job;
@@ -164,11 +165,15 @@
        jsStream = env.loadExternalResource(
            new ExternalReference(absUri, srcValuePos), "text/javascript");
        if (jsStream == null) {
-        parent.removeChild(scriptTag);
          jobs.getMessageQueue().addMessage(
              PluginMessageType.FAILED_TO_LOAD_EXTERNAL_URL,
              srcValuePos, MessagePart.Factory.valueOf("" + srcUri));
-        return;
+        // Throw an exception so any user installed error handler will  
fire.
+        jsStream = CharProducer.Factory.fromString(
+            "throw new Error("
+            + StringLiteral.toQuotedValue("Failed to load " +  
srcUri.toString())
+            + ");",
+            srcPos);
        }
        scriptPos = null;
      }
=======================================
--- /trunk/tests/com/google/caja/opensocial/example-rewritten.xml       Thu Sep 
 
10 18:33:03 2009
+++ /trunk/tests/com/google/caja/opensocial/example-rewritten.xml       Sun Sep 
 
27 22:20:07 2009
@@ -2,6 +2,7 @@
    ___.loadModule({
        'instantiate': function (___, IMPORTS___) {
          var moduleResult___ = ___.NO_RESULT;
+        var Error = ___.readImport(IMPORTS___, 'Error', { });
          var alert = ___.readImport(IMPORTS___, 'alert');
          var handleClicky = ___.readImport(IMPORTS___, 'handleClicky');
          var onerror = ___.readImport(IMPORTS___, 'onerror');
@@ -40,6 +41,13 @@
            emitter___.setAttr(el___, 'id', 'p3-' +  
IMPORTS___.getIdClass___());
            emitter___.discard(emitter___.attach('id_4___'));
          }
+        try {
+          {
+            throw ___.construct(Error, [ 'Failed to load  
example-gadget-files/no-such-file.js' ]);
+          }
+        } catch (ex___) {
+          ___.getNewModuleHandler().handleUncaughtException(ex___,  
onerror, 'example.xml', '28');
+        }
          try {
            {
              moduleResult___ = (x0___ = (function () {
=======================================
--- /trunk/tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java        
 
Thu Jul 23 09:16:30 2009
+++ /trunk/tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java        
 
Sun Sep 27 22:20:07 2009
@@ -175,6 +175,21 @@
          job("{ c(); }", Job.JobType.JAVASCRIPT));
      assertNoErrors();
    }
+
+  public final void testUnloadableScripts() throws Exception {
+    assertPipeline(
+        job("<script src=content:onerror=panic;></script>"
+            + "<script src=\"http://bogus.com/bogus.js#'!\"></script>"
+            + "<script src=content:foo()></script>",
+            Job.JobType.HTML),
+        job("<span jobnum=\"1\"></span><span jobnum=\"2\"></span>"
+            + "<span jobnum=\"3\"></span>",
+            Job.JobType.HTML),
+        job("{\n  onerror = panic;\n}", Job.JobType.JAVASCRIPT),
+        job("{\n  throw new Error('Failed to load  
http://bogus.com/bogus.js#\\'!');\n}", Job.JobType.JAVASCRIPT),
+        job("{ foo(); }", Job.JobType.JAVASCRIPT));
+    assertNoErrors();
+  }

    @Override
    protected boolean runPipeline(Jobs jobs) throws Exception {

Reply via email to