Worker.importScript() should clean errors for cross origin imports.
authorlevin@chromium.org <levin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Feb 2011 00:48:28 +0000 (00:48 +0000)
committerlevin@chromium.org <levin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Feb 2011 00:48:28 +0000 (00:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=52871

Reviewed by Adam Barth and Oliver Hunt.

Source/WebCore:

Test: http/tests/workers/worker-importScriptsOnError.html

* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::evaluate): Use sanitizeScriptError
to determine when to create a clean exception.
* bindings/v8/WorkerContextExecutionProxy.cpp:
(WebCore::WorkerContextExecutionProxy::evaluate): Ditto.
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::sanitizeScriptError): Figure out
if the error needs to be cleaned up.
(WebCore::ScriptExecutionContext::dispatchErrorEvent): Extracted
sanitizeScriptError for use by other places.
* dom/ScriptExecutionContext.h:
* workers/WorkerContext.cpp:
(WebCore::WorkerContext::importScripts): Use the reponse url when
telling the evaluate where the script came fro.
* workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::responseURL): Expose the url that
the script was loaded from (which may be different from url() due
to redirects).
(WebCore::WorkerScriptLoader::didReceiveResponse): Capture the reponse url.
* workers/WorkerScriptLoader.h:

LayoutTests:

* http/tests/workers/resources/worker-importScripts-error.js: Added.
* http/tests/workers/resources/worker-importScripts-throw.js: Added.
* http/tests/workers/worker-importScriptsOnError.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77563 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/resources/worker-importScripts-error.js [new file with mode: 0644]
LayoutTests/http/tests/workers/resources/worker-importScripts-throw.js [new file with mode: 0644]
LayoutTests/http/tests/workers/worker-importScriptsOnError-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/worker-importScriptsOnError.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/WorkerScriptController.cpp
Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/dom/ScriptExecutionContext.h
Source/WebCore/workers/WorkerContext.cpp
Source/WebCore/workers/WorkerScriptLoader.cpp
Source/WebCore/workers/WorkerScriptLoader.h

index 4d64fe6ea8ac981bd09f33132afcad71f28baa4b..8734a27f87b593ff40f3852fc8d8e41007204cda 100644 (file)
@@ -1,3 +1,14 @@
+2011-02-03  David Levin  <levin@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Worker.importScript() should clean errors for cross origin imports.
+        https://bugs.webkit.org/show_bug.cgi?id=52871
+
+        * http/tests/workers/resources/worker-importScripts-error.js: Added.
+        * http/tests/workers/resources/worker-importScripts-throw.js: Added.
+        * http/tests/workers/worker-importScriptsOnError.html: Added.
+
 2011-02-03  Mihai Parparita  <mihaip@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
