WebCore:
authorjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Jul 2009 01:10:06 +0000 (01:10 +0000)
committerjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Jul 2009 01:10:06 +0000 (01:10 +0000)
2009-07-29  Jian Li  <jianli@chromium.org>

        Reviewed by Darin Adler.

        Workers need to throw an exception when presented with invalid URLs.
        https://bugs.webkit.org/show_bug.cgi?id=27770

        Tests covered by worker-constructor.html and worker-redirect.html.

        * bindings/js/JSWorkerConstructor.cpp:
        (WebCore::constructWorker):
        * bindings/v8/custom/V8WorkerCustom.cpp:
        (WebCore::CALLBACK_FUNC_DECL):
        * workers/Worker.cpp:
        (WebCore::Worker::Worker):
        * workers/Worker.h:
        (WebCore::Worker::create):
        * workers/WorkerContext.cpp:
        (WebCore::WorkerContext::importScripts):
        * workers/WorkerScriptLoader.cpp:
        (WebCore::WorkerScriptLoader::loadSynchronously):
        (WebCore::WorkerScriptLoader::loadAsynchronously):
        (WebCore::WorkerScriptLoader::createResourceRequest):
        * workers/WorkerScriptLoader.h:

LayoutTests:

2009-07-29  Jian Li  <jianli@chromium.org>

        Reviewed by Darin Adler.

        Workers need to throw an exception when presented with invalid URLs.
        https://bugs.webkit.org/show_bug.cgi?id=27770

        Update worker-constructor.html and worker-redirect.html per the behavior change.

        * fast/workers/worker-constructor-expected.txt:
        * fast/workers/worker-constructor.html:
        * http/tests/workers/worker-redirect-expected.txt:
        * http/tests/workers/worker-redirect.html:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/workers/worker-constructor-expected.txt
LayoutTests/fast/workers/worker-constructor.html
LayoutTests/http/tests/workers/worker-redirect-expected.txt
LayoutTests/http/tests/workers/worker-redirect.html
WebCore/ChangeLog
WebCore/bindings/js/JSWorkerConstructor.cpp
WebCore/bindings/v8/custom/V8WorkerCustom.cpp
WebCore/workers/Worker.cpp
WebCore/workers/Worker.h
WebCore/workers/WorkerContext.cpp
WebCore/workers/WorkerScriptLoader.cpp
WebCore/workers/WorkerScriptLoader.h

index 2f437f7..d9a5cf8 100644 (file)
@@ -1,3 +1,17 @@
+2009-07-29  Jian Li  <jianli@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Workers need to throw an exception when presented with invalid URLs.
+        https://bugs.webkit.org/show_bug.cgi?id=27770
+
+        Update worker-constructor.html and worker-redirect.html per the behavior change.
+
+        * fast/workers/worker-constructor-expected.txt:
+        * fast/workers/worker-constructor.html:
+        * http/tests/workers/worker-redirect-expected.txt:
+        * http/tests/workers/worker-redirect.html:
+
 2009-07-29  Ryosuke Niwa  <rniwa@webkit.org>
 
         Reviewed by Justin Garcia.
index 311e19b..f1c22d0 100644 (file)
@@ -3,8 +3,8 @@ Test Worker constructor functionality. Should print a series of PASS messages, f
 PASS: toString exception propagated correctly.
 PASS: trying to create workers recursively resulted in an exception (RangeError: Maximum call stack size exceeded.)
 PASS: invoking Worker constructor without arguments resulted in an exception (SyntaxError: Not enough arguments)
-PASS: onerror invoked for an empty script URL.
-PASS: onerror invoked for an invalid script URL.
+PASS: invoking Worker constructor with empty script URL resulted in an exception (Error: SYNTAX_ERR: DOM Exception 12)
+PASS: invoking Worker constructor with invalid script URL resulted in an exception (Error: SYNTAX_ERR: DOM Exception 12)
 PASS: onerror invoked for a script that could not be loaded.
 PASS: Successfully created worker.
 DONE
