From 0c15fa738313f1afba4b84ec53d19c81d4902711 Mon Sep 17 00:00:00 2001 From: Mark Pilgrim Date: Fri, 27 May 2022 12:36:34 -0400 Subject: [PATCH 1/4] Nullify window.name on cross-site navigation --- .../blink/renderer/core/page/frame_tree.cc | 23 +++++++++++++++++++ .../blink/renderer/core/page/frame_tree.h | 17 ++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 chromium_src/third_party/blink/renderer/core/page/frame_tree.cc create mode 100644 chromium_src/third_party/blink/renderer/core/page/frame_tree.h diff --git a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc new file mode 100644 index 000000000000..162ae61da682 --- /dev/null +++ b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc @@ -0,0 +1,23 @@ +/* Copyright (c) 2022 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "third_party/blink/renderer/core/page/frame_tree.h" + +#define CrossSiteCrossBrowsingContextGroupSetNulledName \ + CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl + +#include "src/third_party/blink/renderer/core/page/frame_tree.cc" + +#undef CrossSiteCrossBrowsingContextGroupSetNulledName + +namespace blink { + +void FrameTree::CrossSiteCrossBrowsingContextGroupSetNulledName() { + LOG(ERROR) << "1"; + SetName(g_null_atom, kReplicate); + CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl(); +} + +} // namespace blink diff --git a/chromium_src/third_party/blink/renderer/core/page/frame_tree.h b/chromium_src/third_party/blink/renderer/core/page/frame_tree.h new file mode 100644 index 000000000000..d368319cb61e --- /dev/null +++ b/chromium_src/third_party/blink/renderer/core/page/frame_tree.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2022 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_FRAME_TREE_H_ +#define BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_FRAME_TREE_H_ + +#define CrossSiteCrossBrowsingContextGroupSetNulledName \ + CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl(); \ + void CrossSiteCrossBrowsingContextGroupSetNulledName + +#include "src/third_party/blink/renderer/core/page/frame_tree.h" + +#undef CrossSiteCrossBrowsingContextGroupSetNulledName + +#endif // BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_PAGE_FRAME_TREE_H_ From 95fa4fdc0c9cc0b2153216e7373b94185528a90d Mon Sep 17 00:00:00 2001 From: Mark Pilgrim Date: Thu, 2 Jun 2022 11:45:45 -0400 Subject: [PATCH 2/4] tests --- browser/test/BUILD.gn | 1 + .../window_name_browsertest.cc | 94 +++++++++++++++++++ .../blink/renderer/core/page/frame_tree.cc | 1 - test/data/window_name/get_window_name.html | 8 ++ test/data/window_name/set_window_name.html | 11 +++ 5 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 browser/test/disabled_features/window_name_browsertest.cc create mode 100644 test/data/window_name/get_window_name.html create mode 100644 test/data/window_name/set_window_name.html diff --git a/browser/test/BUILD.gn b/browser/test/BUILD.gn index fbf25bca243a..e5d9d25e1705 100644 --- a/browser/test/BUILD.gn +++ b/browser/test/BUILD.gn @@ -13,6 +13,7 @@ source_set("browser_tests") { "disabled_features/navigator_bluetooth_browsertest.cc", "disabled_features/navigator_storage_browsertest.cc", "disabled_features/reporting_observer_browsertest.cc", + "disabled_features/window_name_browsertest.cc", ] deps = [ diff --git a/browser/test/disabled_features/window_name_browsertest.cc b/browser/test/disabled_features/window_name_browsertest.cc new file mode 100644 index 000000000000..899a3a8e9ae3 --- /dev/null +++ b/browser/test/disabled_features/window_name_browsertest.cc @@ -0,0 +1,94 @@ +/* Copyright (c) 2022 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "base/path_service.h" +#include "brave/browser/brave_content_browser_client.h" +#include "brave/components/constants/brave_paths.h" +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/common/chrome_content_client.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/browser_test_utils.h" +#include "net/dns/mock_host_resolver.h" +#include "url/gurl.h" + +namespace { + +const char kEmbeddedTestServerDirectory[] = "window_name"; +const char kWindowNameScript[] = "window.name"; + +} // namespace + +class BraveWindowNameBrowserTest : public InProcessBrowserTest { + public: + BraveWindowNameBrowserTest() + : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) { + https_server_.SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES); + brave::RegisterPathProvider(); + base::FilePath test_data_dir; + base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir); + test_data_dir = test_data_dir.AppendASCII(kEmbeddedTestServerDirectory); + https_server_.ServeFilesFromDirectory(test_data_dir); + EXPECT_TRUE(https_server_.Start()); + } + + BraveWindowNameBrowserTest(const BraveWindowNameBrowserTest&) = delete; + BraveWindowNameBrowserTest& operator=(const BraveWindowNameBrowserTest&) = + delete; + + ~BraveWindowNameBrowserTest() override {} + + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + content_client_.reset(new ChromeContentClient); + content::SetContentClient(content_client_.get()); + browser_content_client_.reset(new BraveContentBrowserClient()); + content::SetBrowserClientForTesting(browser_content_client_.get()); + host_resolver()->AddRule("*", "127.0.0.1"); + } + + void TearDown() override { + browser_content_client_.reset(); + content_client_.reset(); + } + + protected: + net::EmbeddedTestServer https_server_; + + content::WebContents* web_contents() { + return browser()->tab_strip_model()->GetActiveWebContents(); + } + + private: + std::unique_ptr content_client_; + std::unique_ptr browser_content_client_; +}; + +IN_PROC_BROWSER_TEST_F(BraveWindowNameBrowserTest, SameOrigin) { + GURL url1 = https_server_.GetURL("a.test", "/set_window_name.html"); + GURL url2 = https_server_.GetURL("a.test", "/get_window_name.html"); + + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url1)); + EXPECT_EQ("foo", EvalJs(web_contents(), kWindowNameScript)); + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url2)); + // Since these URLs are in the same origin, window.name should persist across + // navigation. + EXPECT_EQ("foo", EvalJs(web_contents(), kWindowNameScript)); +} + +IN_PROC_BROWSER_TEST_F(BraveWindowNameBrowserTest, CrossOrigin) { + GURL url1 = https_server_.GetURL("a.test", "/set_window_name.html"); + GURL url2 = https_server_.GetURL("b.test", "/get_window_name.html"); + + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url1)); + EXPECT_EQ("foo", EvalJs(web_contents(), kWindowNameScript)); + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url2)); + // Since these URLs are in different origins, window.name should be cleared + // during navigation. + EXPECT_EQ("", EvalJs(web_contents(), kWindowNameScript)); +} diff --git a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc index 162ae61da682..7f9931ee9351 100644 --- a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc +++ b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc @@ -15,7 +15,6 @@ namespace blink { void FrameTree::CrossSiteCrossBrowsingContextGroupSetNulledName() { - LOG(ERROR) << "1"; SetName(g_null_atom, kReplicate); CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl(); } diff --git a/test/data/window_name/get_window_name.html b/test/data/window_name/get_window_name.html new file mode 100644 index 000000000000..3829af946e3a --- /dev/null +++ b/test/data/window_name/get_window_name.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/test/data/window_name/set_window_name.html b/test/data/window_name/set_window_name.html new file mode 100644 index 000000000000..5f6b14f97798 --- /dev/null +++ b/test/data/window_name/set_window_name.html @@ -0,0 +1,11 @@ + + + + + + + + + From 5a114793620ec484e4b20a1abdf722fea9e3e8ac Mon Sep 17 00:00:00 2001 From: Mark Pilgrim Date: Tue, 7 Jun 2022 14:23:23 -0400 Subject: [PATCH 3/4] don't replicate name change, and add a test for back navigation --- .../disabled_features/window_name_browsertest.cc | 15 +++++++++++++++ .../blink/renderer/core/page/frame_tree.cc | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/browser/test/disabled_features/window_name_browsertest.cc b/browser/test/disabled_features/window_name_browsertest.cc index 899a3a8e9ae3..fd33867c6f6e 100644 --- a/browser/test/disabled_features/window_name_browsertest.cc +++ b/browser/test/disabled_features/window_name_browsertest.cc @@ -92,3 +92,18 @@ IN_PROC_BROWSER_TEST_F(BraveWindowNameBrowserTest, CrossOrigin) { // during navigation. EXPECT_EQ("", EvalJs(web_contents(), kWindowNameScript)); } + +IN_PROC_BROWSER_TEST_F(BraveWindowNameBrowserTest, CrossOriginAndBack) { + GURL url1 = https_server_.GetURL("a.test", "/set_window_name.html"); + GURL url2 = https_server_.GetURL("b.test", "/get_window_name.html"); + + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url1)); + EXPECT_EQ("foo", EvalJs(web_contents(), kWindowNameScript)); + EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url2)); + // Since these URLs are in different origins, window.name should be cleared + // during navigation. + EXPECT_EQ("", EvalJs(web_contents(), kWindowNameScript)); + web_contents()->GetController().GoBack(); + EXPECT_TRUE(WaitForLoadStop(web_contents())); + EXPECT_EQ("foo", EvalJs(web_contents(), kWindowNameScript)); +} diff --git a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc index 7f9931ee9351..d928b6ccf045 100644 --- a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc +++ b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc @@ -15,7 +15,7 @@ namespace blink { void FrameTree::CrossSiteCrossBrowsingContextGroupSetNulledName() { - SetName(g_null_atom, kReplicate); + SetName(g_null_atom); CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl(); } From 9224c7e2bd3b4805fb0c4f86cc95db79892a18d8 Mon Sep 17 00:00:00 2001 From: Mark Pilgrim Date: Tue, 7 Jun 2022 14:47:05 -0400 Subject: [PATCH 4/4] replicate --- chromium_src/third_party/blink/renderer/core/page/frame_tree.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc index d928b6ccf045..7f9931ee9351 100644 --- a/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc +++ b/chromium_src/third_party/blink/renderer/core/page/frame_tree.cc @@ -15,7 +15,7 @@ namespace blink { void FrameTree::CrossSiteCrossBrowsingContextGroupSetNulledName() { - SetName(g_null_atom); + SetName(g_null_atom, kReplicate); CrossSiteCrossBrowsingContextGroupSetNulledName_ChromiumImpl(); }