diff --git a/LayoutTests/http/tests/workers/resources/worker-importScripts-error.js b/LayoutTests/http/tests/workers/resources/worker-importScripts-error.js
new file mode 100644 (file)
index 0000000..816352e
--- /dev/null
@@ -0,0 +1,2 @@
+var differentRedirectOrigin = "/resources/redirect.php?url=http://localhost:8000/workers/resources/worker-importScripts-throw.js";
+importScripts(differentRedirectOrigin)
diff --git a/LayoutTests/http/tests/workers/resources/worker-importScripts-throw.js b/LayoutTests/http/tests/workers/resources/worker-importScripts-throw.js
new file mode 100644 (file)
index 0000000..fdc1e01
--- /dev/null
@@ -0,0 +1 @@
+throw("This is a custom error message.");
diff --git a/LayoutTests/http/tests/workers/worker-importScriptsOnError-expected.txt b/LayoutTests/http/tests/workers/worker-importScriptsOnError-expected.txt
new file mode 100644 (file)
index 0000000..d5949d4
--- /dev/null
@@ -0,0 +1,42 @@
+Test importScripts with error.
+
+
+AT_TARGET: 2,
+BLUR: 8192,
+BUBBLING_PHASE: 3,
+CAPTURING_PHASE: 1,
+CHANGE: 32768,
+CLICK: 64,
+DBLCLICK: 128,
+DRAGDROP: 2048,
+FOCUS: 4096,
+KEYDOWN: 256,
+KEYPRESS: 1024,
+KEYUP: 512,
+MOUSEDOWN: 1,
+MOUSEDRAG: 32,
+MOUSEMOVE: 16,
+MOUSEOUT: 8,
+MOUSEOVER: 4,
+MOUSEUP: 2,
+SELECT: 16384,
+bubbles: false,
+cancelBubble: false,
+cancelable: true,
+clipboardData: undefined,
+currentTarget: [object Worker],
+defaultPrevented: false,
+eventPhase: 2,
+filename: http://127.0.0.1:8000/workers/resources/worker-importScripts-error.js,
+initErrorEvent: function initErrorEvent() { [native code] },
+initEvent: function initEvent() { [native code] },
+lineno: 2,
+message: Error: Script error.,
+preventDefault: function preventDefault() { [native code] },
+returnValue: true,
+srcElement: [object Worker],
+stopImmediatePropagation: function stopImmediatePropagation() { [native code] },
+stopPropagation: function stopPropagation() { [native code] },
+target: [object Worker],
+type: error
+
diff --git a/LayoutTests/http/tests/workers/worker-importScriptsOnError.html b/LayoutTests/http/tests/workers/worker-importScriptsOnError.html
new file mode 100644 (file)
index 0000000..2ff392c
--- /dev/null
@@ -0,0 +1,32 @@
+<body>
+<p>Test importScripts with error.</p>
+<div id=result></div>
+<script>
+function log(message)
+{
+    document.getElementById("result").innerHTML += message + "<br>";
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+var worker = new Worker('resources/worker-importScripts-error.js');
+
+worker.onerror = function(error)
+{
+    var properties = [];
+    for (property in error) {
+        if (property == 'timeStamp')
+            continue;
+        properties.push('<br/>' + property + ': ' + error[property]);
+    }
+    properties.sort();
+    log(properties);
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+</script>
+</body>
+</html>
index 9223cc1713476115d4d912052b6dc400af331b83..bbf143fec225a9476dd9fa95442f15952f1d7a06 100644 (file)
@@ -1,3 +1,33 @@
+2011-02-03  David Levin  <levin@chromium.org>
+
+        Reviewed by Adam Barth and Oliver Hunt.
+
+        Worker.importScript() should clean errors for cross origin imports.
+        https://bugs.webkit.org/show_bug.cgi?id=52871
+
+        Test: http/tests/workers/worker-importScriptsOnError.html
+
+        * bindings/js/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::evaluate): Use sanitizeScriptError
+        to determine when to create a clean exception.
+        * bindings/v8/WorkerContextExecutionProxy.cpp:
+        (WebCore::WorkerContextExecutionProxy::evaluate): Ditto.
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::sanitizeScriptError): Figure out
+        if the error needs to be cleaned up.
+        (WebCore::ScriptExecutionContext::dispatchErrorEvent): Extracted
+        sanitizeScriptError for use by other places.
+        * dom/ScriptExecutionContext.h:
+        * workers/WorkerContext.cpp:
+        (WebCore::WorkerContext::importScripts): Use the reponse url when
+        telling the evaluate where the script came fro.
+        * workers/WorkerScriptLoader.cpp:
+        (WebCore::WorkerScriptLoader::responseURL): Expose the url that
+        the script was loaded from (which may be different from url() due
+        to redirects).
+        (WebCore::WorkerScriptLoader::didReceiveResponse): Capture the reponse url.
+        * workers/WorkerScriptLoader.h:
+
 2011-02-03  Mark Mentovai  <mark@chromium.org>
 
         Reviewed by Dimitri Glazkov.
index e758b477644036cb7c8113e9c81a9aab7324555b..0c89632d98ceee47e332fd20f6960f1ac5c9aa97 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -131,8 +132,15 @@ ScriptValue WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode,
     if (comp.complType() == Normal || comp.complType() == ReturnValue)
         return comp.value();
 
-    if (comp.complType() == Throw)
-        *exception = comp.value();
+    if (comp.complType() == Throw) {
+        String errorMessage;
+        int lineNumber = 0;
+        String sourceURL = sourceCode.url().string();
+        if (m_workerContext->sanitizeScriptError(errorMessage, lineNumber, sourceURL))
+            *exception = ScriptValue(throwError(exec, createError(exec, errorMessage.impl())));
+        else
+            *exception = comp.value();
+    }
     return JSValue();
 }
 
