Attachment 275966 Details for Bug 153598 – Patch

[patch] Patch

bug-153598-20160407183831.patch (text/plain), 20.14 KB, created by John Wilander on 2016-04-07 18:38:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: John Wilander

Created: 2016-04-07 18:38:28 PDT

Size: 20.14 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 199205)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2016-04-07  John Wilander  <wilander@apple.com>
>+
>+        CSP: Block XHR when calling XMLHttpRequest.send() and throw network error.
>+        https://bugs.webkit.org/show_bug.cgi?id=153598
>+        <rdar://problem/24391483>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests. Changes to existing tests are sufficient.
>+
>+        * xml/XMLHttpRequest.cpp:
>+        (WebCore::XMLHttpRequest::open):
>+        (WebCore::XMLHttpRequest::initSend):
>+            Moved the CSP check from XMLHttpRequest::open() to XMLHttpRequest::initSend().
>+            Changed the thrown error type from Security to Network for synchronous requests.
>+            Changed from throwing an error to firing an error event for asynchronous requests.
>+            These changes are in conformance with connect-src of Content Security Policy Level 2.
>+            https://www.w3.org/TR/CSP2/#directive-connect-src (W3C Candidate Recommendation, 21 July 2015)
>+
> 2016-04-07  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         Unreviewed, rolling out r199199.
>Index: Source/WebCore/xml/XMLHttpRequest.cpp
>===================================================================
>--- Source/WebCore/xml/XMLHttpRequest.cpp	(revision 199114)
>+++ Source/WebCore/xml/XMLHttpRequest.cpp	(working copy)
>@@ -497,13 +497,6 @@ void XMLHttpRequest::open(const String& 
>         return;
>     }
> 
>-    // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
>-    if (!scriptExecutionContext()->contentSecurityPolicy()->allowConnectToSource(url, scriptExecutionContext()->shouldBypassMainWorldContentSecurityPolicy())) {
>-        // FIXME: Should this be throwing an exception?
>-        ec = SECURITY_ERR;
>-        return;
>-    }
>-
>     if (!async && scriptExecutionContext()->isDocument()) {
>         if (document()->settings() && !document()->settings()->syncXHRInDocumentsEnabled()) {
>             logConsoleError(scriptExecutionContext(), "Synchronous XMLHttpRequests are disabled for this page.");
>@@ -573,6 +566,17 @@ bool XMLHttpRequest::initSend(ExceptionC
>     }
>     ASSERT(!m_loader);
> 
>+    // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
>+    if (!scriptExecutionContext()->contentSecurityPolicy()->allowConnectToSource(m_url, scriptExecutionContext()->shouldBypassMainWorldContentSecurityPolicy())) {
>+        if (m_async) {
>+            setPendingActivity(this);
>+            m_timeoutTimer.stop();
>+            m_networkErrorTimer.startOneShot(0);
>+        } else
>+            ec = NETWORK_ERR;
>+        return false;
>+    }
>+
>     m_error = false;
>     return true;
> }
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 199114)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,31 @@
>+2016-04-07  John Wilander  <wilander@apple.com>
>+
>+        CSP: Block XHR when calling XMLHttpRequest.send() and throw network error.
>+        https://bugs.webkit.org/show_bug.cgi?id=153598
>+        <rdar://problem/24391483>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/workers/resources/worker-inherits-csp-blocks-xhr.js:
>+        (catch):
>+        * fast/workers/worker-inherits-csp-blocks-xhr-expected.txt:
>+            Changed expected error from DOMException.SECURITY_ERR to DOMException.NETWORK_ERR.
>+        * http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked-expected.txt:
>+        * http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked.html:
>+            Now tests that XMLHttpRequest.send() is blocked if the URL voilates the connect-src directive in CSP.
>+        * http/tests/security/contentSecurityPolicy/resources/worker.php:
>+            Added two additional calls to XMLHttpRequest.send() and switched to receiving an error event to make
>+            existing tests work with code changes.
>+        * http/tests/security/contentSecurityPolicy/source-list-parsing-malformed-meta.html:
>+            Added an additional call to XMLHttpRequest.send() and switched to receiving an error event to make 
>+            existing test work with code changes.
>+        * http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr-expected.txt:
>+        * http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr.html:
>+            Added an additional call to XMLHttpRequest.send() and switched to receiving an error event to make 
>+            existing tests work with code changes.
>+            Refactored test mechnism with additional parameters to cover synchronous/asynchronous as well as 
>+            same-origin/cross-origin in isolated worlds.
>+
> 2016-04-06  Sam Weinig  <sam@webkit.org>
> 
>         window.CSS should be a constructor with static functions
>Index: LayoutTests/fast/workers/worker-inherits-csp-blocks-xhr-expected.txt
>===================================================================
>--- LayoutTests/fast/workers/worker-inherits-csp-blocks-xhr-expected.txt	(revision 199114)
>+++ LayoutTests/fast/workers/worker-inherits-csp-blocks-xhr-expected.txt	(working copy)
>@@ -1,4 +1,4 @@
> CONSOLE MESSAGE: Refused to connect to non-existent-file because it does not appear in the connect-src directive of the Content Security Policy.
> This tests that the Content Security Policy (CSP) of the owner document (this page) blocks a file-URL Web Worker from making an XHR request because the parent's CSP contains "connect-src 'none'"
> 
>-PASS threw exception Error: SecurityError: DOM Exception 18.
>+PASS threw exception Error: NetworkError: DOM Exception 19.
>Index: LayoutTests/fast/workers/resources/worker-inherits-csp-blocks-xhr.js
>===================================================================
>--- LayoutTests/fast/workers/resources/worker-inherits-csp-blocks-xhr.js	(revision 199114)
>+++ LayoutTests/fast/workers/resources/worker-inherits-csp-blocks-xhr.js	(working copy)
>@@ -7,8 +7,8 @@ try {
> } catch (e) {
>     exception = e;
> }
>-// FIXME: We should be throwing a DOMException.NETWORK_ERR. See <https://bugs.webkit.org/show_bug.cgi?id=153598>.
>-var expectedExceptionCode = 18; // DOMException.SECURITY_ERR
>+
>+var expectedExceptionCode = 19; // DOMException.NETWORK_ERR
> if (!exception)
>     self.postMessage("FAIL should throw " + expectedExceptionCode + ". But did not throw an exception.");
> else {
>Index: LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked-expected.txt
>===================================================================
>--- LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked-expected.txt	(revision 199114)
>+++ LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked-expected.txt	(working copy)
>@@ -1,3 +1,20 @@
> CONSOLE MESSAGE: Refused to connect to http://localhost:8000/xmlhttprequest/resources/get.txt because it does not appear in the connect-src directive of the Content Security Policy.
>-Pass
>+CONSOLE MESSAGE: Refused to connect to http://localhost:8000/xmlhttprequest/resources/get.txt because it does not appear in the connect-src directive of the Content Security Policy.
>+CONSOLE MESSAGE: Refused to connect to http://localhost:8000/xmlhttprequest/resources/get.txt because it does not appear in the connect-src directive of the Content Security Policy.
>+This tests that a Content Security Policy violation for an XHR is triggered when calling XMLHttpRequest.send().
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS xhrSync.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", false) did not throw exception.
>+PASS xhrSync.send() threw exception Error: NetworkError: DOM Exception 19.
>+PASS xhrAsync.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true) did not throw exception.
>+PASS xhrAsync.send() did not throw exception.
>+PASS xhrAsyncAbort.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true) did not throw exception.
>+PASS xhrAsyncAbort.send();xhrAsyncAbort.abort();  did not throw exception.
>+PASS An error event was received for the asynchronous call.
>+PASS An error event was received for the aborted asynchronous call.
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
> 
>Index: LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked.html
>===================================================================
>--- LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked.html	(revision 199114)
>+++ LayoutTests/http/tests/security/contentSecurityPolicy/connect-src-xmlhttprequest-blocked.html	(working copy)
>@@ -2,27 +2,40 @@
> <html>
> <head>
> <meta http-equiv="Content-Security-Policy" content="connect-src http://127.0.0.1:8000">
>-<script>
>-if (window.testRunner)
>-    testRunner.dumpAsText();
>-</script>
>+<script src="../../../resources/js-test-pre.js"></script>
> </head>
> <body>
>-<pre id="console"></pre>
> <script>
>-function log(msg)
>-{
>-    document.getElementById("console").appendChild(document.createTextNode(msg + "\n"));
>-}
>-
>-try {
>-    var xhr = new XMLHttpRequest;
>-    xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true); 
>-    log("Fail");
>-} catch(e) {
>-    log("Pass");
>-}
>+description("This tests that a Content Security Policy violation for an XHR is triggered when calling XMLHttpRequest.send().");
>+
>+jsTestIsAsync = true;
>+
>+var xhrSync = new XMLHttpRequest;
>+xhrSync.addEventListener("error", function () {
>+    debug("FAIL An error event should not have been received.");
>+});
>+
>+var xhrAsync = new XMLHttpRequest;
>+xhrAsync.addEventListener("error", function () {
>+    debug("PASS An error event was received for the asynchronous call.");
>+});
>+
>+var xhrAsyncAbort = new XMLHttpRequest;
>+xhrAsyncAbort.addEventListener("error", function () {
>+    debug("PASS An error event was received for the aborted asynchronous call.");
>+    finishJSTest();
>+});
>+
>+shouldNotThrow('xhrSync.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", false)'); // Synchronous request
>+shouldThrow("xhrSync.send()", "'Error: NetworkError: DOM Exception 19'");
>+
>+shouldNotThrow('xhrAsync.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true)'); // Asynchronous request
>+shouldNotThrow("xhrAsync.send()");
>+
>+shouldNotThrow('xhrAsyncAbort.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true)'); // Asynchronous request
>+shouldNotThrow("xhrAsyncAbort.send();xhrAsyncAbort.abort(); ");
> 
> </script>
>+<script src="/js-test-resources/js-test-post.js"></script>
> </body>
> </html>
>Index: LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-malformed-meta.html
>===================================================================
>--- LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-malformed-meta.html	(revision 199114)
>+++ LayoutTests/http/tests/security/contentSecurityPolicy/source-list-parsing-malformed-meta.html	(working copy)
>@@ -17,7 +17,8 @@ function log(msg)
> 
> try {
>     var xhr = new XMLHttpRequest;
>-    xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true); 
>+    xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", false);
>+    xhr.send();
>     log("Fail");
> } catch(e) {
>     log("Pass");
>Index: LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php
>===================================================================
>--- LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php	(revision 199114)
>+++ LayoutTests/http/tests/security/contentSecurityPolicy/resources/worker.php	(working copy)
>@@ -58,13 +58,15 @@ try {
> } else if ($_GET["type"] == "make-xhr") {
> ?>
> 
>-try {
>-    var xhr = new XMLHttpRequest;
>-    xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+var xhr = new XMLHttpRequest;
>+xhr.addEventListener("load", function () {
>     postMessage("xhr allowed");
>-} catch(e) {
>+});
>+xhr.addEventListener("error", function () {
>     postMessage("xhr blocked");
>-}
>+});
>+xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+xhr.send();
> 
> <?php
> } else if ($_GET["type"] == "set-timeout") {
>@@ -115,13 +117,15 @@ onconnect = function (e) {
> } else if ($_GET["type"] == "multiple-headers") {
> ?>
> 
>-try {
>-    var xhr = new XMLHttpRequest;
>-    xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+var xhr = new XMLHttpRequest;
>+xhr.addEventListener("load", function () {
>     postMessage("xhr allowed");
>-} catch(e) {
>+});
>+xhr.addEventListener("error", function () {
>     postMessage("xhr blocked");
>-}
>+});
>+xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+xhr.send();
> 
> var id = 0;
> try {
>Index: LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr-expected.txt
>===================================================================
>--- LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr-expected.txt	(revision 199114)
>+++ LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr-expected.txt	(working copy)
>@@ -1,13 +1,25 @@
>-CONSOLE MESSAGE: Refused to connect to http://localhost:8000/security/isolatedWorld/resources/cross-origin-xhr.txt because it does not appear in the connect-src directive of the Content Security Policy.
>+CONSOLE MESSAGE: Refused to connect to http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi because it does not appear in the connect-src directive of the Content Security Policy.
>+CONSOLE MESSAGE: Refused to connect to http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi because it does not appear in the connect-src directive of the Content Security Policy.
> Tests that isolated worlds can have XHRs that the page's CSP wouldn't allow.
> 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> 
> 
>-XHR from main world
>-PASS: XHR.open threw an exception.
>-XHR from isolated world
>-PASS: XHR.open did not throw an exception.
>+Synchronous XHR same-origin from main world
>+PASS: XHR.send threw an exception.
>+Asynchronous XHR same-origin from main world
>+PASS: XHR.send did not throw an exception.
>+PASS: XHR.send received an error event.
>+Synchronous XHR same-origin from isolated world
>+PASS: XHR.send did not throw an exception.
>+Asynchronous XHR same-origin from isolated world
>+PASS: XHR.send did not throw an exception.
>+PASS: XHR.send received a load event.
>+Synchronous XHR cross-origin from isolated world
>+PASS: XHR.send did not throw an exception.
>+Asynchronous XHR cross-origin from isolated world
>+PASS: XHR.send did not throw an exception.
>+PASS: XHR.send received a load event.
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>Index: LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr.html
>===================================================================
>--- LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr.html	(revision 199114)
>+++ LayoutTests/http/tests/security/isolatedWorld/bypass-main-world-csp-for-xhr.html	(working copy)
>@@ -1,7 +1,9 @@
> <!DOCTYPE html>
> <html>
>+<head>
>     <script src="../../js-test-resources/js-test-pre.js"></script>
>     <meta http-equiv="Content-Security-Policy" content="connect-src 'none'">
>+</head>
> <body>
> <p id="description"></p>
> <div id="console"></div>
>@@ -11,15 +13,38 @@ description('Tests that isolated worlds 
> 
> jsTestIsAsync = true;
> 
>+const SameOrigin = true;
>+const CrossOrigin = false;
>+const Asynchronous = true;
>+const Synchronous = false;
>+const ShouldBlock = true;
>+const ShouldNotBlock = false;
>+
> var tests = [
>     function() {
>-        debug('XHR from main world');
>-        xhr(true);
>+        debug('Synchronous XHR same-origin from main world');
>+        xhr({sameOrigin : true}, Synchronous, ShouldBlock);
>+    },
>+    function() {
>+        debug('Asynchronous XHR same-origin from main world');
>+        xhr({sameOrigin : true}, Asynchronous, ShouldBlock);
>+    },
>+    function() {
>+        debug('Synchronous XHR same-origin from isolated world');
>+        invokeInWorld(1, xhr, SameOrigin, Synchronous, ShouldNotBlock);
>+    },
>+    function() {
>+        debug('Asynchronous XHR same-origin from isolated world');
>+        invokeInWorld(2, xhr, SameOrigin, Asynchronous, ShouldNotBlock);
>     },
>     function() {
>-        debug('XHR from isolated world');
>-        runTestInWorld(1, 'xhr', 'false');
>+        debug('Synchronous XHR cross-origin from isolated world');
>+        invokeInWorld(3, xhr, CrossOrigin, Synchronous, ShouldNotBlock);
>     },
>+    function() {
>+        debug('Asynchronous XHR cross-origin from isolated world');
>+        invokeInWorld(4, xhr, CrossOrigin, Asynchronous, ShouldNotBlock);
>+    }
> ];
> var currentTest = 0;
> 
>@@ -52,14 +77,15 @@ if (window.testRunner) {
>     testFailed('Test depends on LayoutTestController and must be run by DRT');
> }
> 
>-function runTestInWorld(worldId, funcName, param)
>-{
>-    testRunner.evaluateScriptInIsolatedWorld(
>-        worldId, String(eval(funcName)) + "\n" + funcName + "(" + param + ");");
>+// This function will only successfully pass on JSON-stringifieable arguments such as numbers and strings to aNamedFunction
>+function invokeInWorld(worldId, aNamedFunction) {
>+    console.assert(aNamedFunction.name);
>+    var argumentsToPass = Array.prototype.slice.call(arguments, 2);
>+    var script = aNamedFunction.toString() + '; ' + aNamedFunction.name + '(' + argumentsToPass.map(JSON.stringify).join(', ') + ');';
>+    testRunner.evaluateScriptInIsolatedWorld(worldId, script);
> }
> 
>-function xhr(shouldBlock)
>-{
>+function xhr(isSameOrigin, isAsync, shouldBlock) {
>     function debug(message) {
>         window.postMessage(JSON.stringify({
>                 'type': 'debug',
>@@ -68,25 +94,58 @@ function xhr(shouldBlock)
>             '*');
>     }
> 
>+    var url = (isSameOrigin ? 'http://127.0.0.1:8000/' : 'http://localhost:8000/') + 'xmlhttprequest/resources/access-control-basic-allow.cgi';
>     var xhr = new XMLHttpRequest();
>+    var asyncCallDone = false;
>+    var finallyClauseDone = false;
>+
>+    xhr.open('GET', url, isAsync);
>+
>+    if (isAsync) {
>+        xhr.addEventListener('load', function() {
>+            if (shouldBlock)
>+                debug('FAIL: XHR.send should not have received a load event.');
>+            else
>+                debug('PASS: XHR.send received a load event.');
>+
>+            if (finallyClauseDone)
>+                window.postMessage(JSON.stringify({'type': 'test-done'}), '*');
>+            else
>+                asyncCallDone = true;
>+        });
>+
>+        xhr.addEventListener('error', function() {
>+            if (shouldBlock)
>+                debug('PASS: XHR.send received an error event.');
>+            else
>+                debug('FAIL: XHR.send should not have received an error event.');
>+
>+            if (finallyClauseDone)
>+                window.postMessage(JSON.stringify({'type': 'test-done'}), '*');
>+            else
>+                asyncCallDone = true;
>+        });
>+    }
>+
>     try {
>-        xhr.open('GET', 'http://localhost:8000/security/isolatedWorld/resources/cross-origin-xhr.txt', true);
>-        if (shouldBlock)
>-            debug('FAIL: XHR.open should have thrown an exception.');
>+        xhr.send();
>+        if (shouldBlock && !isAsync)
>+            debug('FAIL: XHR.send should have thrown an exception.');
>         else
>-            debug('PASS: XHR.open did not throw an exception.');
>+            debug('PASS: XHR.send did not throw an exception.');
>     } catch (e) {
>-        if (shouldBlock)
>-            debug('PASS: XHR.open threw an exception.');
>+        if (shouldBlock && !isAsync)
>+            debug('PASS: XHR.send threw an exception.');
>         else
>-            debug('FAIL: XHR.open should not have thrown an exception.');
>+            debug('FAIL: XHR.send should not have thrown an exception.');
>     } finally {
>-        window.postMessage(JSON.stringify({'type': 'test-done'}), '*');
>+        if (!isAsync || asyncCallDone)
>+            window.postMessage(JSON.stringify({'type': 'test-done'}), '*');
>+        else
>+            finallyClauseDone = true;
>     }
> }
>-
> </script>
>-
> <script src="../../js-test-resources/js-test-post.js"></script>
> </body>
> </html>

Actions: View | Formatted Diff | Diff

Attachments on bug 153598: 275361 | 275362 | 275363 | 275365 | 275366 | 275413 | 275417 | 275431 | 275555 | 275557 | 275629 | 275928 | 275933 | 275966