index 0f83be1..9a1c7e8 100644 (file)
@@ -72,11 +72,11 @@ function testEmptyScriptUrl()
     try {
         var worker = new Worker("");
         worker.onerror = function() {
-            log("PASS: onerror invoked for an empty script URL.");
+            log("FAIL: onerror invoked for an empty script URL.");
             runNextTest();
         }
     } catch (ex) {
-        log("FAIL: unexpected exception " + ex);
+        log("PASS: invoking Worker constructor with empty script URL resulted in an exception (" + ex + ")");
         runNextTest();
     }
 }
@@ -84,13 +84,13 @@ function testEmptyScriptUrl()
 function testInvalidScriptUrl()
 {
     try {
-        var worker = new Worker("invalidurl://");
+        var worker = new Worker("http://invalid:123$");
         worker.onerror = function() {
-            log("PASS: onerror invoked for an invalid script URL.");
+            log("FAIL: onerror invoked for an invalid script URL.");
             runNextTest();
         }
     } catch (ex) {
-        log("FAIL: unexpected exception " + ex);
+        log("PASS: invoking Worker constructor with invalid script URL resulted in an exception (" + ex + ")");
         runNextTest();
     }
 }
index 0b15b3b..b141c73 100644 (file)
@@ -1,6 +1,6 @@
 Test that loading the worker's script does not allow a cross origin redirect (bug 26146)
 
-SUCCESS: threw error when attempting to cross origin while loading the worker script.
+SUCCESS: threw exception (Error: SECURITY_ERR: DOM Exception 18) when attempting to cross origin while loading the worker script.
 SUCCESS: threw error when attempting to redirected cross origin while loading the worker script.
 DONE
 
index 8f7239d..972744f 100644 (file)
@@ -7,40 +7,56 @@ function log(message)
     document.getElementById("result").innerHTML += message + "<br>";
 }
 
