diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index a07110b2c87860..2655e56118acfe 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn @@ -1795,6 +1795,8 @@ static_library("browser") { "url_param_filter/cross_otr_observer.h", "url_param_filter/url_param_classifications_loader.cc", "url_param_filter/url_param_classifications_loader.h", + "url_param_filter/url_param_filter_throttle.cc", + "url_param_filter/url_param_filter_throttle.h", "url_param_filter/url_param_filterer.cc", "url_param_filter/url_param_filterer.h", "usb/frame_usb_services.cc", diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 22b999853b06ba..993c9614909cbf 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -143,6 +143,7 @@ #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h" #include "chrome/browser/ui/webui/log_web_ui_url.h" #include "chrome/browser/universal_web_contents_observers.h" +#include "chrome/browser/url_param_filter/url_param_filter_throttle.h" #include "chrome/browser/usb/frame_usb_services.h" #include "chrome/browser/vr/vr_tab_helper.h" #include "chrome/browser/web_applications/policy/web_app_policy_manager.h" @@ -4638,6 +4639,9 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles( ChromeNavigationUIData* chrome_navigation_ui_data = static_cast(navigation_ui_data); + url_param_filter::UrlParamFilterThrottle::MaybeCreateThrottle( + wc_getter.Run(), request, &result); + #if BUILDFLAG(SAFE_BROWSING_AVAILABLE) bool matches_enterprise_allowlist = safe_browsing::IsURLAllowlistedByPolicy( request.url, *profile->GetPrefs()); diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc index c037f9714c26cd..f7d16b3bb1252c 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -91,7 +91,6 @@ #include "chrome/browser/ui/web_applications/app_browser_controller.h" #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h" #include "chrome/browser/ui/webui/history/foreign_session_handler.h" -#include "chrome/browser/url_param_filter/url_param_filterer.h" #include "chrome/browser/web_applications/system_web_apps/system_web_app_delegate.h" #include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h" #include "chrome/browser/web_applications/web_app_helpers.h" @@ -2442,9 +2441,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { case IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD: // Pass along the |referring_url| so we can show it in browser UI. Note // that this won't and shouldn't be sent via the referrer header. - OpenURLWithExtraHeaders(url_param_filter::FilterUrl( - GetDocumentURL(params_), params_.link_url), - GetDocumentURL(params_), + OpenURLWithExtraHeaders(params_.link_url, GetDocumentURL(params_), WindowOpenDisposition::OFF_THE_RECORD, ui::PAGE_TRANSITION_LINK, "" /* extra_headers */, true /* started_from_context_menu */); diff --git a/chrome/browser/url_param_filter/cross_otr_observer.cc b/chrome/browser/url_param_filter/cross_otr_observer.cc index 2613591eb1c121..36ab753b8d762a 100644 --- a/chrome/browser/url_param_filter/cross_otr_observer.cc +++ b/chrome/browser/url_param_filter/cross_otr_observer.cc @@ -14,6 +14,7 @@ #include "content/public/browser/web_contents_observer.h" #include "net/http/http_response_headers.h" #include "net/http/http_util.h" +#include "ui/base/page_transition_types.h" namespace url_param_filter { @@ -40,14 +41,41 @@ CrossOtrObserver::CrossOtrObserver(content::WebContents* web_contents) : content::WebContentsObserver(web_contents), content::WebContentsUserData(*web_contents) {} +bool CrossOtrObserver::IsCrossOtrState() { + return protecting_navigations_; +} + +void CrossOtrObserver::DidStartNavigation( + content::NavigationHandle* navigation_handle) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + // If we've already observed the end of a navigation, and the navigation is in + // the primary main frame, and it is not the result of a client redirect, + // we've finished the cross-OTR case. Note that observing user activation + // would also serve to stop the protecting_navigations_ case. Note that + // refreshes after page load also trigger this, and thus are not at risk of + // being considered part of the cross-OTR case. + if (observed_response_ && navigation_handle->IsInPrimaryMainFrame() && + !(navigation_handle->GetPageTransition() & + ui::PAGE_TRANSITION_CLIENT_REDIRECT)) { + protecting_navigations_ = false; + } +} + void CrossOtrObserver::DidFinishNavigation( content::NavigationHandle* navigation_handle) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - // We only want the first navigation to be counted; after that point, no + if (!navigation_handle->IsInPrimaryMainFrame() || + navigation_handle->IsSameDocument()) { + // We only are concerned with top-level, non-same doc navigations. + return; + } + + // We only want the first navigation, including client redirects occurring + // without having observed user activation, to be counted; after that, no // response codes should be tracked. The observer is left in place to track // refreshes on the first page. - if (!wrote_response_metric_) { - wrote_response_metric_ = true; + if (protecting_navigations_) { + observed_response_ = true; const net::HttpResponseHeaders* headers = navigation_handle->GetResponseHeaders(); if (headers) { @@ -57,9 +85,7 @@ void CrossOtrObserver::DidFinishNavigation( } } else if (navigation_handle->GetReloadType() != content::ReloadType::NONE) { refresh_count_++; - } else if (navigation_handle->IsInPrimaryMainFrame() && - !navigation_handle->IsSameDocument() && - navigation_handle->HasCommitted()) { + } else if (navigation_handle->HasCommitted() && !protecting_navigations_) { Detach(); // DO NOT add code past this point. `this` is destroyed. } @@ -68,11 +94,18 @@ void CrossOtrObserver::DidFinishNavigation( void CrossOtrObserver::DidRedirectNavigation( content::NavigationHandle* navigation_handle) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + if (!navigation_handle->IsInPrimaryMainFrame() || + navigation_handle->IsSameDocument()) { + // We only are concerned with top-level, non-same doc navigations. + return; + } + const net::HttpResponseHeaders* headers = navigation_handle->GetResponseHeaders(); - // After the first navigation has committed, we no longer want to track + // After the first full navigation has committed, including any client + // redirects that occur without user activation, we no longer want to track // redirects. - if (!wrote_response_metric_ && headers) { + if (protecting_navigations_ && headers) { base::UmaHistogramSparse( kCrossOtrResponseCodeMetricName, net::HttpUtil::MapStatusCodeForHistogram(headers->response_code())); @@ -86,6 +119,14 @@ void CrossOtrObserver::WebContentsDestroyed() { // DO NOT add code past this point. `this` is destroyed. } +void CrossOtrObserver::FrameReceivedUserActivation( + content::RenderFrameHost* render_frame_host) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + // Anytime the user activates a frame in the web contents, we cease to + // consider the case cross-OTR. + protecting_navigations_ = false; +} + void CrossOtrObserver::Detach() { base::UmaHistogramCounts100(kCrossOtrRefreshCountMetricName, refresh_count_); web_contents()->RemoveUserData(CrossOtrObserver::UserDataKey()); diff --git a/chrome/browser/url_param_filter/cross_otr_observer.h b/chrome/browser/url_param_filter/cross_otr_observer.h index 6a7839e7b61452..d8ec2424b7ac03 100644 --- a/chrome/browser/url_param_filter/cross_otr_observer.h +++ b/chrome/browser/url_param_filter/cross_otr_observer.h @@ -12,7 +12,14 @@ namespace url_param_filter { // Observes navigations that originate in normal browsing and move into OTR -// browsing. +// browsing. This class can be thought of as a state machine: +// start-->blocking-->monitoring-->detached +// where the initial cross-OTR navigation moves to blocking; user activation or +// the start of a second navigation not initiated via client redirect moves to +// monitoring; and the next completed non-refresh navigation after that point, +// regardless of cause, detaches. Note that for our purposes, navigation above +// refers to top-level, main frame navigations only; we do not consider e.g., +// subframe loads. class CrossOtrObserver : public content::WebContentsObserver, public content::WebContentsUserData { public: @@ -20,12 +27,17 @@ class CrossOtrObserver : public content::WebContentsObserver, // unchanged otherwise. static void MaybeCreateForWebContents(content::WebContents* web_contents, const NavigateParams& params); + bool IsCrossOtrState(); // content::WebContentsObserver: + void DidStartNavigation( + content::NavigationHandle* navigation_handle) override; void DidFinishNavigation( content::NavigationHandle* navigation_handle) override; void DidRedirectNavigation( content::NavigationHandle* navigation_handle) override; void WebContentsDestroyed() override; + void FrameReceivedUserActivation( + content::RenderFrameHost* render_frame_host) override; private: explicit CrossOtrObserver(content::WebContents* web_contents); @@ -39,11 +51,17 @@ class CrossOtrObserver : public content::WebContentsObserver, // Drives state machine logic; we write the cross-OTR response code metric // only for the first navigation, which is that which would have parameters // filtered. - bool wrote_response_metric_ = false; + bool observed_response_ = false; // Tracks refreshes observed, which could point to an issue with param // filtering causing unexpected behavior for the user. int refresh_count_ = 0; + // Whether top-level navigations should have filtering applied. Starts true, + // and switched to false once a navigation completes and then either: + // user interaction is observed or a new navigation starts that is not a + // client redirect. + bool protecting_navigations_ = true; + WEB_CONTENTS_USER_DATA_KEY_DECL(); }; diff --git a/chrome/browser/url_param_filter/cross_otr_observer_unittest.cc b/chrome/browser/url_param_filter/cross_otr_observer_unittest.cc index 39a7754cff9784..349deaa3827002 100644 --- a/chrome/browser/url_param_filter/cross_otr_observer_unittest.cc +++ b/chrome/browser/url_param_filter/cross_otr_observer_unittest.cc @@ -138,7 +138,10 @@ TEST_F(CrossOtrObserverTest, FinishedNavigation) { scoped_refptr response = base::MakeRefCounted("HTTP/1.1 200 OK"); handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); + + ASSERT_TRUE(observer->IsCrossOtrState()); histogram_tester.ExpectTotalCount(kResponseCodeMetricName, 1); histogram_tester.ExpectUniqueSample( kResponseCodeMetricName, net::HttpUtil::MapStatusCodeForHistogram(200), @@ -177,6 +180,7 @@ TEST_F(CrossOtrObserverTest, BadNavigationResponse) { std::make_unique>(contents); handle->set_response_headers(nullptr); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); histogram_tester.ExpectTotalCount(kResponseCodeMetricName, 0); @@ -202,9 +206,11 @@ TEST_F(CrossOtrObserverTest, RefreshedAfterNavigation) { scoped_refptr response = base::MakeRefCounted("HTTP/1.1 200 OK"); handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); handle->set_reload_type(content::ReloadType::NORMAL); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); observer->WebContentsDestroyed(); @@ -232,7 +238,9 @@ TEST_F(CrossOtrObserverTest, UncommittedNavigationWithRefresh) { scoped_refptr response = base::MakeRefCounted("HTTP/1.1 200 OK"); handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); + ASSERT_TRUE(observer->IsCrossOtrState()); // Finish a non-reload navigation, but one that isn't committed (so no actual // navigation away from the monitored page) @@ -241,10 +249,15 @@ TEST_F(CrossOtrObserverTest, UncommittedNavigationWithRefresh) { handle->set_is_same_document(false); handle->set_has_committed(false); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); + // We just observed another navigation not due to a client redirect, so should + // no longer be in the cross-OTR state. + ASSERT_FALSE(observer->IsCrossOtrState()); // After that uncommitted navigation, trigger a redirect, then destroy. handle->set_reload_type(content::ReloadType::NORMAL); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); observer->WebContentsDestroyed(); @@ -273,11 +286,16 @@ TEST_F(CrossOtrObserverTest, MultipleRefreshesAfterNavigation) { scoped_refptr response = base::MakeRefCounted("HTTP/1.1 200 OK"); handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); // Reload twice and ensure the count is persisted. handle->set_reload_type(content::ReloadType::NORMAL); + observer->DidStartNavigation(handle.get()); + // With the refresh navigation started, we are no longer in cross-OTR mode. + ASSERT_FALSE(observer->IsCrossOtrState()); observer->DidFinishNavigation(handle.get()); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); // Navigating away means no more observer. @@ -285,6 +303,7 @@ TEST_F(CrossOtrObserverTest, MultipleRefreshesAfterNavigation) { handle->set_is_in_primary_main_frame(true); handle->set_is_same_document(false); handle->set_has_committed(true); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); ASSERT_EQ(CrossOtrObserver::FromWebContents(contents), nullptr); @@ -313,10 +332,20 @@ TEST_F(CrossOtrObserverTest, RedirectsAfterNavigation) { scoped_refptr response = base::MakeRefCounted("HTTP/1.1 200 OK"); handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); observer->DidFinishNavigation(handle.get()); + // The first navigation has finished, but we remain cross-OTR until either + // user activation or a non-client redirect navigation begins + ASSERT_TRUE(observer->IsCrossOtrState()); - // Redirects observed after the first navigation has committed should not + // Redirects observed on navigations after the first should not // write responses. + observer->DidStartNavigation(handle.get()); + + // A new, non-client redirect navigation began, so we should no longer be + // filtering. + ASSERT_FALSE(observer->IsCrossOtrState()); + response = base::MakeRefCounted( "HTTP/1.1 302 Moved Temporarily"); handle->set_response_headers(response); @@ -327,4 +356,77 @@ TEST_F(CrossOtrObserverTest, RedirectsAfterNavigation) { kResponseCodeMetricName, net::HttpUtil::MapStatusCodeForHistogram(200), 1); } +TEST_F(CrossOtrObserverTest, ClientRedirectCrossOtr) { + base::HistogramTester histogram_tester; + NavigateParams params(profile(), GURL("https://www.foo.com"), + ui::PAGE_TRANSITION_LINK); + + params.started_from_context_menu = true; + params.privacy_sensitivity = NavigateParams::PrivacySensitivity::CROSS_OTR; + content::WebContents* contents = web_contents(); + CrossOtrObserver::MaybeCreateForWebContents(contents, params); + CrossOtrObserver* observer = CrossOtrObserver::FromWebContents(contents); + ASSERT_NE(observer, nullptr); + std::unique_ptr handle = + std::make_unique>(contents); + + scoped_refptr response = + base::MakeRefCounted("HTTP/1.1 200 OK"); + handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); + observer->DidFinishNavigation(handle.get()); + // The first navigation has finished, but we remain cross-OTR until either + // user activation or a non-client redirect navigation begins + ASSERT_TRUE(observer->IsCrossOtrState()); + + handle->set_page_transition(ui::PAGE_TRANSITION_CLIENT_REDIRECT); + observer->DidStartNavigation(handle.get()); + ASSERT_TRUE(observer->IsCrossOtrState()); + observer->DidFinishNavigation(handle.get()); + ASSERT_TRUE(observer->IsCrossOtrState()); + + handle->set_page_transition(ui::PAGE_TRANSITION_AUTO_BOOKMARK); + observer->DidStartNavigation(handle.get()); + // A new, non-client redirect navigation began, so we should no longer be + // filtering. + ASSERT_FALSE(observer->IsCrossOtrState()); +} +TEST_F(CrossOtrObserverTest, ClientRedirectAfterActivationNotCrossOtr) { + base::HistogramTester histogram_tester; + NavigateParams params(profile(), GURL("https://www.foo.com"), + ui::PAGE_TRANSITION_LINK); + + params.started_from_context_menu = true; + params.privacy_sensitivity = NavigateParams::PrivacySensitivity::CROSS_OTR; + content::WebContents* contents = web_contents(); + CrossOtrObserver::MaybeCreateForWebContents(contents, params); + CrossOtrObserver* observer = CrossOtrObserver::FromWebContents(contents); + ASSERT_NE(observer, nullptr); + std::unique_ptr handle = + std::make_unique>(contents); + + scoped_refptr response = + base::MakeRefCounted("HTTP/1.1 200 OK"); + handle->set_response_headers(response); + observer->DidStartNavigation(handle.get()); + observer->DidFinishNavigation(handle.get()); + // The first navigation has finished, but we remain cross-OTR until either + // user activation or a non-client redirect navigation begins + ASSERT_TRUE(observer->IsCrossOtrState()); + + handle->set_page_transition(ui::PAGE_TRANSITION_CLIENT_REDIRECT); + observer->DidStartNavigation(handle.get()); + ASSERT_TRUE(observer->IsCrossOtrState()); + observer->DidFinishNavigation(handle.get()); + ASSERT_TRUE(observer->IsCrossOtrState()); + + observer->FrameReceivedUserActivation(nullptr); + // Receiving user activation means we leave the cross-OTR state and instead + // allow client redirects to occur unfiltered. + ASSERT_FALSE(observer->IsCrossOtrState()); + handle->set_page_transition(ui::PAGE_TRANSITION_AUTO_BOOKMARK); + observer->DidStartNavigation(handle.get()); + // The client redirect should not reset the OTR state. + ASSERT_FALSE(observer->IsCrossOtrState()); +} } // namespace url_param_filter diff --git a/chrome/browser/url_param_filter/url_param_filter_browsertest.cc b/chrome/browser/url_param_filter/url_param_filter_browsertest.cc index e2e1a009db8a82..f9fca077a8abcc 100644 --- a/chrome/browser/url_param_filter/url_param_filter_browsertest.cc +++ b/chrome/browser/url_param_filter/url_param_filter_browsertest.cc @@ -15,7 +15,9 @@ #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" +#include "net/base/escape.h" #include "net/http/http_util.h" +#include "third_party/blink/public/common/input/web_mouse_event.h" class ContextMenuIncognitoFilterBrowserTest : public InProcessBrowserTest { public: @@ -31,7 +33,8 @@ class ContextMenuIncognitoFilterBrowserTest : public InProcessBrowserTest { // or a source of: foo.com with outgoing param plzblock1 std::string encoded_classification = url_param_filter:: CreateBase64EncodedFilterParamClassificationForTesting( - {{"foo.com", {"plzblock1"}}}, {{"", {"plzblock"}}}); + {{"foo.com", {"plzblock1"}}, {"", {"plzblockredirect"}}}, + {{"", {"plzblock"}}}); scoped_feature_list_.InitAndEnableFeatureWithParameters( features::kIncognitoParamFilterEnabled, {{"classifications", encoded_classification}}); @@ -80,11 +83,241 @@ IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterBrowserTest, ASSERT_EQ(expected, tab->GetLastCommittedURL()); // The response was a 200, and the navigation went from normal-->OTR browsing. - histogram_tester.ExpectUniqueSample( + // Because we intervened, an artificial redirect was injected, so we also + // expect a 307. + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(307), 1); + histogram_tester.ExpectBucketCount( kCrossOtrResponseMetricName, net::HttpUtil::MapStatusCodeForHistogram(200), 1); } +// Enable "Open Link in Incognito Window" URL parameter filtering, and ensure +// that it filters as expected. +IN_PROC_BROWSER_TEST_F( + ContextMenuIncognitoFilterBrowserTest, + OpenIncognitoUrlParamFilterClientRedirectAfterActivation) { + base::HistogramTester histogram_tester; + + ui_test_utils::AllBrowserTabAddedWaiter add_tab; + + ASSERT_TRUE(embedded_test_server()->Start()); + GURL test_root(embedded_test_server()->GetURL( + "/empty.html?plzblock=1&nochanges=2&plzblock1=2")); + + // Go to a |page| with a link to a URL that has associated filtering rules. + GURL page("data:text/html,link"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), page)); + + // Set up the source URL to an eTLD+1 that also has a filtering rule. + const GURL kSource("http://foo.com/test"); + + // Set up menu with link URL. + content::ContextMenuParams context_menu_params; + context_menu_params.page_url = kSource; + context_menu_params.link_url = test_root; + + // Select "Open Link in Incognito Window" and wait for window to be added. + TestRenderViewContextMenu menu( + *browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(), + context_menu_params); + menu.Init(); + menu.ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD, 0); + + content::WebContents* tab = add_tab.Wait(); + EXPECT_TRUE(content::WaitForLoadStop(tab)); + + // Verify that it loaded the filtered URL. + GURL expected(embedded_test_server()->GetURL("/empty.html?nochanges=2")); + ASSERT_EQ(expected, tab->GetLastCommittedURL()); + + // Trigger user activation, at which point client redirects are no longer + // protected. + content::SimulateMouseClick(tab, 0, blink::WebMouseEvent::Button::kLeft); + content::LoadStopObserver client_redirect_load_observer(tab); + std::string script = "document.location.href=\"" + test_root.spec() + "\""; + ASSERT_TRUE(content::ExecJs(tab, script)); + client_redirect_load_observer.Wait(); + ASSERT_EQ(test_root, tab->GetLastCommittedURL()); +} + +// Enable "Open Link in Incognito Window" URL parameter filtering, and ensure +// that it filters as expected when server redirects are encountered. +IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterBrowserTest, + OpenIncognitoUrlParamFilterServerRedirect) { + base::HistogramTester histogram_tester; + + ui_test_utils::AllBrowserTabAddedWaiter add_tab; + + ASSERT_TRUE(embedded_test_server()->Start()); + // `plzblockredirect` is blocked on navigations from IP source domains; + // `plzblock` is blocked when an IP is the destination domain. + GURL test_root(embedded_test_server()->GetURL( + "/empty.html?plzblock=1&nochanges=2&plzblockredirect=2")); + GURL redirect_page(embedded_test_server()->GetURL( + "/server-redirect?" + + net::EscapeQueryParamValue(test_root.spec(), false))); + + // Go to a |page| with a link to a URL that has associated filtering rules. + GURL page("data:text/html,link"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), page)); + + // Set up the source URL to an eTLD+1 that also has a filtering rule. + const GURL kSource("http://foo.com/test"); + + // Set up menu with link URL. + content::ContextMenuParams context_menu_params; + context_menu_params.page_url = kSource; + context_menu_params.link_url = redirect_page; + + // Select "Open Link in Incognito Window" and wait for window to be added. + TestRenderViewContextMenu menu( + *browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(), + context_menu_params); + menu.Init(); + menu.ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD, 0); + + content::WebContents* tab = add_tab.Wait(); + EXPECT_TRUE(content::WaitForLoadStop(tab)); + + // Verify that it loaded the filtered URL. + GURL expected(embedded_test_server()->GetURL("/empty.html?nochanges=2")); + ASSERT_EQ(expected, tab->GetLastCommittedURL()); + + // The response was a 301-->200, and the navigation went from normal-->OTR + // browsing. Because we intervened, an artificial redirect was injected, so we + // also expect a 307. + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(301), 1); + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(307), 1); + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(200), 1); +} + +// Enable "Open Link in Incognito Window" URL parameter filtering, and ensure +// that it filters as expected when client redirects are encountered. +IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterBrowserTest, + OpenIncognitoUrlParamFilterClientRedirect) { + base::HistogramTester histogram_tester; + + ui_test_utils::AllBrowserTabAddedWaiter add_tab; + + ASSERT_TRUE(embedded_test_server()->Start()); + // `plzblock1` is blocked only on navs from foo.com. Because analysis will see + // this as a separate navigation, the source domain of the client redirect + // will be localhost. + GURL test_root( + embedded_test_server()->GetURL("/empty.html?plzblock=1&nochanges=2")); + GURL redirect_page(embedded_test_server()->GetURL( + "/client-redirect?" + + net::EscapeQueryParamValue(test_root.spec(), false))); + + // Go to a |page| with a link to a URL that has associated filtering rules. + GURL page("data:text/html,link"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), page)); + + // Set up the source URL to an eTLD+1 that also has a filtering rule. + const GURL kSource("http://foo.com/test"); + + // Set up menu with link URL. + content::ContextMenuParams context_menu_params; + context_menu_params.page_url = kSource; + context_menu_params.link_url = redirect_page; + + // Select "Open Link in Incognito Window" and wait for window to be added. + TestRenderViewContextMenu menu( + *browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(), + context_menu_params); + menu.Init(); + menu.ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD, 0); + + content::WebContents* tab = add_tab.Wait(); + EXPECT_TRUE(content::WaitForLoadStop(tab)); + // The prior load stop succeeds for the initial response; we now wait for the + // client redirect to occur. + content::LoadStopObserver client_redirect_load_observer(tab); + client_redirect_load_observer.Wait(); + // Verify that it loaded the filtered URL. + GURL expected(embedded_test_server()->GetURL("/empty.html?nochanges=2")); + ASSERT_EQ(expected, tab->GetLastCommittedURL()); + + // The response was a 200-->client redirect-->200, and the navigation went + // from normal-->OTR browsing. Because we intervened, an artificial redirect + // was injected, so we also expect a 307. + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(307), 1); + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(200), 2); +} + +// Enable "Open Link in Incognito Window" URL parameter filtering, and ensure +// that it filters as expected when client redirects are encountered. +IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterBrowserTest, + OpenIncognitoUrlParamFilterClientRedirectThenRefresh) { + base::HistogramTester histogram_tester; + + ui_test_utils::AllBrowserTabAddedWaiter add_tab; + + ASSERT_TRUE(embedded_test_server()->Start()); + GURL test_root( + embedded_test_server()->GetURL("/empty.html?plzblock=1&nochanges=2")); + GURL redirect_page(embedded_test_server()->GetURL( + "/client-redirect?" + + net::EscapeQueryParamValue(test_root.spec(), false))); + + // Go to a |page| with a link to a URL that has associated filtering rules. + GURL page("data:text/html,link"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), page)); + + // Set up the source URL to an eTLD+1 that also has a filtering rule. + const GURL kSource("http://foo.com/test"); + + // Set up menu with link URL. + content::ContextMenuParams context_menu_params; + context_menu_params.page_url = kSource; + context_menu_params.link_url = redirect_page; + + // Select "Open Link in Incognito Window" and wait for window to be added. + TestRenderViewContextMenu menu( + *browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(), + context_menu_params); + menu.Init(); + menu.ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD, 0); + + content::WebContents* tab = add_tab.Wait(); + EXPECT_TRUE(content::WaitForLoadStop(tab)); + // The prior load stop succeeds for the initial response; we now wait for the + // client redirect to occur. + content::LoadStopObserver client_redirect_load_observer(tab); + client_redirect_load_observer.Wait(); + // Verify that it loaded the filtered URL. + GURL expected(embedded_test_server()->GetURL("/empty.html?nochanges=2")); + ASSERT_EQ(expected, tab->GetLastCommittedURL()); + + tab->GetController().Reload(content::ReloadType::NORMAL, false); + EXPECT_TRUE(content::WaitForLoadStop(tab)); + tab->Close(); + + // The response was a 200-->client redirect-->200, and the navigation went + // from normal-->OTR browsing. Because we intervened, an artificial redirect + // was injected, so we also expect a 307. + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(307), 1); + histogram_tester.ExpectBucketCount( + kCrossOtrResponseMetricName, + net::HttpUtil::MapStatusCodeForHistogram(200), 2); + histogram_tester.ExpectTotalCount(kCrossOtrRefreshCountMetricName, 1); + ASSERT_EQ(histogram_tester.GetTotalSum(kCrossOtrRefreshCountMetricName), 1); +} + // Verify that appropriate metrics are written when redirects are encountered. IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterBrowserTest, OpenIncognitoUrlParamFilterRedirect) { diff --git a/chrome/browser/url_param_filter/url_param_filter_throttle.cc b/chrome/browser/url_param_filter/url_param_filter_throttle.cc new file mode 100644 index 00000000000000..1fb5a2808abc4f --- /dev/null +++ b/chrome/browser/url_param_filter/url_param_filter_throttle.cc @@ -0,0 +1,72 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/url_param_filter/url_param_filter_throttle.h" + +#include + +#include "chrome/browser/url_param_filter/cross_otr_observer.h" +#include "chrome/browser/url_param_filter/url_param_filterer.h" +#include "content/public/browser/browser_thread.h" +#include "net/url_request/redirect_info.h" +#include "services/network/public/cpp/resource_request.h" + +namespace url_param_filter { + +void UrlParamFilterThrottle::MaybeCreateThrottle( + content::WebContents* web_contents, + const network::ResourceRequest& request, + std::vector>* throttle_list) { + if (!web_contents) { + return; + } + // Only main frame navigations are in scope. We do not modify other + // navigations. + if (!request.is_main_frame) { + return; + } + CrossOtrObserver* observer = CrossOtrObserver::FromWebContents(web_contents); + if (observer && observer->IsCrossOtrState()) { + throttle_list->push_back( + std::make_unique(request.request_initiator)); + } +} + +UrlParamFilterThrottle::UrlParamFilterThrottle( + const absl::optional& request_initiator_origin) { + last_hop_initiator_ = request_initiator_origin.has_value() + ? request_initiator_origin->GetURL() + : GURL(); +} +UrlParamFilterThrottle::~UrlParamFilterThrottle() = default; + +void UrlParamFilterThrottle::DetachFromCurrentSequence() {} + +void UrlParamFilterThrottle::WillStartRequest(network::ResourceRequest* request, + bool* defer) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + request->url = FilterUrl(last_hop_initiator_, request->url); + last_hop_initiator_ = request->url; +} + +void UrlParamFilterThrottle::WillRedirectRequest( + net::RedirectInfo* redirect_info, + const network::mojom::URLResponseHead& response_head, + bool* defer, + std::vector* to_be_removed_request_headers, + net::HttpRequestHeaders* modified_request_headers, + net::HttpRequestHeaders* modified_cors_exempt_request_headers) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + redirect_info->new_url = + FilterUrl(last_hop_initiator_, redirect_info->new_url); + // Future redirects should use the redirect's domain as the navigation source. + last_hop_initiator_ = redirect_info->new_url; +} + +bool UrlParamFilterThrottle::makes_unsafe_redirect() { + // Scheme changes are not possible with this throttle. Only URL params are + // modified. + return false; +} +} // namespace url_param_filter diff --git a/chrome/browser/url_param_filter/url_param_filter_throttle.h b/chrome/browser/url_param_filter/url_param_filter_throttle.h new file mode 100644 index 00000000000000..199b6e2894f2bd --- /dev/null +++ b/chrome/browser/url_param_filter/url_param_filter_throttle.h @@ -0,0 +1,49 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_URL_PARAM_FILTER_URL_PARAM_FILTER_THROTTLE_H_ +#define CHROME_BROWSER_URL_PARAM_FILTER_URL_PARAM_FILTER_THROTTLE_H_ + +#include + +#include "content/public/browser/web_contents.h" +#include "third_party/abseil-cpp/absl/types/optional.h" +#include "third_party/blink/public/common/loader/url_loader_throttle.h" +#include "url/origin.h" + +namespace url_param_filter { +class UrlParamFilterThrottle : public blink::URLLoaderThrottle { + public: + // Create the throttle if conditions warrant doing so, and add it to + // `throttle_list` if created. Otherwise, leave `throttle_list` unchanged and + // do nothing. + static void MaybeCreateThrottle( + content::WebContents* web_contents, + const network::ResourceRequest& request, + std::vector>* throttle_list); + explicit UrlParamFilterThrottle( + const absl::optional& request_initiator_origin); + ~UrlParamFilterThrottle() override; + + UrlParamFilterThrottle(const UrlParamFilterThrottle&) = delete; + UrlParamFilterThrottle& operator=(const UrlParamFilterThrottle&) = delete; + + // blink::URLLoaderThrottle implementation. + void DetachFromCurrentSequence() override; + void WillStartRequest(network::ResourceRequest* request, + bool* defer) override; + void WillRedirectRequest( + net::RedirectInfo* redirect_info, + const network::mojom::URLResponseHead& response_head, + bool* defer, + std::vector* to_be_removed_request_headers, + net::HttpRequestHeaders* modified_request_headers, + net::HttpRequestHeaders* modified_cors_exempt_request_headers) override; + bool makes_unsafe_redirect() override; + + private: + GURL last_hop_initiator_; +}; +} // namespace url_param_filter +#endif // CHROME_BROWSER_URL_PARAM_FILTER_URL_PARAM_FILTER_THROTTLE_H_ diff --git a/chrome/browser/url_param_filter/url_param_filter_throttle_unittest.cc b/chrome/browser/url_param_filter/url_param_filter_throttle_unittest.cc new file mode 100644 index 00000000000000..bafec6b7949635 --- /dev/null +++ b/chrome/browser/url_param_filter/url_param_filter_throttle_unittest.cc @@ -0,0 +1,249 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/url_param_filter/url_param_filter_throttle.h" + +#include "base/test/scoped_feature_list.h" +#include "chrome/browser/ui/browser_navigator_params.h" +#include "chrome/browser/url_param_filter/cross_otr_observer.h" +#include "chrome/browser/url_param_filter/url_param_filter_test_helper.h" +#include "chrome/common/chrome_features.h" +#include "chrome/test/base/chrome_render_view_host_test_harness.h" +#include "net/http/http_request_headers.h" +#include "services/network/public/mojom/url_response_head.mojom.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" +#include "third_party/blink/public/common/loader/url_loader_throttle.h" +#include "url/origin.h" + +namespace url_param_filter { + +// Tests the UrlParamFilterThrottle, which is currently a very thin wrapper +// around the url_param_filter::FilterUrl() function. Coverage is accordingly +// somewhat less thorough than that seen in url_param_filterer_unittest. +class UrlParamFilterThrottleTest : public ChromeRenderViewHostTestHarness { + public: + UrlParamFilterThrottleTest() { + std::string encoded_classification = + CreateBase64EncodedFilterParamClassificationForTesting( + {{"source.xyz", {"plzblock"}}, + {"redirect.abc", {"plzblockredirect"}}, + {"redirect2.abc", {"plzblockredirect2"}}}, + {{"destination.xyz", {"plzblock1"}}}); + // With the flag set, the URL should be filtered. + scoped_feature_list_.InitAndEnableFeatureWithParameters( + features::kIncognitoParamFilterEnabled, + {{"classifications", encoded_classification}}); + } + + void CreateCrossOtrState() { + NavigateParams params(profile(), GURL("https://www.foo.com"), + ui::PAGE_TRANSITION_LINK); + + params.started_from_context_menu = true; + params.privacy_sensitivity = NavigateParams::PrivacySensitivity::CROSS_OTR; + content::WebContents* contents = web_contents(); + CrossOtrObserver::MaybeCreateForWebContents(contents, params); + } + + private: + base::test::ScopedFeatureList scoped_feature_list_; +}; + +TEST_F(UrlParamFilterThrottleTest, ShouldCreateThrottleNullContents) { + network::ResourceRequest resource_request; + resource_request.is_main_frame = true; + std::vector> result; + + UrlParamFilterThrottle::MaybeCreateThrottle(nullptr, resource_request, + &result); + + ASSERT_EQ(result.size(), 0u); +} + +TEST_F(UrlParamFilterThrottleTest, ShouldCreateThrottleNotCrossOtr) { + network::ResourceRequest resource_request; + resource_request.is_main_frame = true; + std::vector> result; + + UrlParamFilterThrottle::MaybeCreateThrottle(web_contents(), resource_request, + &result); + + ASSERT_EQ(result.size(), 0u); +} + +TEST_F(UrlParamFilterThrottleTest, ShouldCreateThrottleNotMainFrame) { + CreateCrossOtrState(); + network::ResourceRequest resource_request; + resource_request.is_main_frame = false; + std::vector> result; + + UrlParamFilterThrottle::MaybeCreateThrottle(web_contents(), resource_request, + &result); + + ASSERT_EQ(result.size(), 0u); +} + +TEST_F(UrlParamFilterThrottleTest, ShouldCreateThrottleTrueCase) { + CreateCrossOtrState(); + network::ResourceRequest resource_request; + resource_request.is_main_frame = true; + std::vector> result; + + UrlParamFilterThrottle::MaybeCreateThrottle(web_contents(), resource_request, + &result); + + ASSERT_EQ(result.size(), 1u); +} + +TEST_F(UrlParamFilterThrottleTest, WillStartRequestNullInitiatorNoChanges) { + std::unique_ptr resource_request = + std::make_unique(); + GURL expected_url = GURL("https://no-rule.xyz?asdf=1"); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(absl::nullopt); + resource_request->url = expected_url; + + bool defer = false; + + throttle.WillStartRequest(resource_request.get(), &defer); + + ASSERT_EQ(resource_request->url, expected_url); + ASSERT_FALSE(defer); +} + +TEST_F(UrlParamFilterThrottleTest, WillStartRequestInitiatorChanges) { + std::unique_ptr resource_request = + std::make_unique(); + GURL destination_url = GURL("https://no-rule.xyz?asdf=1&plzblock=1"); + GURL expected_url = GURL("https://no-rule.xyz?asdf=1"); + absl::optional initiator = + absl::make_optional(url::Origin::Create(GURL("https://source.xyz"))); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(initiator); + resource_request->url = destination_url; + + bool defer = false; + + throttle.WillStartRequest(resource_request.get(), &defer); + + ASSERT_EQ(resource_request->url, expected_url); + ASSERT_FALSE(defer); +} +TEST_F(UrlParamFilterThrottleTest, + WillStartRequestNoInitiatorDestinationChanges) { + std::unique_ptr resource_request = + std::make_unique(); + GURL destination_url = GURL("https://destination.xyz?asdf=1&plzblock1=1"); + GURL expected_url = GURL("https://destination.xyz?asdf=1"); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(absl::nullopt); + resource_request->url = destination_url; + + bool defer = false; + + throttle.WillStartRequest(resource_request.get(), &defer); + + ASSERT_EQ(resource_request->url, expected_url); + ASSERT_FALSE(defer); +} + +TEST_F(UrlParamFilterThrottleTest, WillRedirectRequestNullInitiatorNoChanges) { + std::unique_ptr redirect_info = + std::make_unique(); + GURL expected_url = GURL("https://no-rule.xyz?asdf=1"); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(absl::nullopt); + redirect_info->new_url = expected_url; + + bool defer = false; + net::HttpRequestHeaders headers; + std::vector removed_headers; + auto response_head = network::mojom::URLResponseHead::New(); + + throttle.WillRedirectRequest(redirect_info.get(), *response_head, &defer, + &removed_headers, &headers, &headers); + + ASSERT_EQ(redirect_info->new_url, expected_url); + ASSERT_FALSE(defer); +} + +TEST_F(UrlParamFilterThrottleTest, WillRedirectRequestInitiatorChanges) { + std::unique_ptr redirect_info = + std::make_unique(); + GURL destination_url = GURL("https://no-rule.xyz?asdf=1&plzblock=1"); + GURL expected_url = GURL("https://no-rule.xyz?asdf=1"); + absl::optional initiator = + absl::make_optional(url::Origin::Create(GURL("https://source.xyz"))); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(initiator); + redirect_info->new_url = destination_url; + + bool defer = false; + net::HttpRequestHeaders headers; + std::vector removed_headers; + auto response_head = network::mojom::URLResponseHead::New(); + + throttle.WillRedirectRequest(redirect_info.get(), *response_head, &defer, + &removed_headers, &headers, &headers); + + ASSERT_EQ(redirect_info->new_url, expected_url); + ASSERT_FALSE(defer); +} +TEST_F(UrlParamFilterThrottleTest, + WillRedirectRequestNoInitiatorDestinationChanges) { + std::unique_ptr redirect_info = + std::make_unique(); + GURL destination_url = GURL("https://destination.xyz?asdf=1&plzblock1=1"); + GURL expected_url = GURL("https://destination.xyz?asdf=1"); + UrlParamFilterThrottle throttle = UrlParamFilterThrottle(absl::nullopt); + redirect_info->new_url = destination_url; + + bool defer = false; + net::HttpRequestHeaders headers; + std::vector removed_headers; + auto response_head = network::mojom::URLResponseHead::New(); + + throttle.WillRedirectRequest(redirect_info.get(), *response_head, &defer, + &removed_headers, &headers, &headers); + + ASSERT_EQ(redirect_info->new_url, expected_url); + ASSERT_FALSE(defer); +} + +TEST_F(UrlParamFilterThrottleTest, MultipleRedirects) { + std::unique_ptr redirect_info = + std::make_unique(); + std::unique_ptr resource_request = + std::make_unique(); + + // The chain is: source.xyz-->redirect.abc-->redirect2.abc-->destination.xyz. + resource_request->url = GURL("https://redirect.abc?plzblock=1"); + GURL expected_first_intermediate_url = GURL("https://redirect.abc"); + GURL redirect_url = GURL("https://redirect2.abc?plzblockredirect=1"); + GURL expected_second_intermediate_url = GURL("https://redirect2.abc"); + GURL destination_url = + GURL("https://destination.xyz?asdf=1&plzblockredirect2=1"); + GURL expected_url = GURL("https://destination.xyz?asdf=1"); + UrlParamFilterThrottle throttle = + UrlParamFilterThrottle(url::Origin::Create(GURL("https://source.xyz"))); + redirect_info->new_url = redirect_url; + + bool defer = false; + net::HttpRequestHeaders headers; + std::vector removed_headers; + auto response_head = network::mojom::URLResponseHead::New(); + + throttle.WillStartRequest(resource_request.get(), &defer); + ASSERT_EQ(resource_request->url, expected_first_intermediate_url); + + throttle.WillRedirectRequest(redirect_info.get(), *response_head, &defer, + &removed_headers, &headers, &headers); + ASSERT_EQ(redirect_info->new_url, expected_second_intermediate_url); + // The new source should be redirect.abc; the new destination includes + // plzblockredirect, which should be filtered. + redirect_info->new_url = destination_url; + throttle.WillRedirectRequest(redirect_info.get(), *response_head, &defer, + &removed_headers, &headers, &headers); + + ASSERT_EQ(redirect_info->new_url, expected_url); + ASSERT_FALSE(defer); +} + +} // namespace url_param_filter diff --git a/chrome/browser/url_param_filter/url_param_filterer.cc b/chrome/browser/url_param_filter/url_param_filterer.cc index 508ae1271641c2..553d2ac73f4b1d 100644 --- a/chrome/browser/url_param_filter/url_param_filterer.cc +++ b/chrome/browser/url_param_filter/url_param_filterer.cc @@ -49,6 +49,11 @@ FilterResult FilterUrl(const GURL& source_url, GURL result = GURL{destination_url}; int filtered_params_count = 0; + // If there's no query string, we can short-circuit immediately. + if (!destination_url.has_query()) { + return FilterResult{destination_url, filtered_params_count}; + } + std::string source_etld_plus1 = GetEtldPlusOne(source_url); std::string destination_etld_plus1 = GetEtldPlusOne(destination_url); @@ -106,7 +111,11 @@ FilterResult FilterUrl(const GURL& source_url, std::string new_query = base::JoinString(query_parts, "&"); GURL::Replacements replacements; - replacements.SetQueryStr(new_query); + if (new_query == "") { + replacements.ClearQuery(); + } else { + replacements.SetQueryStr(new_query); + } result = result.ReplaceComponents(replacements); return FilterResult{result, filtered_params_count}; } diff --git a/chrome/browser/url_param_filter/url_param_filterer_unittest.cc b/chrome/browser/url_param_filter/url_param_filterer_unittest.cc index f99e50f970d666..5d29849679e8c6 100644 --- a/chrome/browser/url_param_filter/url_param_filterer_unittest.cc +++ b/chrome/browser/url_param_filter/url_param_filterer_unittest.cc @@ -338,6 +338,47 @@ TEST_F(UrlParamFiltererTest, FeatureDeactivated) { ASSERT_EQ(result, expected); } +TEST_F(UrlParamFiltererTest, FeatureActivatedNoQueryString) { + GURL source = GURL{"http://source.xyz"}; + GURL destination = GURL{"https://destination.xyz"}; + + std::string encoded_classification = + CreateBase64EncodedFilterParamClassificationForTesting( + {{"source.xyz", {"plzblock"}}}, {{"destination.xyz", {"plzblock1"}}}); + + base::test::ScopedFeatureList scoped_feature_list; + // With the flag set, the URL should be filtered. + scoped_feature_list.InitAndEnableFeatureWithParameters( + features::kIncognitoParamFilterEnabled, + {{"classifications", encoded_classification}}); + + GURL expected = GURL{"https://destination.xyz"}; + GURL result = FilterUrl(source, destination); + + ASSERT_EQ(result, expected); +} + +TEST_F(UrlParamFiltererTest, FeatureActivatedAllRemoved) { + GURL source = GURL{"http://source.xyz"}; + GURL destination = + GURL{"https://destination.xyz?plzblock=adf&plzblock1=asdffdsa"}; + + std::string encoded_classification = + CreateBase64EncodedFilterParamClassificationForTesting( + {{"source.xyz", {"plzblock"}}}, {{"destination.xyz", {"plzblock1"}}}); + + base::test::ScopedFeatureList scoped_feature_list; + // With the flag set, the URL should be filtered. + scoped_feature_list.InitAndEnableFeatureWithParameters( + features::kIncognitoParamFilterEnabled, + {{"classifications", encoded_classification}}); + + GURL expected = GURL{"https://destination.xyz"}; + GURL result = FilterUrl(source, destination); + + ASSERT_EQ(result, expected); +} + TEST_F(UrlParamFiltererTest, FeatureActivatedSourceAndDestinationRemoval) { GURL source = GURL{"http://source.xyz"}; GURL destination = diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 4d86fa8ef7030a..d291726fb6a3b5 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -5195,6 +5195,7 @@ test("unit_tests") { "../browser/update_client/chrome_update_query_params_delegate_unittest.cc", "../browser/url_param_filter/cross_otr_observer_unittest.cc", "../browser/url_param_filter/url_param_classifications_loader_unittest.cc", + "../browser/url_param_filter/url_param_filter_throttle_unittest.cc", "../browser/url_param_filter/url_param_filterer_unittest.cc", "../browser/visibility_timer_tab_helper_unittest.cc", "../browser/vr/vr_tab_helper_unittest.cc",