index aef892ab44afeb2696461df759f4c7d96c786dde..baae849c036acbbac815c78d43b7bafac1fd69eb 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2009, 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -205,10 +205,14 @@ ScriptValue WorkerContextExecutionProxy::evaluate(const String& script, const St
     if (exceptionCatcher.HasCaught()) {
         v8::Local<v8::Message> message = exceptionCatcher.Message();
         state->hadException = true;
-        state->exception = ScriptValue(exceptionCatcher.Exception());
         state->errorMessage = toWebCoreString(message->Get());
         state->lineNumber = message->GetLineNumber();
         state->sourceURL = toWebCoreString(message->GetScriptResourceName());
+        if (m_workerContext->sanitizeScriptError(state->errorMessage, state->lineNumber, state->sourceURL))
+            state->exception = V8Proxy::throwError(V8Proxy::GeneralError, state->errorMessage.utf8().data());
+        else
+            state->exception = ScriptValue(exceptionCatcher.Exception());
+
         exceptionCatcher.Reset();
     } else
         state->hadException = false;
index 7f8cc9f14f28518ac670c543433b28fb39c4a912..19267c63d78c30795eb67cf3c63c7ed4610af6f1 100644 (file)
@@ -305,6 +305,17 @@ void ScriptExecutionContext::setSecurityOrigin(PassRefPtr<SecurityOrigin> securi
     m_securityOrigin = securityOrigin;
 }
 
+bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& lineNumber, String& sourceURL)
+{
+    KURL targetURL = completeURL(sourceURL);
+    if (securityOrigin()->canRequest(targetURL))
+        return false;
+    errorMessage = "Script error.";
+    sourceURL = String();
+    lineNumber = 0;
+    return true;
+}
+
 void ScriptExecutionContext::reportException(const String& errorMessage, int lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack> callStack)
 {
     if (m_inDispatchErrorEvent) {
@@ -334,19 +345,10 @@ bool ScriptExecutionContext::dispatchErrorEvent(const String& errorMessage, int
     if (!target)
         return false;
 
-    String message;
-    int line;
-    String sourceName;
-    KURL targetUrl = completeURL(sourceURL);
-    if (securityOrigin()->canRequest(targetUrl)) {
-        message = errorMessage;
-        line = lineNumber;
-        sourceName = sourceURL;
-    } else {
-        message = "Script error.";
-        sourceName = String();
-        line = 0;
-    }
+    String message = errorMessage;
+    int line = lineNumber;
+    String sourceName = sourceURL;
+    sanitizeScriptError(message, line, sourceName);
 
     ASSERT(!m_inDispatchErrorEvent);
     m_inDispatchErrorEvent = true;
index 19a3387f088f8298efc50db62607c79b2036052b..642906c4082e05aec6b3293f0f6a594b242df0cb 100644 (file)
@@ -91,6 +91,7 @@ namespace WebCore {
 
         SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); }
 
+        bool sanitizeScriptError(String& errorMessage, int& lineNumber, String& sourceURL);
         void reportException(const String& errorMessage, int lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack>);
         virtual void addMessage(MessageSource, MessageType, MessageLevel, const String& message, unsigned lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack>) = 0;
 
index 36c42154a7d5886b36893c103940cb0af94a1c8c..acf7f046d753761a111fb8b38d2527423aa9f512 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
- * Copyright (C) 2009 Google Inc. All Rights Reserved.
+ * Copyright (C) 2009, 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -249,7 +249,7 @@ void WorkerContext::importScripts(const Vector<String>& urls, ExceptionCode& ec)
        InspectorInstrumentation::scriptImported(scriptExecutionContext(), scriptLoader.identifier(), scriptLoader.script());
 
         ScriptValue exception;
-        m_script->evaluate(ScriptSourceCode(scriptLoader.script(), *it), &exception);
+        m_script->evaluate(ScriptSourceCode(scriptLoader.script(), scriptLoader.responseURL()), &exception);
         if (!exception.hasNoValue()) {
             m_script->setException(exception);
             return;
index 1786b89c810add47cf3750cb9f8006b1c4f69493..2224d87228fafc7d6222c46bfb77c09d8c6dd376 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2009 Apple Inc. All Rights Reserved.
- * Copyright (C) 2009 Google Inc. All Rights Reserved.
+ * Copyright (C) 2009, 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -87,6 +87,12 @@ void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext* scriptExecut
     m_threadableLoader = ThreadableLoader::create(scriptExecutionContext, this, *request, options);
 }
 
+const KURL& WorkerScriptLoader::responseURL() const
+{
+    ASSERT(!failed());
+    return m_responseURL;
+}
+
 PassOwnPtr<ResourceRequest> WorkerScriptLoader::createResourceRequest()
 {
     OwnPtr<ResourceRequest> request(new ResourceRequest(m_url));
@@ -101,6 +107,7 @@ void WorkerScriptLoader::didReceiveResponse(const ResourceResponse& response)
         m_failed = true;
         return;
     }
+    m_responseURL = response.url();
     m_responseEncoding = response.textEncodingName();
     if (m_client)
         m_client->didReceiveResponse(response);
index fc8b0b44efeb14e0b2c3f3fac542373ecc21c0b3..98e21e1a34094b91e1fc6df5e0461e6cb526f55a 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2009 Apple Inc. All Rights Reserved.
- * Copyright (C) 2009 Google Inc. All Rights Reserved.
+ * Copyright (C) 2009, 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -53,6 +53,7 @@ namespace WebCore {
 
         const String& script() const { return m_script; }
         const KURL& url() const { return m_url; }
+        const KURL& responseURL() const;
         bool failed() const { return m_failed; }
         unsigned long identifier() const { return m_identifier; }
 
@@ -73,6 +74,7 @@ namespace WebCore {
         RefPtr<TextResourceDecoder> m_decoder;
         String m_script;
         KURL m_url;
+        KURL m_responseURL;
         bool m_failed;
         unsigned long m_identifier;
         ResourceRequestBase::TargetType m_targetType;