Setting a frame's src to a javascript URL should not run it synchronously
https://bugs.webkit.org/show_bug.cgi?id=197466
Reviewed by Darin Adler.
Source/WebCore:
When an iframe's src attribute is set to a javascript URL, whether when parsing
or later on via JS, we now execute the URL's JavaScript asynchronously. We used
to execute it synchronously, which was a source of bugs and also did not match
other browsers.
I have verified that our new behavior is aligned with both Firefox and Chrome.
Note that for backward-compatibility and interoperability with Blink
(https://bugs.chromium.org/p/chromium/issues/detail?id=923585), the
"javascript:''" URL will still run synchronously. We should consider dropping
this quirk at some point.
Test: fast/dom/frame-src-javascript-url-async.html
* loader/NavigationScheduler.cpp:
(WebCore::ScheduledLocationChange::ScheduledLocationChange):
(WebCore::ScheduledLocationChange::~ScheduledLocationChange):
(WebCore::NavigationScheduler::scheduleLocationChange):
* loader/NavigationScheduler.h:
(WebCore::NavigationScheduler::scheduleLocationChange):
* loader/SubframeLoader.cpp:
(WebCore::SubframeLoader::requestFrame):
LayoutTests:
* fast/dom/frame-src-javascript-url-async-expected.txt: Added.
* fast/dom/frame-src-javascript-url-async.html: Added.
Add layout test coverage for the fact that the javascript URL is executed asynchronously
whether set during parsing or later via JS. Also makes sure that executing the javascript
URL asynchronously does not replace the frame's window. This test passes in both Chrome
and Firefox.
* imported/blink/fast/frames/navigation-in-pagehide.html:
Re-sync this test from the Blink repository.
* fast/dom/Element/id-in-frameset-expected.txt:
* fast/dom/Element/id-in-frameset.html:
* fast/dom/insertedIntoDocument-iframe-expected.txt:
* fast/dom/javascript-url-exception-isolation-expected.txt:
* fast/dom/javascript-url-exception-isolation.html:
* fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt:
* fast/dom/resources/javascript-url-crash-function-iframe.html:
* fast/frames/adopt-from-created-document.html:
* fast/frames/out-of-document-iframe-has-child-frame.html:
* fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html:
* fast/loader/javascript-url-iframe-remove-on-navigate.html:
* fast/loader/unload-mutation-crash.html:
* fast/parser/resources/set-parent-to-javascript-url.html:
* fast/parser/xml-error-adopted.xml:
* http/tests/navigation/lockedhistory-iframe-expected.txt:
* http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt:
* http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt:
* http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt:
* http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt:
* http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html:
* http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html:
* http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html:
* imported/blink/loader/iframe-sync-loads-expected.txt:
* js/dom/call-base-resolution.html:
* platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt:
Update / Rebaseline existing tests to reflect behavior change. I ran those tests in Firefox and Chrome to confirm that our behavior
is indeed aligned.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244892 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index b22ab65..4796a18 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,48 @@
+2019-05-02 Chris Dumez <cdumez@apple.com>
+
+ Setting a frame's src to a javascript URL should not run it synchronously
+ https://bugs.webkit.org/show_bug.cgi?id=197466
+
+ Reviewed by Darin Adler.
+
+ * fast/dom/frame-src-javascript-url-async-expected.txt: Added.
+ * fast/dom/frame-src-javascript-url-async.html: Added.
+ Add layout test coverage for the fact that the javascript URL is executed asynchronously
+ whether set during parsing or later via JS. Also makes sure that executing the javascript
+ URL asynchronously does not replace the frame's window. This test passes in both Chrome
+ and Firefox.
+
+ * imported/blink/fast/frames/navigation-in-pagehide.html:
+ Re-sync this test from the Blink repository.
+
+ * fast/dom/Element/id-in-frameset-expected.txt:
+ * fast/dom/Element/id-in-frameset.html:
+ * fast/dom/insertedIntoDocument-iframe-expected.txt:
+ * fast/dom/javascript-url-exception-isolation-expected.txt:
+ * fast/dom/javascript-url-exception-isolation.html:
+ * fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt:
+ * fast/dom/resources/javascript-url-crash-function-iframe.html:
+ * fast/frames/adopt-from-created-document.html:
+ * fast/frames/out-of-document-iframe-has-child-frame.html:
+ * fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html:
+ * fast/loader/javascript-url-iframe-remove-on-navigate.html:
+ * fast/loader/unload-mutation-crash.html:
+ * fast/parser/resources/set-parent-to-javascript-url.html:
+ * fast/parser/xml-error-adopted.xml:
+ * http/tests/navigation/lockedhistory-iframe-expected.txt:
+ * http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt:
+ * http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt:
+ * http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt:
+ * http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt:
+ * http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html:
+ * http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html:
+ * http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html:
+ * imported/blink/loader/iframe-sync-loads-expected.txt:
+ * js/dom/call-base-resolution.html:
+ * platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt:
+ Update / Rebaseline existing tests to reflect behavior change. I ran those tests in Firefox and Chrome to confirm that our behavior
+ is indeed aligned.
+
2019-05-02 Gary Katsevman <git@gkatsev.com>
WebVTT: vertical cue text alignment is the wrong way around
diff --git a/LayoutTests/fast/dom/Element/id-in-frameset-expected.txt b/LayoutTests/fast/dom/Element/id-in-frameset-expected.txt
index b3db1a3..a71080b 100644
--- a/LayoutTests/fast/dom/Element/id-in-frameset-expected.txt
+++ b/LayoutTests/fast/dom/Element/id-in-frameset-expected.txt
@@ -1,2 +1,2 @@
-ALERT: 1
+ALERT: 2
diff --git a/LayoutTests/fast/dom/Element/id-in-frameset.html b/LayoutTests/fast/dom/Element/id-in-frameset.html
index 7916535..9b63203 100644
--- a/LayoutTests/fast/dom/Element/id-in-frameset.html
+++ b/LayoutTests/fast/dom/Element/id-in-frameset.html
@@ -1,5 +1,10 @@
<html>
-
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+</script>
<frameset id="frameset">
<frame name="frame2" src="about:blank">
<frame name="frame1" src="javascript:
@@ -16,6 +21,8 @@
top.frameset.removeChild(top.frame2.frameElement);
log(top.frameset.children.length);
+ if (window.testRunner)
+ testRunner.notifyDone();
">
<frame name="frame3" src="about:blank">
diff --git a/LayoutTests/fast/dom/frame-src-javascript-url-async-expected.txt b/LayoutTests/fast/dom/frame-src-javascript-url-async-expected.txt
new file mode 100644
index 0000000..3fb2066
--- /dev/null
+++ b/LayoutTests/fast/dom/frame-src-javascript-url-async-expected.txt
@@ -0,0 +1,21 @@
+Checks that setting an iframe's src attribute to a javascript URL runs the javascript asynchronously
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS frame2.contentWindow is initialFrame2Window
+PASS frame2.contentDocument is initialFrame2Document
+PASS messages is "1234"
+PASS frame1.contentWindow is initialFrame1Window
+PASS frame1.contentDocument is initialFrame1Document
+PASS frame2.contentWindow is initialFrame2Window
+PASS frame2.contentDocument is initialFrame2Document
+PASS frame3.contentWindow is initialFrame3Window
+PASS frame3.contentDocument is not initialFrame3Document
+PASS frame3.contentWindow is initialFrame3Window
+PASS frame3.contentDocument is not initialFrame3Document
+PASS frame3.contentDocument.documentElement.textContent is "1"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/frame-src-javascript-url-async.html b/LayoutTests/fast/dom/frame-src-javascript-url-async.html
new file mode 100644
index 0000000..215dc40
--- /dev/null
+++ b/LayoutTests/fast/dom/frame-src-javascript-url-async.html
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+description("Checks that setting an iframe's src attribute to a javascript URL runs the javascript asynchronously");
+jsTestIsAsync = true;
+
+let messages = "";
+const expectedMessageCount = 4;
+function log(msg)
+{
+ messages += msg;
+ if (messages.length == expectedMessageCount) {
+ shouldBeEqualToString("messages", "1234");
+ shouldBe("frame1.contentWindow", "initialFrame1Window");
+ shouldBe("frame1.contentDocument", "initialFrame1Document");
+ shouldBe("frame2.contentWindow", "initialFrame2Window");
+ shouldBe("frame2.contentDocument", "initialFrame2Document");
+ shouldBe("frame3.contentWindow", "initialFrame3Window");
+ // Firefox 66 and Chrome 74 disagree here, we match Chrome.
+ shouldNotBe("frame3.contentDocument", "initialFrame3Document");
+ setTimeout(() => {
+ shouldBe("frame3.contentWindow", "initialFrame3Window");
+ shouldNotBe("frame3.contentDocument", "initialFrame3Document");
+ shouldBeEqualToString("frame3.contentDocument.documentElement.textContent", "1");
+ finishJSTest();
+ }), 0;
+ }
+}
+</script>
+<iframe id="frame1" src="javascript:parent.log('3')"></iframe>
+<iframe id="frame2"></iframe>
+<iframe id="frame3" src="javascript:'1'"></iframe>
+<script>
+frame1 = document.getElementById("frame1");
+frame2 = document.getElementById("frame2");
+frame3 = document.getElementById("frame3");
+initialFrame1Window = frame1.contentWindow;
+initialFrame1Document = frame1.contentDocument;
+initialFrame2Window = frame2.contentWindow;
+initialFrame2Document = frame2.contentDocument;
+initialFrame3Window = frame3.contentWindow;
+initialFrame3Document = frame3.contentDocument;
+log('1');
+frame2.src = "javascript:parent.log('4')";
+shouldBe("frame2.contentWindow", "initialFrame2Window");
+shouldBe("frame2.contentDocument", "initialFrame2Document");
+log('2');
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/insertedIntoDocument-iframe-expected.txt b/LayoutTests/fast/dom/insertedIntoDocument-iframe-expected.txt
index 7ef22e9..6e1f6df 100644
--- a/LayoutTests/fast/dom/insertedIntoDocument-iframe-expected.txt
+++ b/LayoutTests/fast/dom/insertedIntoDocument-iframe-expected.txt
@@ -1 +1,3 @@
+CONSOLE MESSAGE: line 1: TypeError: Argument 1 ('child') to Node.removeChild must be an instance of Node
PASS
+
diff --git a/LayoutTests/fast/dom/javascript-url-exception-isolation-expected.txt b/LayoutTests/fast/dom/javascript-url-exception-isolation-expected.txt
index 51b3435..b7485a1 100644
--- a/LayoutTests/fast/dom/javascript-url-exception-isolation-expected.txt
+++ b/LayoutTests/fast/dom/javascript-url-exception-isolation-expected.txt
@@ -1,5 +1,5 @@
CONSOLE MESSAGE: line 1: 42
-CONSOLE MESSAGE: line 25: SyntaxError: Unexpected token '<'
+CONSOLE MESSAGE: line 1: SyntaxError: Unexpected token '<'
Exceptions thrown in javascript URLs should not propagate to the main script.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
diff --git a/LayoutTests/fast/dom/javascript-url-exception-isolation.html b/LayoutTests/fast/dom/javascript-url-exception-isolation.html
index 6f39b2c..66dad1c 100644
--- a/LayoutTests/fast/dom/javascript-url-exception-isolation.html
+++ b/LayoutTests/fast/dom/javascript-url-exception-isolation.html
@@ -20,9 +20,12 @@
}
shouldBeFalse('caughtException');
+var subframe2 = document.createElement("iframe");
+document.body.appendChild(subframe2);
+
// Compile-time exception.
try {
- subframe.src = 'javascript:<html></html>';
+ subframe2.src = 'javascript:<html></html>';
} catch(e) {
caughtException = true;
}
diff --git a/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt b/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt
index e96375a..2cb8efa 100644
--- a/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt
+++ b/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 14: SyntaxError: Unexpected identifier 'orem'
+CONSOLE MESSAGE: line 1: SyntaxError: Unexpected identifier 'orem'
This tests that we do not assert when a malformed JS URL is passed to the 'src' attribute of an iframe. The test passes if it does not ASSERT.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
diff --git a/LayoutTests/fast/dom/resources/javascript-url-crash-function-iframe.html b/LayoutTests/fast/dom/resources/javascript-url-crash-function-iframe.html
index 1d6a49d..0a0154c4 100644
--- a/LayoutTests/fast/dom/resources/javascript-url-crash-function-iframe.html
+++ b/LayoutTests/fast/dom/resources/javascript-url-crash-function-iframe.html
@@ -16,7 +16,9 @@
setTimeout(function ()
{
test();
- if (window.testRunner)
- testRunner.notifyDone();
+ top.setTimeout(() => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
}, 0);
</script>
diff --git a/LayoutTests/fast/frames/adopt-from-created-document.html b/LayoutTests/fast/frames/adopt-from-created-document.html
index 2a9829b..33855d5 100644
--- a/LayoutTests/fast/frames/adopt-from-created-document.html
+++ b/LayoutTests/fast/frames/adopt-from-created-document.html
@@ -8,10 +8,10 @@
alert(2);
var ifr = doc.createElement('iframe');
alert(3);
-ifr.setAttribute('src', 'javascript:alert(6)');
+ifr.setAttribute('src', 'javascript:alert(7)');
alert(4);
var adopted = document.adoptNode(ifr)
alert(5);
document.body.appendChild(adopted);
-alert(7);
+alert(6);
</script>
diff --git a/LayoutTests/fast/frames/out-of-document-iframe-has-child-frame.html b/LayoutTests/fast/frames/out-of-document-iframe-has-child-frame.html
index e9e576b..8db38d7 100644
--- a/LayoutTests/fast/frames/out-of-document-iframe-has-child-frame.html
+++ b/LayoutTests/fast/frames/out-of-document-iframe-has-child-frame.html
@@ -1,12 +1,13 @@
<html>
<head>
-<script src="../../resources/js-test-pre.js"></script>
+<script src="../../resources/js-test.js"></script>
</head>
<body>
<div id="main"/>
<script>
description("This tests that several ways of making an iframe that isn't inserted into a document tree"
+ " but has a child frame will fail.");
+jsTestIsAsync = true;
main = document.getElementById("main");
@@ -44,9 +45,11 @@
helperFrame.src = "javascript:top.container.removeChild(top.targetFrame3)";
document.body.appendChild(container);
} catch (e) { }
-shouldBeTrue("targetFrame3.contentWindow == undefined");
-isSuccessfullyParsed();
+setTimeout(() => {
+ shouldBeTrue("targetFrame3.contentWindow == undefined");
+ finishJSTest();
+}, 0);
</script>
</body>
</html>
diff --git a/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html b/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html
index a3f633c..73fcd76 100644
--- a/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html
+++ b/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate-async-delegate.html
@@ -8,13 +8,14 @@
}
let frame = document.getElementById("target");
-frame.contentWindow.onbeforeunload = function() {
- setTimeout(function() {
- frame.src = "javascript:alert('FAIL')";
- }, 0);
-};
window.addEventListener("load", function() {
+ frame.contentWindow.onbeforeunload = function() {
+ setTimeout(function() {
+ frame.src = "javascript:alert('FAIL')";
+ }, 0);
+ };
+
document.write("PASS - Javascript URL blocked without crashing.");
if (window.testRunner)
testRunner.notifyDone();
diff --git a/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate.html b/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate.html
index af3764d..2594457 100644
--- a/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate.html
+++ b/LayoutTests/fast/loader/javascript-url-iframe-remove-on-navigate.html
@@ -6,13 +6,13 @@
}
let frame = document.getElementById("target");
-frame.contentWindow.onbeforeunload = function() {
- setTimeout(function() {
- frame.src = "javascript:alert('FAIL')";
- }, 0);
-};
window.addEventListener("load", function() {
+ frame.contentWindow.onbeforeunload = function() {
+ setTimeout(function() {
+ frame.src = "javascript:alert('FAIL')";
+ }, 0);
+ };
document.write("PASS - Javascript URL blocked without crashing.");
if (window.testRunner)
testRunner.notifyDone();
diff --git a/LayoutTests/fast/loader/unload-mutation-crash.html b/LayoutTests/fast/loader/unload-mutation-crash.html
index 246fe16..c78db43 100644
--- a/LayoutTests/fast/loader/unload-mutation-crash.html
+++ b/LayoutTests/fast/loader/unload-mutation-crash.html
@@ -2,8 +2,10 @@
<html>
<head>
<script>
-if (window.testRunner)
- window.testRunner.dumpAsText();
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
function start() {
window.firstFrame = document.createElement('iframe');
@@ -20,6 +22,8 @@
window.firstFrame.src = 'javascript:"";';
document.write("PASS. WebKit didn't crash.");
+ if (window.testRunner)
+ testRunner.notifyDone();
}
</script>
</head>
diff --git a/LayoutTests/fast/parser/resources/set-parent-to-javascript-url.html b/LayoutTests/fast/parser/resources/set-parent-to-javascript-url.html
index 71d78bf..4f1ebdd 100644
--- a/LayoutTests/fast/parser/resources/set-parent-to-javascript-url.html
+++ b/LayoutTests/fast/parser/resources/set-parent-to-javascript-url.html
@@ -1,7 +1,7 @@
<script>
const parent = window.parent;
alert(1);
-parent.document.getElementsByTagName('iframe')[0].src = "javascript:alert(2),'PASS<script>alert(3)<\/script>'";
-alert(4);
+parent.document.getElementsByTagName('iframe')[0].src = "javascript:alert(3),'PASS<script>alert(4)<\/script>'";
+alert(2);
parent.setTimeout("done()", 0);
</script>
diff --git a/LayoutTests/fast/parser/xml-error-adopted.xml b/LayoutTests/fast/parser/xml-error-adopted.xml
index 9b1abeb..f3e97ff 100644
--- a/LayoutTests/fast/parser/xml-error-adopted.xml
+++ b/LayoutTests/fast/parser/xml-error-adopted.xml
@@ -15,7 +15,9 @@
testRunner.notifyDone();
}
-setTimeout(test, 0);
+onload = () => {
+ setTimeout(test, 0);
+};
</script>
<elt attr="1" attr="2"/>
-</svg>
\ No newline at end of file
+</svg>
diff --git a/LayoutTests/http/tests/navigation/lockedhistory-iframe-expected.txt b/LayoutTests/http/tests/navigation/lockedhistory-iframe-expected.txt
index 558eca8..8267def 100644
--- a/LayoutTests/http/tests/navigation/lockedhistory-iframe-expected.txt
+++ b/LayoutTests/http/tests/navigation/lockedhistory-iframe-expected.txt
@@ -4,5 +4,6 @@
============== Back Forward List ==============
curr-> http://127.0.0.1:8000/navigation/lockedhistory-iframe.html **nav target**
- about:blank (in frame "<!--frame1-->")
+ http://127.0.0.1:8000/navigation/lockedhistory-iframe.html# (in frame "<!--frame1-->")
+ about:blank (in frame "<!--frame2-->")
===============================================
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
index 8c43c18..ae41c50 100644
--- a/LayoutTests/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
@@ -6,8 +6,9 @@
frame "<!--frame2-->" - didFinishDocumentLoadForFrame
frame "<!--frame2-->" - didHandleOnloadEventsForFrame
frame "<!--frame2-->" - didFinishLoadForFrame
-CONSOLE MESSAGE: Blocked mixed content http://127.0.0.1:8000/security/resources/compass.jpg because 'block-all-mixed-content' appears in the Content Security Policy.
+frame "<!--frame2-->" - willPerformClientRedirectToURL: javascript:document.write('%3Cimg%20src=%22http://127.0.0.1:8000/security/resources/compass.jpg%22%3E');
frame "<!--frame1-->" - didFinishDocumentLoadForFrame
+CONSOLE MESSAGE: Blocked mixed content http://127.0.0.1:8000/security/resources/compass.jpg because 'block-all-mixed-content' appears in the Content Security Policy.
frame "<!--frame1-->" - didFinishLoadForFrame
main frame - didFinishLoadForFrame
This test loads a secure iframe that loads an insecure image inside a JavaScript URL iframe. We should trigger a mixed content block because the child frame has CSP directive block-all-mixed-content and a JavaScript URL executes in the same origin as its embedding document.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt
index ef66293..3a1a9f4 100644
--- a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt
+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-allowed-expected.txt
@@ -1,7 +1,7 @@
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
+CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
+CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
ALERT: PASS
-CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
-CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt
index c64af31..f295915 100644
--- a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt
+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star-expected.txt
@@ -1,4 +1,4 @@
+CONSOLE MESSAGE: Refused to load javascript:alert('FAIL'); because it appears in neither the object-src directive nor the default-src directive of the Content Security Policy.
+CONSOLE MESSAGE: Refused to load javascript:alert('FAIL'); because it appears in neither the object-src directive nor the default-src directive of the Content Security Policy.
CONSOLE MESSAGE: line 1: Refused to execute a script because its hash, its nonce, or 'unsafe-inline' appears in neither the script-src directive nor the default-src directive of the Content Security Policy.
-CONSOLE MESSAGE: Refused to load javascript:alert('FAIL'); because it appears in neither the object-src directive nor the default-src directive of the Content Security Policy.
-CONSOLE MESSAGE: Refused to load javascript:alert('FAIL'); because it appears in neither the object-src directive nor the default-src directive of the Content Security Policy.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt
index 967bfa6..7e1a919 100644
--- a/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt
+++ b/LayoutTests/http/tests/security/contentSecurityPolicy/javascript-url-blocked-expected.txt
@@ -1,7 +1,7 @@
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
+CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
+CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
CONSOLE MESSAGE: line 1: Refused to execute a script because its hash, its nonce, or 'unsafe-inline' does not appear in the script-src directive of the Content Security Policy.
-CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
-CONSOLE MESSAGE: The 'allow' directive has been replaced with 'default-src'. Please use that directive instead, as 'allow' has no effect.
diff --git a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
index 85ab9be..236042a 100644
--- a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
+++ b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html
@@ -7,11 +7,13 @@
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
+ testRunner.waitUntilDone();
}
var innerURL = 'javascript:\\\"<html>'
+ "<scr" + "ipt>"
+ 'top.document.getElementById(\\\\\\\"accessMe\\\\\\\").innerHTML = \\\\\\\"PASS: Cross frame access from a javascript: URL inside another javascript: URL was allowed!\\\\\\\";'
+ + 'top.setTimeout(() => { testRunner.notifyDone(); }, 0);'
+ "</scri" + "pt>"
+ "<body>"
+ "<p>Inner-inner iframe.</p>"
diff --git a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html
index ce83c72..327d9ce 100644
--- a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html
+++ b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame.html
@@ -7,6 +7,7 @@
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
+ testRunner.waitUntilDone();
}
var url = "javascript:\"<html>"
@@ -20,6 +21,12 @@
var iframe = document.getElementById("aFrame");
iframe.src = url;
+ onload = () => {
+ setTimeout(() => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }
</script>
</body>
</html>
diff --git a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
index 17f9af7..bbfa7bb 100644
--- a/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
+++ b/LayoutTests/http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html
@@ -7,6 +7,7 @@
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
+ testRunner.waitUntilDone();
}
var innerURL = 'javascript:\\\"<html>'
@@ -30,6 +31,13 @@
var iframe = document.getElementById("aFrame");
iframe.src = url;
+
+ onload = () => {
+ setTimeout(() => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ };
</script>
</body>
</html>
diff --git a/LayoutTests/imported/blink/fast/frames/navigation-in-pagehide.html b/LayoutTests/imported/blink/fast/frames/navigation-in-pagehide.html
index 37c2149..feca095 100644
--- a/LayoutTests/imported/blink/fast/frames/navigation-in-pagehide.html
+++ b/LayoutTests/imported/blink/fast/frames/navigation-in-pagehide.html
@@ -17,20 +17,13 @@
var div = document.createElement('div');
firstFrame.appendChild(div);
secondFrame = document.createElement('iframe');
- secondFrame.src = 'javascript:window.top.maybeStart();';
+ secondFrame.src = 'javascript:window.top.reallyStart();';
div.appendChild(secondFrame);
var firstFrameRoot = firstFrame.contentDocument.documentElement;
document.documentElement.appendChild(div);
firstFrameRoot.appendChild(secondFrame);
}
-function maybeStart() {
- if (callbackCount++ > 1) {
- reallyStart();
- return;
- }
-}
-
function reallyStart(frame) {
secondFrame.contentWindow.onpagehide = function () {
firstFrame.src = 'javascript:window.top.navigateThere();';
@@ -39,7 +32,7 @@
if (window.location.hash == '#done') {
if (window.testRunner)
- window.testRunner.notifyDone();
+ testRunner.notifyDone();
return;
}
diff --git a/LayoutTests/imported/blink/loader/iframe-sync-loads-expected.txt b/LayoutTests/imported/blink/loader/iframe-sync-loads-expected.txt
index 1159c11..686d3b3 100644
--- a/LayoutTests/imported/blink/loader/iframe-sync-loads-expected.txt
+++ b/LayoutTests/imported/blink/loader/iframe-sync-loads-expected.txt
@@ -1,4 +1,4 @@
- sync : src = javascript:"content"
+ASYNC : src = javascript:"content"
ASYNC : src = data:text/html,content
ASYNC : srcdoc = "content"
done
diff --git a/LayoutTests/js/dom/call-base-resolution.html b/LayoutTests/js/dom/call-base-resolution.html
index c131bdb..4bdb08b 100644
--- a/LayoutTests/js/dom/call-base-resolution.html
+++ b/LayoutTests/js/dom/call-base-resolution.html
@@ -4,7 +4,7 @@
</head>
<body>
-<script src="../../resources/js-test-pre.js"></script>
+<script src="../../resources/js-test.js"></script>
<script>
window.name = "o";
function f() {
@@ -77,7 +77,5 @@
parent.testFailed(results + ' should be ' + expected + ', but was not.');
">
</iframe>
-<script src="../../resources/js-test-post.js"></script>
-
</body>
</html>
diff --git a/LayoutTests/platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt b/LayoutTests/platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
index 147ccce..8ec742d 100644
--- a/LayoutTests/platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
+++ b/LayoutTests/platform/wk2/http/tests/security/contentSecurityPolicy/block-all-mixed-content/insecure-image-in-javascript-url-iframe-in-iframe-expected.txt
@@ -6,8 +6,9 @@
frame "<!--frame2-->" - didFinishDocumentLoadForFrame
frame "<!--frame2-->" - didHandleOnloadEventsForFrame
frame "<!--frame2-->" - didFinishLoadForFrame
-CONSOLE MESSAGE: Blocked mixed content http://127.0.0.1:8000/security/resources/compass.jpg because 'block-all-mixed-content' appears in the Content Security Policy.
+frame "<!--frame2-->" - willPerformClientRedirectToURL: javascript:document.write('<img src=%22http://127.0.0.1:8000/security/resources/compass.jpg%22>');
frame "<!--frame1-->" - didFinishDocumentLoadForFrame
+CONSOLE MESSAGE: Blocked mixed content http://127.0.0.1:8000/security/resources/compass.jpg because 'block-all-mixed-content' appears in the Content Security Policy.
frame "<!--frame1-->" - didFinishLoadForFrame
main frame - didFinishLoadForFrame
This test loads a secure iframe that loads an insecure image inside a JavaScript URL iframe. We should trigger a mixed content block because the child frame has CSP directive block-all-mixed-content and a JavaScript URL executes in the same origin as its embedding document.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 080e940..d2b9ea1 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2019-05-02 Chris Dumez <cdumez@apple.com>
+
+ Setting a frame's src to a javascript URL should not run it synchronously
+ https://bugs.webkit.org/show_bug.cgi?id=197466
+
+ Reviewed by Darin Adler.
+
+ When an iframe's src attribute is set to a javascript URL, whether when parsing
+ or later on via JS, we now execute the URL's JavaScript asynchronously. We used
+ to execute it synchronously, which was a source of bugs and also did not match
+ other browsers.
+
+ I have verified that our new behavior is aligned with both Firefox and Chrome.
+
+ Note that for backward-compatibility and interoperability with Blink
+ (https://bugs.chromium.org/p/chromium/issues/detail?id=923585), the
+ "javascript:''" URL will still run synchronously. We should consider dropping
+ this quirk at some point.
+
+ Test: fast/dom/frame-src-javascript-url-async.html
+
+ * loader/NavigationScheduler.cpp:
+ (WebCore::ScheduledLocationChange::ScheduledLocationChange):
+ (WebCore::ScheduledLocationChange::~ScheduledLocationChange):
+ (WebCore::NavigationScheduler::scheduleLocationChange):
+ * loader/NavigationScheduler.h:
+ (WebCore::NavigationScheduler::scheduleLocationChange):
+ * loader/SubframeLoader.cpp:
+ (WebCore::SubframeLoader::requestFrame):
+
2019-05-02 Gary Katsevman <git@gkatsev.com>
WebVTT: fix vertical cue alignment.
diff --git a/Source/WebCore/loader/NavigationScheduler.cpp b/Source/WebCore/loader/NavigationScheduler.cpp
index 617dedb..558c780 100644
--- a/Source/WebCore/loader/NavigationScheduler.cpp
+++ b/Source/WebCore/loader/NavigationScheduler.cpp
@@ -193,8 +193,17 @@
class ScheduledLocationChange : public ScheduledURLNavigation {
public:
- ScheduledLocationChange(Document& initiatingDocument, SecurityOrigin* securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad)
- : ScheduledURLNavigation(initiatingDocument, 0.0, securityOrigin, url, referrer, lockHistory, lockBackForwardList, duringLoad, true) { }
+ ScheduledLocationChange(Document& initiatingDocument, SecurityOrigin* securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad, CompletionHandler<void()>&& completionHandler)
+ : ScheduledURLNavigation(initiatingDocument, 0.0, securityOrigin, url, referrer, lockHistory, lockBackForwardList, duringLoad, true)
+ , m_completionHandler(WTFMove(completionHandler))
+ {
+ }
+
+ ~ScheduledLocationChange()
+ {
+ if (m_completionHandler)
+ m_completionHandler();
+ }
void fire(Frame& frame) override
{
@@ -203,8 +212,13 @@
ResourceRequest resourceRequest { url(), referrer(), ResourceRequestCachePolicy::UseProtocolCachePolicy };
FrameLoadRequest frameLoadRequest { initiatingDocument(), *securityOrigin(), resourceRequest, "_self", lockHistory(), lockBackForwardList(), MaybeSendReferrer, AllowNavigationToInvalidURL::No, NewFrameOpenerPolicy::Allow, shouldOpenExternalURLs(), initiatedByMainFrame() };
+ auto completionHandler = WTFMove(m_completionHandler);
frame.loader().changeLocation(WTFMove(frameLoadRequest));
+ completionHandler();
}
+
+private:
+ CompletionHandler<void()> m_completionHandler;
};
class ScheduledRefresh : public ScheduledURLNavigation {
@@ -405,10 +419,10 @@
return LockBackForwardList::No;
}
-void NavigationScheduler::scheduleLocationChange(Document& initiatingDocument, SecurityOrigin& securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList)
+void NavigationScheduler::scheduleLocationChange(Document& initiatingDocument, SecurityOrigin& securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, CompletionHandler<void()>&& completionHandler)
{
if (!shouldScheduleNavigation(url))
- return;
+ return completionHandler();
if (lockBackForwardList == LockBackForwardList::No)
lockBackForwardList = mustLockBackForwardList(m_frame);
@@ -424,14 +438,14 @@
FrameLoadRequest frameLoadRequest { initiatingDocument, securityOrigin, resourceRequest, "_self"_s, lockHistory, lockBackForwardList, MaybeSendReferrer, AllowNavigationToInvalidURL::No, NewFrameOpenerPolicy::Allow, initiatingDocument.shouldOpenExternalURLsPolicyToPropagate(), initiatedByMainFrame };
loader.changeLocation(WTFMove(frameLoadRequest));
- return;
+ return completionHandler();
}
// Handle a location change of a page with no document as a special case.
// This may happen when a frame changes the location of another frame.
bool duringLoad = !loader.stateMachine().committedFirstRealDocumentLoad();
- schedule(std::make_unique<ScheduledLocationChange>(initiatingDocument, &securityOrigin, url, referrer, lockHistory, lockBackForwardList, duringLoad));
+ schedule(std::make_unique<ScheduledLocationChange>(initiatingDocument, &securityOrigin, url, referrer, lockHistory, lockBackForwardList, duringLoad, WTFMove(completionHandler)));
}
void NavigationScheduler::scheduleFormSubmission(Ref<FormSubmission>&& submission)
diff --git a/Source/WebCore/loader/NavigationScheduler.h b/Source/WebCore/loader/NavigationScheduler.h
index b97d986..cbd99c2 100644
--- a/Source/WebCore/loader/NavigationScheduler.h
+++ b/Source/WebCore/loader/NavigationScheduler.h
@@ -53,7 +53,7 @@
bool locationChangePending();
void scheduleRedirect(Document& initiatingDocument, double delay, const URL&);
- void scheduleLocationChange(Document& initiatingDocument, SecurityOrigin&, const URL&, const String& referrer, LockHistory = LockHistory::Yes, LockBackForwardList = LockBackForwardList::Yes);
+ void scheduleLocationChange(Document& initiatingDocument, SecurityOrigin&, const URL&, const String& referrer, LockHistory = LockHistory::Yes, LockBackForwardList = LockBackForwardList::Yes, CompletionHandler<void()>&& = [] { });
void scheduleFormSubmission(Ref<FormSubmission>&&);
void scheduleRefresh(Document& initiatingDocument);
void scheduleHistoryNavigation(int steps);
diff --git a/Source/WebCore/loader/SubframeLoader.cpp b/Source/WebCore/loader/SubframeLoader.cpp
index 1f90a83..5f98d55 100644
--- a/Source/WebCore/loader/SubframeLoader.cpp
+++ b/Source/WebCore/loader/SubframeLoader.cpp
@@ -56,6 +56,7 @@
#include "SecurityOrigin.h"
#include "SecurityPolicy.h"
#include "Settings.h"
+#include <wtf/CompletionHandler.h>
namespace WebCore {
@@ -86,17 +87,27 @@
if (shouldConvertInvalidURLsToBlank() && !url.isValid())
url = WTF::blankURL();
- bool hasExistingFrame = ownerElement.contentFrame();
+ // If we will schedule a JavaScript URL load, we need to delay the firing of the load event at least until we've run the JavaScript in the URL.
+ CompletionHandlerCallingScope stopDelayingLoadEvent;
+ if (!scriptURL.isEmpty()) {
+ ownerElement.document().incrementLoadEventDelayCount();
+ stopDelayingLoadEvent = CompletionHandlerCallingScope([ownerDocument = makeRef(ownerElement.document())] {
+ ownerDocument->decrementLoadEventDelayCount();
+ });
+ }
+
Frame* frame = loadOrRedirectSubframe(ownerElement, url, frameName, lockHistory, lockBackForwardList);
if (!frame)
return false;
- // If we create a new subframe then an empty document is loaded into it synchronously and may
- // cause script execution (say, via a DOM load event handler) that can do anything, including
- // navigating the subframe. We only want to evaluate scriptURL if the frame has not been navigated.
- bool canExecuteScript = hasExistingFrame || (frame->loader().documentLoader() && frame->loader().documentLoader()->originalURL() == WTF::blankURL());
- if (!scriptURL.isEmpty() && canExecuteScript && ownerElement.isURLAllowed(scriptURL))
- frame->script().executeIfJavaScriptURL(scriptURL);
+ if (!scriptURL.isEmpty() && ownerElement.isURLAllowed(scriptURL)) {
+ // FIXME: Some sites rely on the javascript:'' loading synchronously, which is why we have this special case.
+ // Blink has the same workaround (https://bugs.chromium.org/p/chromium/issues/detail?id=923585).
+ if (urlString == "javascript:''" || urlString == "javascript:\"\"")
+ frame->script().executeIfJavaScriptURL(scriptURL);
+ else
+ frame->navigationScheduler().scheduleLocationChange(ownerElement.document(), ownerElement.document().securityOrigin(), scriptURL, m_frame.loader().outgoingReferrer(), lockHistory, lockBackForwardList, stopDelayingLoadEvent.release());
+ }
return true;
}
