diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc index d8f6b9d1214d..35ff1af5aba9 100644 --- a/browser/brave_content_browser_client.cc +++ b/browser/brave_content_browser_client.cc @@ -1346,11 +1346,11 @@ blink::UserAgentMetadata BraveContentBrowserClient::GetUserAgentMetadata() { return metadata; } -GURL BraveContentBrowserClient::SanitizeURL( +std::optional BraveContentBrowserClient::SanitizeURL( content::RenderFrameHost* render_frame_host, const GURL& url) { if (!base::FeatureList::IsEnabled(features::kBraveCopyCleanLinkFromJs)) { - return url; + return std::nullopt; } CHECK(render_frame_host); CHECK(render_frame_host->GetBrowserContext()); @@ -1360,9 +1360,14 @@ GURL BraveContentBrowserClient::SanitizeURL( CHECK(url_sanitizer_service); if (!url_sanitizer_service->CheckJsPermission( render_frame_host->GetLastCommittedURL())) { - return url; + return std::nullopt; + } + const GURL& sanitized_url = url_sanitizer_service->SanitizeURL(url); + if (sanitized_url == url) { + // No actual replacements were made. + return std::nullopt; } - return url_sanitizer_service->SanitizeURL(url); + return sanitized_url; } bool BraveContentBrowserClient::AllowSignedExchange( diff --git a/browser/brave_content_browser_client.h b/browser/brave_content_browser_client.h index ecd6535e1892..da6540947cc7 100644 --- a/browser/brave_content_browser_client.h +++ b/browser/brave_content_browser_client.h @@ -171,8 +171,8 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient { blink::web_pref::WebPreferences* prefs) override; blink::UserAgentMetadata GetUserAgentMetadata() override; - GURL SanitizeURL(content::RenderFrameHost* render_frame_host, - const GURL& url) override; + std::optional SanitizeURL(content::RenderFrameHost* render_frame_host, + const GURL& url) override; bool AllowSignedExchange(content::BrowserContext* context) override; diff --git a/browser/url_sanitizer/url_sanitizer_browsertest.cc b/browser/url_sanitizer/url_sanitizer_browsertest.cc index a536b2c4866f..108a3b76867d 100644 --- a/browser/url_sanitizer/url_sanitizer_browsertest.cc +++ b/browser/url_sanitizer/url_sanitizer_browsertest.cc @@ -148,7 +148,8 @@ class URLSanitizerTestBase : public InProcessBrowserTest { const GURL url("https://www.YoUtUbE.com/url_sanitizer/js_api.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); - auto check = [this](const std::string& name, bool should_sanitize) { + auto check = [this](const std::string& name, bool should_sanitize, + const std::string& expected_text) { { ui::ScopedClipboardWriter clear_clipboard( ui::ClipboardBuffer::kCopyPaste); @@ -161,11 +162,11 @@ class URLSanitizerTestBase : public InProcessBrowserTest { constexpr const char kClickButton[] = R"js( - (function() { - const button = document.getElementById('$1'); - button.click(); - })(); - )js"; + (function() { + const button = document.getElementById('$1'); + button.click(); + })(); + )js"; const auto script = base::ReplaceStringPlaceholders(kClickButton, {name}, nullptr); web_contents->Focus(); @@ -173,36 +174,45 @@ class URLSanitizerTestBase : public InProcessBrowserTest { const std::string text_from_clipboard = WaitClipboard(); - EXPECT_TRUE(base::StartsWith(text_from_clipboard, "https://youtu.be/")) + EXPECT_EQ(expected_text, text_from_clipboard) << name << " " << should_sanitize << " " << text_from_clipboard; - - if (should_sanitize) { - EXPECT_EQ(std::string::npos, text_from_clipboard.find("si=")) - << name << " " << should_sanitize << " " << text_from_clipboard; - } else { - EXPECT_NE(std::string::npos, text_from_clipboard.find("si=")) - << name << " " << should_sanitize << " " << text_from_clipboard; - } }; const bool should_sanitize = base::FeatureList::IsEnabled(features::kBraveCopyCleanLinkFromJs); - check("test_1", should_sanitize); - check("test_2", should_sanitize); - check("test_3", should_sanitize); - check("test_4", should_sanitize); - check("test_5", should_sanitize); + const std::string sanitized = "https://youtu.be/B"; + const std::string unsanitized = "hTtPs://Youtu.Be/B?si=oLb865I64uJlLRJX"; + const std::string& expected = (should_sanitize) ? sanitized : unsanitized; + + check("test_1", should_sanitize, expected); + check("test_2", should_sanitize, expected); + check("test_3", should_sanitize, expected); + check("test_4", should_sanitize, expected); + check("test_5", should_sanitize, expected); + // We cannot distinguish the context, so even if a password similar to the + // URL is copied, we will sanitize it. + check("test_sanitizable_password", should_sanitize, expected); + // Not sanitazable password should be copied as is. + check("test_not_sanitizable_password_1", should_sanitize, "Pa$$w0rd"); + check("test_not_sanitizable_password_2", should_sanitize, "A:^C,D"); + check("test_not_sanitizable_password_3", should_sanitize, + "Ftp://Example.Com/?si=12345"); const GURL no_permission_url( "https://no_permission.com/url_sanitizer/js_api.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), no_permission_url)); - check("test_1", false); - check("test_2", false); - check("test_3", false); - check("test_4", false); - check("test_5", false); + check("test_1", false, unsanitized); + check("test_2", false, unsanitized); + check("test_3", false, unsanitized); + check("test_4", false, unsanitized); + check("test_5", false, unsanitized); + check("test_sanitizable_password", false, unsanitized); + check("test_not_sanitizable_password_1", false, "Pa$$w0rd"); + check("test_not_sanitizable_password_2", false, "A:^C,D"); + check("test_not_sanitizable_password_3", false, + "Ftp://Example.Com/?si=12345"); } protected: diff --git a/chromium_src/content/browser/renderer_host/clipboard_host_impl.cc b/chromium_src/content/browser/renderer_host/clipboard_host_impl.cc index fdb7b4845eaa..b7d40a25a0b5 100644 --- a/chromium_src/content/browser/renderer_host/clipboard_host_impl.cc +++ b/chromium_src/content/browser/renderer_host/clipboard_host_impl.cc @@ -23,9 +23,13 @@ std::u16string Sanitize(content::ContentBrowserClient* client, } const GURL url(data); - if (url.is_valid() && !url.is_empty()) { - const GURL sanitized_url = client->SanitizeURL(render_frame_host, url); - return base::UTF8ToUTF16(sanitized_url.spec()); + if (url.is_valid() && !url.is_empty() && url.SchemeIsHTTPOrHTTPS()) { + std::optional sanitized_url = + client->SanitizeURL(render_frame_host, url); + if (!sanitized_url) { + return data; + } + return base::UTF8ToUTF16(sanitized_url->spec()); } return data; } diff --git a/chromium_src/content/public/browser/content_browser_client.cc b/chromium_src/content/public/browser/content_browser_client.cc index 5263004304a7..00afc1224637 100644 --- a/chromium_src/content/public/browser/content_browser_client.cc +++ b/chromium_src/content/public/browser/content_browser_client.cc @@ -32,9 +32,9 @@ uint8_t ContentBrowserClient::WorkerGetBraveFarblingLevel( return 1 /* OFF */; } -GURL ContentBrowserClient::SanitizeURL(content::RenderFrameHost*, - const GURL& url) { - return url; +std::optional ContentBrowserClient::SanitizeURL(content::RenderFrameHost*, + const GURL& url) { + return std::nullopt; } } // namespace content diff --git a/chromium_src/content/public/browser/content_browser_client.h b/chromium_src/content/public/browser/content_browser_client.h index 18062069b85d..51b7445daa51 100644 --- a/chromium_src/content/public/browser/content_browser_client.h +++ b/chromium_src/content/public/browser/content_browser_client.h @@ -27,7 +27,8 @@ BrowserContext* browser_context); \ virtual uint8_t WorkerGetBraveFarblingLevel( \ const GURL& url, BrowserContext* browser_context); \ - virtual GURL SanitizeURL(content::RenderFrameHost*, const GURL&); \ + virtual std::optional SanitizeURL(content::RenderFrameHost*, \ + const GURL&); \ virtual void SetBrowserStartupIsCompleteForTesting #include "src/content/public/browser/content_browser_client.h" // IWYU pragma: export diff --git a/test/data/url_sanitizer/js_api.html b/test/data/url_sanitizer/js_api.html index d0dcaaa83d0a..459357452f03 100644 --- a/test/data/url_sanitizer/js_api.html +++ b/test/data/url_sanitizer/js_api.html @@ -9,6 +9,8 @@ @@ -61,28 +80,27 @@
Put any text you want, then click 'Copy'. Data should be sanitized.
API: document.exec('copy')
- +

-
+
Copy from text area
+
Data should be sanitized.
+
API: document.exec('copy')
-
Copy from text area
-
Data should be sanitized.
-
API: document.exec('copy')
-
- -
- + +
+

Navigator Clipboard API
-
https://youtu.be/E?si=oLb865I64uJlLRJX
+
hTtPs://Youtu.Be/B?si=oLb865I64uJlLRJX
API: navigator.clipboard.writeText
@@ -91,7 +109,7 @@
Copy from the temporary textarea
-
https://youtu.be/F?si=oLb865I64uJlLRJX
+
hTtPs://Youtu.Be/B?si=oLb865I64uJlLRJX
API: document.exec('copy')
@@ -100,11 +118,38 @@
Copy from div
-
https://youtu.be/G?si=oLb865I64uJlLRJX
+
hTtPs://Youtu.Be/B?si=oLb865I64uJlLRJX
API: document.exec('copy')
+
+
+
+
Copy sanitizable password
+ +
+
+
+
+
Copy not sanitizable password
+ +
+ +
+
+
+
Copy not sanitizable password
+ +
+ +
+
+
+
Copy not sanitizable password
+ +
+ \ No newline at end of file