Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick f781748dcb3c from chromium (#32257)
* chore: cherry-pick f781748dcb3c from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
1 parent
fe8508a
commit f74940b
Showing
2 changed files
with
196 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,195 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Ravjit <ravjit@chromium.org> | ||
Date: Thu, 16 Sep 2021 12:29:55 +0000 | ||
Subject: Show the origin of the last redirecting server in external protocol | ||
handler dialogs | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
External protocol handlers always show the initiating url's origin. This can be misleading if there are server side redirects. | ||
Now we will show the origin of the last redirecting server (falling back to the request_initiator if there were no redirects / if the request goes straight to an external protocol). | ||
|
||
Bug: 1197889 | ||
Change-Id: I3cf7ccf3a8bd79d161364680a1871d1c88bec813 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3113931 | ||
Commit-Queue: Ravjit Singh Uppal <ravjit@chromium.org> | ||
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> | ||
Reviewed-by: Collin Baker <collinbaker@chromium.org> | ||
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> | ||
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> | ||
Cr-Commit-Position: refs/heads/main@{#922096} | ||
|
||
diff --git a/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc b/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc | ||
index 7e953fe89ff8dadf4da9578c0fdc6c3aada11aca..5c7e331ea4fbbcf36ac6463b37884cc9c6ca7e41 100644 | ||
--- a/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc | ||
+++ b/chrome/browser/ui/views/external_protocol_dialog_browsertest.cc | ||
@@ -14,11 +14,15 @@ | ||
#include "chrome/browser/ui/test/test_browser_dialog.h" | ||
#include "chrome/browser/ui/views/external_protocol_dialog.h" | ||
#include "chrome/test/base/in_process_browser_test.h" | ||
+#include "chrome/test/base/ui_test_utils.h" | ||
#include "content/public/browser/render_frame_host.h" | ||
#include "content/public/browser/render_process_host.h" | ||
#include "content/public/browser/render_view_host.h" | ||
#include "content/public/browser/web_contents.h" | ||
#include "content/public/test/browser_test.h" | ||
+#include "net/dns/mock_host_resolver.h" | ||
+#include "net/test/embedded_test_server/http_request.h" | ||
+#include "net/test/embedded_test_server/http_response.h" | ||
#include "ui/views/controls/button/checkbox.h" | ||
#include "url/gurl.h" | ||
|
||
@@ -41,6 +45,33 @@ class ExternalProtocolDialogTestApi { | ||
|
||
} // namespace test | ||
|
||
+namespace { | ||
+constexpr char kInitiatingOrigin[] = "a.test"; | ||
+constexpr char kRedirectingOrigin[] = "b.test"; | ||
+ | ||
+class FakeDefaultProtocolClientWorker | ||
+ : public shell_integration::DefaultProtocolClientWorker { | ||
+ public: | ||
+ explicit FakeDefaultProtocolClientWorker(const std::string& protocol) | ||
+ : DefaultProtocolClientWorker(protocol) {} | ||
+ FakeDefaultProtocolClientWorker(const FakeDefaultProtocolClientWorker&) = | ||
+ delete; | ||
+ FakeDefaultProtocolClientWorker& operator=( | ||
+ const FakeDefaultProtocolClientWorker&) = delete; | ||
+ | ||
+ private: | ||
+ ~FakeDefaultProtocolClientWorker() override = default; | ||
+ shell_integration::DefaultWebClientState CheckIsDefaultImpl() override { | ||
+ return shell_integration::DefaultWebClientState::NOT_DEFAULT; | ||
+ } | ||
+ | ||
+ void SetAsDefaultImpl(base::OnceClosure on_finished_callback) override { | ||
+ base::SequencedTaskRunnerHandle::Get()->PostTask( | ||
+ FROM_HERE, std::move(on_finished_callback)); | ||
+ } | ||
+}; | ||
+} // namespace | ||
+ | ||
class ExternalProtocolDialogBrowserTest | ||
: public DialogBrowserTest, | ||
public ExternalProtocolHandler::Delegate { | ||
@@ -71,11 +102,11 @@ class ExternalProtocolDialogBrowserTest | ||
// ExternalProtocolHander::Delegate: | ||
scoped_refptr<shell_integration::DefaultProtocolClientWorker> | ||
CreateShellWorker(const std::string& protocol) override { | ||
- return nullptr; | ||
+ return base::MakeRefCounted<FakeDefaultProtocolClientWorker>(protocol); | ||
} | ||
ExternalProtocolHandler::BlockState GetBlockState(const std::string& scheme, | ||
Profile* profile) override { | ||
- return ExternalProtocolHandler::DONT_BLOCK; | ||
+ return ExternalProtocolHandler::UNKNOWN; | ||
} | ||
void BlockRequest() override {} | ||
void RunExternalProtocolDialog( | ||
@@ -83,7 +114,10 @@ class ExternalProtocolDialogBrowserTest | ||
content::WebContents* web_contents, | ||
ui::PageTransition page_transition, | ||
bool has_user_gesture, | ||
- const absl::optional<url::Origin>& initiating_origin) override {} | ||
+ const absl::optional<url::Origin>& initiating_origin) override { | ||
+ url_did_launch_ = true; | ||
+ launch_url_ = initiating_origin->host(); | ||
+ } | ||
void LaunchUrlWithoutSecurityCheck( | ||
const GURL& url, | ||
content::WebContents* web_contents) override { | ||
@@ -98,6 +132,12 @@ class ExternalProtocolDialogBrowserTest | ||
blocked_state_ = state; | ||
} | ||
|
||
+ void SetUpOnMainThread() override { | ||
+ DialogBrowserTest::SetUpOnMainThread(); | ||
+ host_resolver()->AddRule(kInitiatingOrigin, "127.0.0.1"); | ||
+ host_resolver()->AddRule(kRedirectingOrigin, "127.0.0.1"); | ||
+ } | ||
+ | ||
base::HistogramTester histogram_tester_; | ||
|
||
protected: | ||
@@ -106,6 +146,7 @@ class ExternalProtocolDialogBrowserTest | ||
url::Origin blocked_origin_; | ||
BlockState blocked_state_ = BlockState::UNKNOWN; | ||
bool url_did_launch_ = false; | ||
+ std::string launch_url_; | ||
|
||
private: | ||
DISALLOW_COPY_AND_ASSIGN(ExternalProtocolDialogBrowserTest); | ||
@@ -231,3 +272,21 @@ IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, TestFocus) { | ||
const views::View* focused_view = focus_manager->GetFocusedView(); | ||
EXPECT_TRUE(focused_view); | ||
} | ||
+ | ||
+IN_PROC_BROWSER_TEST_F(ExternalProtocolDialogBrowserTest, OriginNameTest) { | ||
+ ASSERT_TRUE(embedded_test_server()->Start()); | ||
+ content::WebContents* web_contents = | ||
+ browser()->tab_strip_model()->GetActiveWebContents(); | ||
+ EXPECT_TRUE(ui_test_utils::NavigateToURL( | ||
+ browser(), embedded_test_server()->GetURL("a.test", "/empty.html"))); | ||
+ EXPECT_TRUE(content::ExecJs( | ||
+ web_contents, | ||
+ content::JsReplace("location.href = $1", | ||
+ embedded_test_server()->GetURL( | ||
+ "b.test", "/server-redirect?ms-calc:")))); | ||
+ content::WaitForLoadStop(web_contents); | ||
+ EXPECT_TRUE(url_did_launch_); | ||
+ // The url should be the url of the last redirecting server and not of the | ||
+ // request initiator | ||
+ EXPECT_EQ(launch_url_, "b.test"); | ||
+} | ||
diff --git a/content/browser/loader/navigation_url_loader_impl.cc b/content/browser/loader/navigation_url_loader_impl.cc | ||
index 42f62ccb3830bf718806f2361d9fb8a4abd53c6a..3d91eadbed7fc9cfd9a70805415eb660b15398d4 100644 | ||
--- a/content/browser/loader/navigation_url_loader_impl.cc | ||
+++ b/content/browser/loader/navigation_url_loader_impl.cc | ||
@@ -626,6 +626,13 @@ NavigationURLLoaderImpl::PrepareForNonInterceptedRequest( | ||
if (known_schemes_.find(resource_request_->url.scheme()) == | ||
known_schemes_.end()) { | ||
mojo::PendingRemote<network::mojom::URLLoaderFactory> loader_factory; | ||
+ absl::optional<url::Origin> initiating_origin; | ||
+ if (url_chain_.size() > 1) { | ||
+ initiating_origin = | ||
+ url::Origin::Create(url_chain_[url_chain_.size() - 2]); | ||
+ } else { | ||
+ initiating_origin = resource_request_->request_initiator; | ||
+ } | ||
bool handled = GetContentClient()->browser()->HandleExternalProtocol( | ||
resource_request_->url, web_contents_getter_, | ||
ChildProcessHost::kInvalidUniqueID, frame_tree_node_id_, | ||
@@ -633,8 +640,8 @@ NavigationURLLoaderImpl::PrepareForNonInterceptedRequest( | ||
resource_request_->resource_type == | ||
static_cast<int>(blink::mojom::ResourceType::kMainFrame), | ||
static_cast<ui::PageTransition>(resource_request_->transition_type), | ||
- resource_request_->has_user_gesture, | ||
- resource_request_->request_initiator, &loader_factory); | ||
+ resource_request_->has_user_gesture, initiating_origin, | ||
+ &loader_factory); | ||
|
||
if (loader_factory) { | ||
factory = base::MakeRefCounted<network::WrapperSharedURLLoaderFactory>( | ||
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h | ||
index 86b630ca2fa26150d71451a1ffe77552cdd589a9..c8981fea3bf8596797d68a3a164309444bb6e1ec 100644 | ||
--- a/content/public/browser/content_browser_client.h | ||
+++ b/content/public/browser/content_browser_client.h | ||
@@ -1758,10 +1758,12 @@ class CONTENT_EXPORT ContentBrowserClient { | ||
// Otherwise child_id will be the process id and |navigation_ui_data| will be | ||
// nullptr. | ||
// | ||
- // |initiating_origin| is the origin that initiated the navigation to the | ||
- // external protocol, and may be null, e.g. in the case of browser-initiated | ||
- // navigations. The initiating origin is intended to help users make security | ||
- // decisions about whether to allow an external application to launch. | ||
+ // |initiating_origin| is the origin of the last redirecting server (falling | ||
+ // back to the request initiator if there were no redirects / if the request | ||
+ // goes straight to an external protocol, or null, e.g. in the case of | ||
+ // browser-initiated navigations. The initiating origin is intended to help | ||
+ // users make security decisions about whether to allow an external | ||
+ // application to launch. | ||
virtual bool HandleExternalProtocol( | ||
const GURL& url, | ||
base::RepeatingCallback<WebContents*()> web_contents_getter, |