- Revision
- 280826
- Author
- [email protected]
- Date
- 2021-08-10 01:25:31 -0700 (Tue, 10 Aug 2021)
Log Message
Partially revert r280256 behavior change since it broke Facetime
https://bugs.webkit.org/show_bug.cgi?id=228943
rdar://81700268
Reviewed by Mark Lam.
Source/WebCore:
In r280256, custom accessors start getting holder's JSGlobalObject (this is right behavior).
We also introduced IncumbentWindow IDL attribute to get the caller's JSGlobalObject, which is
necessary in Location. At that time, we replaced all ActiveWindow to IncumbentWindow, but it
changes the JSGlobalObject of `replace`, `assign`, and `reload` since they are functions (not
custom accessors). We observed that, after this change, facetime Web page is broken: when opening
the link to Facetime, it should show the prompt of "Open in Facetime app" in iOS. But after this
change, it won't happen.
We revert the semantic change by using ActiveWindow for replace, assign, and reload. This is
not the correct semantics from the spec, but this is the same to one before r280256, and we
will change this once we find the way to bypass this issue.
* page/Location.cpp:
(WebCore::Location::assign):
(WebCore::Location::replace):
(WebCore::Location::reload):
* page/Location.h:
* page/Location.idl:
LayoutTests:
* fast/frames/sandboxed-iframe-navigation-parent-expected.txt:
* fast/frames/sandboxed-iframe-navigation-parent.html:
* http/tests/security/frameNavigation/context-for-location-assign-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (280825 => 280826)
--- trunk/LayoutTests/ChangeLog 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/LayoutTests/ChangeLog 2021-08-10 08:25:31 UTC (rev 280826)
@@ -1,3 +1,15 @@
+2021-08-09 Yusuke Suzuki <[email protected]>
+
+ Partially revert r280256 behavior change since it broke Facetime
+ https://bugs.webkit.org/show_bug.cgi?id=228943
+ rdar://81700268
+
+ Reviewed by Mark Lam.
+
+ * fast/frames/sandboxed-iframe-navigation-parent-expected.txt:
+ * fast/frames/sandboxed-iframe-navigation-parent.html:
+ * http/tests/security/frameNavigation/context-for-location-assign-expected.txt:
+
2021-08-09 Chris Dumez <[email protected]>
Blob URL changes after loading it
Modified: trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent-expected.txt (280825 => 280826)
--- trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent-expected.txt 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent-expected.txt 2021-08-10 08:25:31 UTC (rev 280826)
@@ -1,10 +1,5 @@
-CONSOLE MESSAGE: Unsafe _javascript_ attempt to initiate navigation for frame with URL 'sandboxed-iframe-navigation-source.html'. The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors.
+This test verifies that a sandboxed IFrame with the keywords allow-scripts and allow-same-origin is allowed to navigate an ancestor in the frame tree by assigning the location attribute.
-CONSOLE MESSAGE: SecurityError: The operation is insecure.
-This test verifies that a sandboxed IFrame cannot navigate an ancestor in the frame tree by assigning the location attribute.
-
-This is done by loading ten non-sandboxed IFrames, and a single sandboxed one. Expect ten frames to be navigated, but the sandboxed one to not be one of them.
-
This test will print "PASS" on success.
Modified: trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent.html (280825 => 280826)
--- trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent.html 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/LayoutTests/fast/frames/sandboxed-iframe-navigation-parent.html 2021-08-10 08:25:31 UTC (rev 280826)
@@ -13,9 +13,9 @@
{
var testStatus = document.getElementById('testStatus');
- if (self.navigated.location.href.indexOf('parent.html') == -1)
- testStatus.innerHTML = 'FAIL: disallowed frame navigated';
- else if (navigatedFrames == 10)
+ if (self.navigated.location.href.indexOf('parent.html') != -1)
+ testStatus.innerHTML = 'FAIL: sandboxed frame couldn\'t navigate';
+ else if (navigatedFrames == 11)
testStatus.innerHTML = 'PASS';
else
testStatus.innerHTML = 'FAIL: frames not navigated properly';
@@ -28,7 +28,7 @@
{
++navigatedFrames;
- if (navigatedFrames == 10)
+ if (navigatedFrames == 11)
checkResults();
}
@@ -37,13 +37,10 @@
<body>
- <p>This test verifies that a sandboxed IFrame cannot navigate an ancestor
+ <p>This test verifies that a sandboxed IFrame with the keywords
+ allow-scripts and allow-same-origin is allowed to navigate an ancestor
in the frame tree by assigning the location attribute.</p>
- <p>This is done by loading ten non-sandboxed IFrames, and a single
- sandboxed one. Expect ten frames to be navigated, but the sandboxed
- one to <b>not</b> be one of them.</p>
-
<p>This test will print "PASS" on success.</p>
<iframe src=""
Modified: trunk/LayoutTests/http/tests/security/frameNavigation/context-for-location-assign-expected.txt (280825 => 280826)
--- trunk/LayoutTests/http/tests/security/frameNavigation/context-for-location-assign-expected.txt 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/LayoutTests/http/tests/security/frameNavigation/context-for-location-assign-expected.txt 2021-08-10 08:25:31 UTC (rev 280826)
@@ -1,2 +1,2 @@
-document.referrer = http://127.0.0.1:8000/security/frameNavigation/context-for-location-assign.html
+document.referrer =
Modified: trunk/Source/WebCore/ChangeLog (280825 => 280826)
--- trunk/Source/WebCore/ChangeLog 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/Source/WebCore/ChangeLog 2021-08-10 08:25:31 UTC (rev 280826)
@@ -1,3 +1,30 @@
+2021-08-09 Yusuke Suzuki <[email protected]>
+
+ Partially revert r280256 behavior change since it broke Facetime
+ https://bugs.webkit.org/show_bug.cgi?id=228943
+ rdar://81700268
+
+ Reviewed by Mark Lam.
+
+ In r280256, custom accessors start getting holder's JSGlobalObject (this is right behavior).
+ We also introduced IncumbentWindow IDL attribute to get the caller's JSGlobalObject, which is
+ necessary in Location. At that time, we replaced all ActiveWindow to IncumbentWindow, but it
+ changes the JSGlobalObject of `replace`, `assign`, and `reload` since they are functions (not
+ custom accessors). We observed that, after this change, facetime Web page is broken: when opening
+ the link to Facetime, it should show the prompt of "Open in Facetime app" in iOS. But after this
+ change, it won't happen.
+
+ We revert the semantic change by using ActiveWindow for replace, assign, and reload. This is
+ not the correct semantics from the spec, but this is the same to one before r280256, and we
+ will change this once we find the way to bypass this issue.
+
+ * page/Location.cpp:
+ (WebCore::Location::assign):
+ (WebCore::Location::replace):
+ (WebCore::Location::reload):
+ * page/Location.h:
+ * page/Location.idl:
+
2021-08-09 Chris Dumez <[email protected]>
Blob URL changes after loading it
Modified: trunk/Source/WebCore/page/Location.cpp (280825 => 280826)
--- trunk/Source/WebCore/page/Location.cpp 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/Source/WebCore/page/Location.cpp 2021-08-10 08:25:31 UTC (rev 280826)
@@ -210,14 +210,14 @@
return setLocation(incumbentWindow, firstWindow, url.string());
}
-ExceptionOr<void> Location::assign(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String& url)
+ExceptionOr<void> Location::assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& url)
{
if (!frame())
return { };
- return setLocation(incumbentWindow, firstWindow, url);
+ return setLocation(activeWindow, firstWindow, url);
}
-ExceptionOr<void> Location::replace(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String& urlString)
+ExceptionOr<void> Location::replace(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString)
{
auto* frame = this->frame();
if (!frame)
@@ -234,21 +234,21 @@
return Exception { SyntaxError };
// We call DOMWindow::setLocation directly here because replace() always operates on the current frame.
- frame->document()->domWindow()->setLocation(incumbentWindow, completedURL, LockHistoryAndBackForwardList);
+ frame->document()->domWindow()->setLocation(activeWindow, completedURL, LockHistoryAndBackForwardList);
return { };
}
-void Location::reload(DOMWindow& incumbentWindow)
+void Location::reload(DOMWindow& activeWindow)
{
auto* frame = this->frame();
if (!frame)
return;
- ASSERT(incumbentWindow.document());
+ ASSERT(activeWindow.document());
ASSERT(frame->document());
ASSERT(frame->document()->domWindow());
- auto& activeDocument = *incumbentWindow.document();
+ auto& activeDocument = *activeWindow.document();
auto& targetDocument = *frame->document();
// FIXME: It's not clear this cross-origin security check is valuable.
@@ -256,7 +256,7 @@
// Other location operations simply block use of _javascript_ URLs cross origin.
if (!activeDocument.securityOrigin().isSameOriginDomain(targetDocument.securityOrigin())) {
auto& targetWindow = *targetDocument.domWindow();
- targetWindow.printErrorMessage(targetWindow.crossDomainAccessErrorMessage(incumbentWindow, IncludeTargetOrigin::Yes));
+ targetWindow.printErrorMessage(targetWindow.crossDomainAccessErrorMessage(activeWindow, IncludeTargetOrigin::Yes));
return;
}
Modified: trunk/Source/WebCore/page/Location.h (280825 => 280826)
--- trunk/Source/WebCore/page/Location.h 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/Source/WebCore/page/Location.h 2021-08-10 08:25:31 UTC (rev 280826)
@@ -45,9 +45,9 @@
ExceptionOr<void> setHref(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String&);
String href() const;
- ExceptionOr<void> assign(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String&);
- ExceptionOr<void> replace(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String&);
- void reload(DOMWindow& incumbentWindow);
+ ExceptionOr<void> assign(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
+ ExceptionOr<void> replace(DOMWindow& activeWindow, DOMWindow& firstWindow, const String&);
+ void reload(DOMWindow& activeWindow);
ExceptionOr<void> setProtocol(DOMWindow& incumbentWindow, DOMWindow& firstWindow, const String&);
String protocol() const;
Modified: trunk/Source/WebCore/page/Location.idl (280825 => 280826)
--- trunk/Source/WebCore/page/Location.idl 2021-08-10 06:56:17 UTC (rev 280825)
+++ trunk/Source/WebCore/page/Location.idl 2021-08-10 08:25:31 UTC (rev 280826)
@@ -45,9 +45,10 @@
] interface Location {
[SetterCallWith=IncumbentWindow&FirstWindow, DoNotCheckSecurityOnSetter] stringifier attribute USVString href;
- [CallWith=IncumbentWindow&FirstWindow] undefined assign(USVString url);
- [CallWith=IncumbentWindow&FirstWindow, DoNotCheckSecurity] undefined replace(USVString url);
- [CallWith=IncumbentWindow] undefined reload();
+ // FIXME: We should use IncumbentWindow once we found why https://bugs.webkit.org/show_bug.cgi?id=228943 issue occured.
+ [CallWith=ActiveWindow&FirstWindow] undefined assign(USVString url);
+ [CallWith=ActiveWindow&FirstWindow, DoNotCheckSecurity] undefined replace(USVString url);
+ [CallWith=ActiveWindow] undefined reload();
// URI decomposition attributes
[SetterCallWith=IncumbentWindow&FirstWindow] attribute USVString protocol;