+var testCases = [
+    "testCrossOriginLoad",
+    "testCrossOriginRedirectedLoad",
+];
+var testIndex = 0;
+
 function runNextTest()
 {
-    testIndex++;
-    if (testIndex > totalTests) {
+    if (testIndex < testCases.length) {
+        testIndex++;
+        window[testCases[testIndex - 1]]();
+    } else {
         log("DONE");
         if (window.layoutTestController)
             layoutTestController.notifyDone();
-    } else {
-        eval("test" + testIndex + "();");
     }
 }
 
-function test1()
+function testCrossOriginLoad()
 {
-    var worker = new Worker('http://localhost:8000/workers/resources/worker-redirect-target.js');
-    worker.onerror = function(evt) {
-        log("SUCCESS: threw error when attempting to cross origin while loading the worker script.");
-        runNextTest();
-    }
-    worker.onmessage = function(evt) {
-        log("FAIL: executed script when redirect cross origin.");
+    try {
+        var worker = new Worker('http://localhost:8000/workers/resources/worker-redirect-target.js');
+        worker.onerror = function(evt) {
+            log("FAIL: threw error when attempting to cross origin while loading the worker script.");
+            runNextTest();
+        }
+        worker.onmessage = function(evt) {
+            log("FAIL: executed script when redirect cross origin.");
+            runNextTest();
+        }
+    } catch (ex) {
+        log("SUCCESS: threw exception (" + ex + ") when attempting to cross origin while loading the worker script.");
         runNextTest();
     }
 }
 
-function test2()
+function testCrossOriginRedirectedLoad()
 {
-    var worker = new Worker('/resources/redirect.php?url=http://localhost:8000/workers/resources/worker-redirect-target.js');
-    worker.onerror = function(evt) {
-        log("SUCCESS: threw error when attempting to redirected cross origin while loading the worker script.");
-        runNextTest();
-    }
-    worker.onmessage = function(evt) {
-        log("FAIL: executed script when redirect cross origin.");
+    try {
+        var worker = new Worker('/resources/redirect.php?url=http://localhost:8000/workers/resources/worker-redirect-target.js');
+        worker.onerror = function(evt) {
+            log("SUCCESS: threw error when attempting to redirected cross origin while loading the worker script.");
+            runNextTest();
+        }
+        worker.onmessage = function(evt) {
+            log("FAIL: executed script when redirect cross origin.");
+            runNextTest();
+        }
+    } catch (ex) {
+        log("FAIL: unexpected exception " + ex);
         runNextTest();
     }
 }
@@ -50,8 +66,6 @@ if (window.layoutTestController) {
     layoutTestController.waitUntilDone();
 }
 
-var totalTests = 2;
-var testIndex = 0;
 runNextTest();
 
 </script>
index 998e321..db9f339 100644 (file)
@@ -1,3 +1,28 @@
+2009-07-29  Jian Li  <jianli@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Workers need to throw an exception when presented with invalid URLs.
+        https://bugs.webkit.org/show_bug.cgi?id=27770
+
+        Tests covered by worker-constructor.html and worker-redirect.html.
+
+        * bindings/js/JSWorkerConstructor.cpp:
+        (WebCore::constructWorker):
+        * bindings/v8/custom/V8WorkerCustom.cpp:
+        (WebCore::CALLBACK_FUNC_DECL):
+        * workers/Worker.cpp:
+        (WebCore::Worker::Worker):
+        * workers/Worker.h:
+        (WebCore::Worker::create):
+        * workers/WorkerContext.cpp:
+        (WebCore::WorkerContext::importScripts):
+        * workers/WorkerScriptLoader.cpp:
+        (WebCore::WorkerScriptLoader::loadSynchronously):
+        (WebCore::WorkerScriptLoader::loadAsynchronously):
+        (WebCore::WorkerScriptLoader::createResourceRequest):
+        * workers/WorkerScriptLoader.h:
+
 2009-07-29  Brady Eidson  <beidson@apple.com>
 
         Reviewed by Darin Adler.
index e1686f7..9943cfb 100644 (file)
@@ -62,7 +62,12 @@ static JSObject* constructWorker(ExecState* exec, JSObject* constructor, const A
     // See section 4.8.2 step 14 of WebWorkers for why this is the lexicalGlobalObject. 
     DOMWindow* window = asJSDOMWindow(exec->lexicalGlobalObject())->impl();
 
-    RefPtr<Worker> worker = Worker::create(scriptURL, window->document());
+    ExceptionCode ec = 0;
+    RefPtr<Worker> worker = Worker::create(scriptURL, window->document(), ec);
+    if (ec) {
+        setDOMException(exec, ec);
+        return 0;
+    }
 
     return asObject(toJS(exec, jsConstructor->globalObject(), worker.release()));
 }
index 1f8aeaf..08e8be6 100644 (file)
@@ -78,7 +78,10 @@ CALLBACK_FUNC_DECL(WorkerConstructor)
 
     // Create the worker object.
     // Note: it's OK to let this RefPtr go out of scope because we also call setDOMWrapper(), which effectively holds a reference to obj.
-    RefPtr<Worker> obj = Worker::create(toWebCoreString(scriptUrl), context);
+    ExceptionCode ec = 0;
+    RefPtr<Worker> obj = Worker::create(toWebCoreString(scriptUrl), context, ec);
+    if (ec)
+        return throwError(ec);
 
     // Setup the standard wrapper object internal fields.
     v8::Handle<v8::Object> wrapperObject = args.Holder();
index 866687f..9185bae 100644 (file)
 
 namespace WebCore {
 
-Worker::Worker(const String& url, ScriptExecutionContext* context)
+Worker::Worker(const String& url, ScriptExecutionContext* context, ExceptionCode& ec)
     : AbstractWorker(context)
     , m_contextProxy(WorkerContextProxy::create(this))
 {
+    if (url.isEmpty()) {
+        ec = SYNTAX_ERR;
+        return;
+    }
+
+    KURL scriptURL = context->completeURL(url);
+    if (!scriptURL.isValid()) {
+        ec = SYNTAX_ERR;
+        return;
+    }
+
+    if (!context->securityOrigin()->canAccess(SecurityOrigin::create(scriptURL).get())) {
+        ec = SECURITY_ERR;
+        return;
+    }
+
     m_scriptLoader = new WorkerScriptLoader();
-    m_scriptLoader->loadAsynchronously(scriptExecutionContext(), url, CompleteURL, DenyCrossOriginLoad, this);
+    m_scriptLoader->loadAsynchronously(scriptExecutionContext(), scriptURL, DenyCrossOriginRedirect, this);
     setPendingActivity(this);  // The worker context does not exist while loading, so we must ensure that the worker object is not collected, as well as its event listeners.
 }
 
index 6b8ee63..66a8ae9 100644 (file)
@@ -51,7 +51,7 @@ namespace WebCore {
 
     class Worker : public AbstractWorker, private WorkerScriptLoaderClient {
     public:
-        static PassRefPtr<Worker> create(const String& url, ScriptExecutionContext* context) { return adoptRef(new Worker(url, context)); }
+        static PassRefPtr<Worker> create(const String& url, ScriptExecutionContext* context, ExceptionCode& ec) { return adoptRef(new Worker(url, context, ec)); }
         ~Worker();
 
         virtual Worker* toWorker() { return this; }
@@ -72,7 +72,7 @@ namespace WebCore {
         EventListener* onmessage() const { return m_onMessageListener.get(); }
 
     private:
-        Worker(const String&, ScriptExecutionContext*);
+        Worker(const String&, ScriptExecutionContext*, ExceptionCode&);
 
         virtual void notifyFinished();
 
index 88eca62..eb82a5b 100644 (file)
@@ -244,7 +244,7 @@ void WorkerContext::importScripts(const Vector<String>& urls, const String& call
 
     for (Vector<KURL>::const_iterator it = completedURLs.begin(); it != end; ++it) {
         WorkerScriptLoader scriptLoader;
-        scriptLoader.loadSynchronously(scriptExecutionContext(), *it, DoNotCompleteURL, AllowCrossOriginLoad);
+        scriptLoader.loadSynchronously(scriptExecutionContext(), *it, AllowCrossOriginRedirect);
 
         // If the fetching attempt failed, throw a NETWORK_ERR exception and abort all these steps.
         if (scriptLoader.failed()) {
index 093bf07..0162b26 100644 (file)
@@ -50,55 +50,33 @@ WorkerScriptLoader::WorkerScriptLoader()
 {
 }
 
-static CrossOriginRedirectPolicy toCrossOriginRedirectPolicy(CrossOriginLoadPolicy crossOriginLoadPolicy)
+void WorkerScriptLoader::loadSynchronously(ScriptExecutionContext* scriptExecutionContext, const KURL& url, CrossOriginRedirectPolicy crossOriginRedirectPolicy)
 {
-    return (crossOriginLoadPolicy == DenyCrossOriginLoad) ? DenyCrossOriginRedirect : AllowCrossOriginRedirect;
-}
+    m_url = url;
 
-void WorkerScriptLoader::loadSynchronously(ScriptExecutionContext* scriptExecutionContext, const String& url, URLCompletionPolicy urlCompletionPolicy, CrossOriginLoadPolicy crossOriginLoadPolicy)
-{
-    OwnPtr<ResourceRequest> request(createResourceRequest(scriptExecutionContext, url, urlCompletionPolicy, crossOriginLoadPolicy));
+    OwnPtr<ResourceRequest> request(createResourceRequest());
     if (!request)
         return;
 
     ASSERT(scriptExecutionContext->isWorkerContext());
-    WorkerThreadableLoader::loadResourceSynchronously(static_cast<WorkerContext*>(scriptExecutionContext), *request, *this, AllowStoredCredentials, toCrossOriginRedirectPolicy(crossOriginLoadPolicy));
+    WorkerThreadableLoader::loadResourceSynchronously(static_cast<WorkerContext*>(scriptExecutionContext), *request, *this, AllowStoredCredentials, crossOriginRedirectPolicy);
 }
     
-void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext* scriptExecutionContext, const String& url, URLCompletionPolicy urlCompletionPolicy, CrossOriginLoadPolicy crossOriginLoadPolicy, WorkerScriptLoaderClient* client)
+void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext* scriptExecutionContext, const KURL& url, CrossOriginRedirectPolicy crossOriginRedirectPolicy, WorkerScriptLoaderClient* client)
 {
     ASSERT(client);
     m_client = client;
+    m_url = url;
 
-    OwnPtr<ResourceRequest> request(createResourceRequest(scriptExecutionContext, url, urlCompletionPolicy, crossOriginLoadPolicy));
+    OwnPtr<ResourceRequest> request(createResourceRequest());
     if (!request)
         return;
 
-    m_threadableLoader = ThreadableLoader::create(scriptExecutionContext, this, *request, DoNotSendLoadCallbacks, DoNotSniffContent, AllowStoredCredentials, toCrossOriginRedirectPolicy(crossOriginLoadPolicy));
+    m_threadableLoader = ThreadableLoader::create(scriptExecutionContext, this, *request, DoNotSendLoadCallbacks, DoNotSniffContent, AllowStoredCredentials, crossOriginRedirectPolicy);
 }
 
-static void notifyLoadErrorTask(ScriptExecutionContext* context, WorkerScriptLoader* loader)
+PassOwnPtr<ResourceRequest> WorkerScriptLoader::createResourceRequest()
 {
-    UNUSED_PARAM(context);
-    loader->notifyError();
-}
-
-PassOwnPtr<ResourceRequest> WorkerScriptLoader::createResourceRequest(ScriptExecutionContext* scriptExecutionContext, const String& url, URLCompletionPolicy urlCompletionPolicy, CrossOriginLoadPolicy crossOriginLoadPolicy)
-{
-    if (urlCompletionPolicy == CompleteURL) {
-        m_url = scriptExecutionContext->completeURL(url);
-        if (url.isEmpty() || !m_url.isValid()) {
-            scriptExecutionContext->postTask(createCallbackTask(&notifyLoadErrorTask, this));
-            return 0;
-        }
-    } else
-        m_url = KURL(url);
-
-    if (crossOriginLoadPolicy == DenyCrossOriginLoad && !scriptExecutionContext->securityOrigin()->canAccess(SecurityOrigin::create(m_url).get())) {
-        scriptExecutionContext->postTask(createCallbackTask(&notifyLoadErrorTask, this));
-        return 0;
-    }
-
     OwnPtr<ResourceRequest> request(new ResourceRequest(m_url));
     request->setHTTPMethod("GET");
 
index f465d7f..2924ec8 100644 (file)
@@ -42,22 +42,12 @@ namespace WebCore {
     class ScriptExecutionContext;
     class WorkerScriptLoaderClient;
 
-    enum URLCompletionPolicy {
-        CompleteURL,
-        DoNotCompleteURL
-    };
-
-    enum CrossOriginLoadPolicy {
-        DenyCrossOriginLoad,
-        AllowCrossOriginLoad
-    };
-
     class WorkerScriptLoader : public ThreadableLoaderClient {
     public:
         WorkerScriptLoader();
 
-        void loadSynchronously(ScriptExecutionContext*, const String& url, URLCompletionPolicy, CrossOriginLoadPolicy);
-        void loadAsynchronously(ScriptExecutionContext*, const String& url, URLCompletionPolicy, CrossOriginLoadPolicy, WorkerScriptLoaderClient*);
+        void loadSynchronously(ScriptExecutionContext*, const KURL&, CrossOriginRedirectPolicy);
+        void loadAsynchronously(ScriptExecutionContext*, const KURL&, CrossOriginRedirectPolicy, WorkerScriptLoaderClient*);
 
         void notifyError();
 
@@ -74,7 +64,7 @@ namespace WebCore {
         virtual void didReceiveAuthenticationCancellation(const ResourceResponse&);
 
     private:
-        PassOwnPtr<ResourceRequest> createResourceRequest(ScriptExecutionContext*, const String& url, URLCompletionPolicy, CrossOriginLoadPolicy);
+        PassOwnPtr<ResourceRequest> createResourceRequest();
         void notifyFinished();
 
         WorkerScriptLoaderClient* m_client;