Attachment 275933 Details for Bug 153598 – Patch

[patch] Patch

bug-153598-20160407140942.patch (text/plain), 17.46 KB, created by John Wilander on 2016-04-07 14:09:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: John Wilander

Created: 2016-04-07 14:09:38 PDT

Size: 17.46 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 199176)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,21 @@
>+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()
>+            and changed the error type from Security to Network.
>+            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-05  Ada Chan  <adachan@apple.com>
> 
>         Rename TextTrackRepresentationiOS to TextTrackRepresentationCocoa and enable on Mac
>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,13 @@ 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.
>+    // FIXME: We should only throw an exception for a synchronous request and should dispatch a DOM error event so as to make this CSP violation indistinguishable from a XHR network error. See <https://bugs.webkit.org/show_bug.cgi?id=156322>.
>+    if (!scriptExecutionContext()->contentSecurityPolicy()->allowConnectToSource(m_url, scriptExecutionContext()->shouldBypassMainWorldContentSecurityPolicy())) {
>+        ec = NETWORK_ERR;
>+        return false;
>+    }
>+
>     m_error = false;
>     return true;
> }
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 199114)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+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() to make existing tests work with the changes.
>+        * http/tests/security/contentSecurityPolicy/source-list-parsing-malformed-meta.html:
>+            Added an additional call to XMLHttpRequest.send() to make existing test work with the 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() to make existing test work with the changes.
>+            Refactored test mechnism with additional parameters for various test scenarios (see below).
>+            Now tests 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,15 @@
> 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.
>+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 xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true) did not throw exception.
>+PASS xhr.send() threw exception Error: NetworkError: DOM Exception 19.
>+PASS xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", false) did not throw exception.
>+PASS xhr.send() threw exception Error: NetworkError: DOM Exception 19.
>+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,21 @@
> <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"));
>-}
>+description("This tests that a Content Security Policy violation for an XHR is triggered when calling XMLHttpRequest.send().");
>+
>+var xhr = new XMLHttpRequest;
>+
>+shouldNotThrow('xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true)'); // Asynchronous request
>+shouldThrow("xhr.send()", "'Error: NetworkError: DOM Exception 19'");
> 
>-try {
>-    var xhr = new XMLHttpRequest;
>-    xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", true); 
>-    log("Fail");
>-} catch(e) {
>-    log("Pass");
>-}
>+shouldNotThrow('xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/get.txt", false)'); // Synchronous request
>+shouldThrow("xhr.send()", "'Error: NetworkError: DOM Exception 19'");
> 
> </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", true);
>+    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)
>@@ -61,6 +61,7 @@ try {
> try {
>     var xhr = new XMLHttpRequest;
>     xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+    xhr.send();
>     postMessage("xhr allowed");
> } catch(e) {
>     postMessage("xhr blocked");
>@@ -118,6 +119,7 @@ onconnect = function (e) {
> try {
>     var xhr = new XMLHttpRequest;
>     xhr.open("GET", "http://127.0.0.1:8000/xmlhttprequest/resources/get.txt", true);
>+    xhr.send();
>     postMessage("xhr allowed");
> } catch(e) {
>     postMessage("xhr blocked");
>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,21 @@
>-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.
> 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.
>+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)
>@@ -11,15 +11,39 @@ 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);
>+    },
>+// FIXME: Add asynchronous same-origin test when https://bugs.webkit.org/show_bug.cgi?id=153155 is done.
>+//    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('XHR from isolated world');
>-        runTestInWorld(1, 'xhr', 'false');
>+        debug('Asynchronous XHR same-origin from isolated world');
>+        invokeInWorld(2, xhr, SameOrigin, Asynchronous, ShouldNotBlock);
>     },
>+    function() {
>+        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 +76,17 @@ 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) {
>+    if (!aNamedFunction.name) {
>+        throw new Error("invokeInWorld requires a named function.");
>+    }
>+    var slice = Function.prototype.call.bind([].slice);
>+    var script = aNamedFunction.toString() + "; " + aNamedFunction.name + "(" + slice(arguments, 2).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 +95,46 @@ function xhr(shouldBlock)
>             '*');
>     }
> 
>-    var xhr = new XMLHttpRequest();
>+    var url = (isSameOrigin ? "http://127.0.0.1:8000/" : "http://localhost:8000/") + "xmlhttprequest/resources/access-control-basic-allow.cgi",
>+            xhr = new XMLHttpRequest(),
>+            asyncCallDone = false, 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;
>+        });
>+
>+        // FIXME: Add event listener for error when https://bugs.webkit.org/show_bug.cgi?id=153155 is done.
>+    }
>+
>     try {
>-        xhr.open('GET', 'http://localhost:8000/security/isolatedWorld/resources/cross-origin-xhr.txt', true);
>+        xhr.send();
>         if (shouldBlock)
>-            debug('FAIL: XHR.open should have thrown an exception.');
>+            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.');
>+            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