Attachment 357998 Details for Bug 192997 – Patch

[patch] Patch

bug-192997-20181221160135.patch (text/plain), 8.34 KB, created by Chris Dumez on 2018-12-21 16:01:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-12-21 16:01:36 PST

Size: 8.34 KB

patch

obsolete

>Subversion Revision: 239472
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bf8f92c9aca92d35b5100c79410bd6c9c9a414bc..288eab74299142bd16648393dd2ee7eba11f581b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-12-21  Chris Dumez  <cdumez@apple.com>
>+
>+        Potential infinite recursion in isFrameFamiliarWith(Frame&, Frame&)
>+        https://bugs.webkit.org/show_bug.cgi?id=192997
>+        <rdar://problem/46217271>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        isFrameFamiliarWith(Frame&, Frame&) was called recursively using the passed frames' openers.
>+        The issue is that a Frame can be its opener. There could also be a cycle in the opener chain.
>+
>+        To address the issue, simplify isFrameFamiliarWith() so that it is no longer recursive. We now
>+        only check if the frames belong to the same pages or if their openers do. We no longer check
>+        openers' opener and up.
>+
>+        Note that this function is used to check if a frame is allowed to target another. In practice,
>+        it is unlikely to be useful to navigate an opener's opener and an openee's openee.
>+
>+        Tests: fast/dom/Window/window-open-opener-cycle.html
>+               fast/dom/Window/window-open-self-as-opener.html
>+
>+        * page/FrameTree.cpp:
>+        (WebCore::isFrameFamiliarWith):
>+
> 2018-12-21  Chris Dumez  <cdumez@apple.com>
> 
>         navigator.userAgent in service workers does not reflect customUserAgent set by client
>diff --git a/Source/WebCore/page/FrameTree.cpp b/Source/WebCore/page/FrameTree.cpp
>index a9fec153c9782f39462d90109fc0652c51bd1fd3..d63f861a780528f720334b3dbb81f7bb08c19a9c 100644
>--- a/Source/WebCore/page/FrameTree.cpp
>+++ b/Source/WebCore/page/FrameTree.cpp
>@@ -215,13 +215,9 @@ static bool isFrameFamiliarWith(Frame& frameA, Frame& frameB)
>     if (frameA.page() == frameB.page())
>         return true;
> 
>-    if (auto* frameAOpener = frameA.mainFrame().loader().opener())
>-        return isFrameFamiliarWith(*frameAOpener, frameB);
>-
>-    if (auto* frameBOpener = frameB.mainFrame().loader().opener())
>-        return isFrameFamiliarWith(frameA, *frameBOpener);
>-
>-    return false;
>+    auto* frameAOpener = frameA.mainFrame().loader().opener();
>+    auto* frameBOpener = frameB.mainFrame().loader().opener();
>+    return (frameAOpener && frameAOpener->page() == frameB.page()) || (frameBOpener && frameBOpener->page() == frameA.page()) || (frameAOpener && frameBOpener && frameAOpener->page() == frameBOpener->page());
> }
> 
> Frame* FrameTree::find(const AtomicString& name, Frame& activeFrame) const
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 81bbf4b9cf252864b1cf621f6b353a797c00a7e9..daaf9134165d97308a60dc405ca69872fc453067 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-12-21  Chris Dumez  <cdumez@apple.com>
>+
>+        Potential infinite recursion in isFrameFamiliarWith(Frame&, Frame&)
>+        https://bugs.webkit.org/show_bug.cgi?id=192997
>+        <rdar://problem/46217271>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * fast/dom/Window/resources/window-open-opener-cycle2.html: Added.
>+        * fast/dom/Window/resources/window-open-opener-cycle3.html: Added.
>+        * fast/dom/Window/resources/window-opens-self.html: Added.
>+        * fast/dom/Window/window-open-opener-cycle-expected.txt: Added.
>+        * fast/dom/Window/window-open-opener-cycle.html: Added.
>+        * fast/dom/Window/window-open-self-as-opener-expected.txt: Added.
>+        * fast/dom/Window/window-open-self-as-opener.html: Added.
>+
> 2018-12-20  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         [WebAuthN] Remove hash from Client Data
>diff --git a/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle2.html b/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle2.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..dd697e6237b26be80494413ae1aff588e82fb03b
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle2.html
>@@ -0,0 +1,13 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script>
>+if (window.testRunner)
>+    testRunner.setCanOpenWindows();
>+
>+onload = function() {
>+    w = open("window-open-opener-cycle3.html", "bar");
>+}
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle3.html b/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle3.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f47a930cc1f70117c59980b533f4f8441573f04d
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/resources/window-open-opener-cycle3.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script>
>+
>+if (window.testRunner)
>+    testRunner.setCanOpenWindows();
>+
>+onload = function() {
>+    root = opener.opener;
>+    open("", "foo");
>+    if (opener.opener === self)
>+        root.testPassed("opener.opener === self");
>+    else
>+        root.testFailed("opener.opener !== self");
>+    root.tryNavigateFoo();
>+}
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/Window/resources/window-opens-self.html b/LayoutTests/fast/dom/Window/resources/window-opens-self.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..1624993a365bec6809fddffe0917808c322c4d84
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/resources/window-opens-self.html
>@@ -0,0 +1,10 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script>
>+function setOpenerAsSelf() {
>+    open("", "_self");
>+}
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/Window/window-open-opener-cycle-expected.txt b/LayoutTests/fast/dom/Window/window-open-opener-cycle-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..9c483d7b88def32a620701e6a35c2df936aa6715
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/window-open-opener-cycle-expected.txt
>@@ -0,0 +1,10 @@
>+Tests navigating a window which has an opener cycle.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS opener.opener === self
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/dom/Window/window-open-opener-cycle.html b/LayoutTests/fast/dom/Window/window-open-opener-cycle.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7b35e623279f19e8e136325859291c8bf0f3c27c
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/window-open-opener-cycle.html
>@@ -0,0 +1,22 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../../resources/js-test.js"></script>
>+<script>
>+description("Tests navigating a window which has an opener cycle.");
>+jsTestIsAsync = true;
>+if (window.testRunner)
>+    testRunner.setCanOpenWindows();
>+
>+function tryNavigateFoo()
>+{
>+    open("about:blank", "foo");
>+    finishJSTest();
>+}
>+
>+onload = function() {
>+    w = window.open("resources/window-open-opener-cycle2.html", "foo");
>+}
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/Window/window-open-self-as-opener-expected.txt b/LayoutTests/fast/dom/Window/window-open-self-as-opener-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..0a0e6f8e002bd5eb2dfafa1694070766a3f43e63
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/window-open-self-as-opener-expected.txt
>@@ -0,0 +1,12 @@
>+Tests navigating a window whose opener is itself
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS w.opener is self
>+PASS w.opener is w
>+PASS w.opener is self
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/dom/Window/window-open-self-as-opener.html b/LayoutTests/fast/dom/Window/window-open-self-as-opener.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f89c98e42ed5f86fcc018caa3c256039096d449a
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Window/window-open-self-as-opener.html
>@@ -0,0 +1,26 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../../resources/js-test.js"></script>
>+<script>
>+description("Tests navigating a window whose opener is itself");
>+jsTestIsAsync = true;
>+if (window.testRunner)
>+    testRunner.setCanOpenWindows();
>+
>+onload = function() {
>+    w = window.open("resources/window-opens-self.html", "foo");
>+    shouldBe("w.opener", "self");
>+    w.onload = function() {
>+        w.setOpenerAsSelf();
>+        shouldBe("w.opener", "w");
>+
>+        w = window.open("about:blank", "foo");
>+        shouldBe("w.opener", "self");
>+
>+        finishJSTest();
>+    }
>+}
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192997: 357986 | 357996 | 357998