Upgrade URLs typed with http: (#19822)

browser/brave_shields/https_upgrade_browsertest.cc

@@ -30,7 +30,9 @@
 #if BUILDFLAG(IS_ANDROID)
 #include "chrome/test/base/android/android_browser_test.h"
 #else
+#include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
 #endif
 
 using brave_shields::ControlType;
@@ -45,6 +47,7 @@ struct TestCase {
   const char* domain;
   const char* path;
   ControlType control_type;
+  bool type_url;
   PageResult expected_result;
 };
 
@@ -53,24 +56,36 @@ constexpr char kSimple[] = "/simple.html";
 constexpr char kNonexistent[] = "/nonexistent.html";
 
 constexpr TestCase kTestCases[] = {
-    {false, "insecure1.com", kSimple, ControlType::ALLOW, PageResult::kHttp},
-    {false, "insecure2.com", kSimple, ControlType::BLOCK_THIRD_PARTY,
+    {false, "insecure1.com", kSimple, ControlType::ALLOW, false,
      PageResult::kHttp},
-    {false, "insecure3.com", kSimple, ControlType::BLOCK,
-     PageResult::kInterstitial},
-    {false, "broken1.com", kNonexistent, ControlType::ALLOW, PageResult::kHttp},
-    {false, "broken2.com", kNonexistent, ControlType::BLOCK_THIRD_PARTY,
+    {false, "insecure2.com", kSimple, ControlType::BLOCK_THIRD_PARTY, false,
      PageResult::kHttp},
-    {false, "broken3.com", kNonexistent, ControlType::BLOCK,
+    {false, "insecure3.com", kSimple, ControlType::BLOCK, false,
      PageResult::kInterstitial},
-    {false, "upgradable1.com", kSimple, ControlType::ALLOW, PageResult::kHttp},
-    {false, "upgradable2.com", kSimple, ControlType::BLOCK_THIRD_PARTY,
+    {false, "broken1.com", kNonexistent, ControlType::ALLOW, false,
+     PageResult::kHttp},
+    {false, "broken2.com", kNonexistent, ControlType::BLOCK_THIRD_PARTY, false,
+     PageResult::kHttp},
+    {false, "broken3.com", kNonexistent, ControlType::BLOCK, false,
+     PageResult::kInterstitial},
+    {false, "upgradable1.com", kSimple, ControlType::ALLOW, false,
+     PageResult::kHttp},
+    {false, "upgradable2.com", kSimple, ControlType::BLOCK_THIRD_PARTY, false,
      PageResult::kHttps},
-    {false, "upgradable3.com", kSimple, ControlType::BLOCK, PageResult::kHttps},
-    {true, "secure1.com", kSimple, ControlType::ALLOW, PageResult::kHttps},
-    {true, "secure2.com", kSimple, ControlType::BLOCK_THIRD_PARTY,
+    {false, "upgradable3.com", kSimple, ControlType::BLOCK, false,
      PageResult::kHttps},
-    {true, "secure3.com", kSimple, ControlType::BLOCK, PageResult::kHttps}};
+    {false, "upgradable1.com", kSimple, ControlType::ALLOW, true,
+     PageResult::kHttp},
+    {false, "upgradable2.com", kSimple, ControlType::BLOCK_THIRD_PARTY, true,
+     PageResult::kHttps},
+    {false, "upgradable3.com", kSimple, ControlType::BLOCK, true,
+     PageResult::kHttps},
+    {true, "secure1.com", kSimple, ControlType::ALLOW, false,
+     PageResult::kHttps},
+    {true, "secure2.com", kSimple, ControlType::BLOCK_THIRD_PARTY, false,
+     PageResult::kHttps},
+    {true, "secure3.com", kSimple, ControlType::BLOCK, false,
+     PageResult::kHttps}};
 
 base::FilePath GetTestDataDir() {
   return base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest"));
@@ -135,14 +150,24 @@ class HttpsUpgradeBrowserTest : public PlatformBrowserTest {
     mock_cert_verifier_.TearDownInProcessBrowserTestFixture();
   }
 
-  void AttemptToNavigateToURL(const GURL& url) {
+  void AttemptNavigation(const GURL& url, bool url_typed_with_http_scheme) {
+#if BUILDFLAG(IS_ANDROID)
+    // Chromium Android does not appear to skip upgrading URLs typed
+    // with an http scheme, so we don't need a special setting:
     content::NavigateToURLBlockUntilNavigationsComplete(Contents(), url, 1,
                                                         true);
+#else
+    NavigateParams params(chrome_test_utils::GetProfile(this), url,
+                          ui::PAGE_TRANSITION_TYPED);
+    params.url_typed_with_http_scheme = url_typed_with_http_scheme;
+    ui_test_utils::NavigateToURL(&params);
+#endif
   }
 
   GURL RunTestCaseNavigation(bool shields_enabled,
                              bool global_setting,
-                             const TestCase& test_case) {
+                             const TestCase& test_case,
+                             bool type_url) {
     SCOPED_TRACE(testing::Message()
                  << "global_setting: " << global_setting << ", "
                  << "test_case.init_secure: " << test_case.init_secure << ", "
@@ -161,7 +186,7 @@ class HttpsUpgradeBrowserTest : public PlatformBrowserTest {
     // Run navigation twice to ensure that the behavior doesn't
     // change after first run.
     for (int i = 0; i < 2; ++i) {
-      AttemptToNavigateToURL(initial_url);
+      AttemptNavigation(initial_url, type_url);
     }
     return initial_url;
   }
@@ -201,7 +226,8 @@ class HttpsUpgradeBrowserTest_FlagDisabled : public HttpsUpgradeBrowserTest {
 IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, CheckUpgrades) {
   for (bool global_setting : {true, false}) {
     for (const TestCase& test_case : kTestCases) {
-      RunTestCaseNavigation(true, global_setting, test_case);
+      RunTestCaseNavigation(true, global_setting, test_case,
+                            test_case.type_url);
       bool interstitial_showing =
           chrome_browser_interstitials::IsShowingInterstitial(Contents());
       if (test_case.expected_result == PageResult::kInterstitial) {
@@ -221,8 +247,8 @@ IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, CheckUpgrades) {
 IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, CheckUpgradesWithShieldsDown) {
   for (bool global_setting : {true, false}) {
     for (const TestCase& test_case : kTestCases) {
-      const GURL initial_url =
-          RunTestCaseNavigation(false, global_setting, test_case);
+      const GURL initial_url = RunTestCaseNavigation(
+          false, global_setting, test_case, test_case.type_url);
       // Shields down means no URLs change and no interstitials shown.
       EXPECT_EQ(initial_url, Contents()->GetLastCommittedURL());
       bool interstitial_showing =
@@ -235,8 +261,8 @@ IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, CheckUpgradesWithShieldsDown) {
 IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest_FlagDisabled, CheckUpgrades) {
   for (bool global_setting : {true, false}) {
     for (const TestCase& test_case : kTestCases) {
-      const GURL initial_url =
-          RunTestCaseNavigation(true, global_setting, test_case);
+      const GURL initial_url = RunTestCaseNavigation(
+          true, global_setting, test_case, test_case.type_url);
       // Disabled flag means no URLs change and no interstitials shown.
       EXPECT_EQ(initial_url, Contents()->GetLastCommittedURL());
       bool interstitial_showing =

chromium_src/chrome/browser/renderer_host/chrome_navigation_ui_data.h

@@ -0,0 +1,19 @@
+/* Copyright (c) 2023 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 https://mozilla.org/MPL/2.0/. */
+
+#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_RENDERER_HOST_CHROME_NAVIGATION_UI_DATA_H_
+#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_RENDERER_HOST_CHROME_NAVIGATION_UI_DATA_H_
+
+#define url_is_typed_with_http_scheme() \
+  return_false() const {                \
+    return false;                       \
+  }                                     \
+  bool url_is_typed_with_http_scheme()
+
+#include "src/chrome/browser/renderer_host/chrome_navigation_ui_data.h"  // IWYU pragma: export
+
+#undef url_is_typed_with_http_scheme
+
+#endif  // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_RENDERER_HOST_CHROME_NAVIGATION_UI_DATA_H_

chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc

@@ -11,6 +11,9 @@
 #include "net/base/features.h"
 #include "net/base/url_util.h"
 
+// Prevent double-defining macro
+#include "chrome/browser/renderer_host/chrome_navigation_ui_data.h"
+
 #define MaybeCreateLoader(...)                                                \
   MaybeCreateLoader(__VA_ARGS__) {                                            \
     if (brave_shields::IsHttpsByDefaultFeatureEnabled()) {                    \
@@ -57,9 +60,12 @@
 
 #define IsLocalhost(URL) IsLocalhostOrOnion(URL)
 
+#define url_is_typed_with_http_scheme() return_false()
+
 #include "src/chrome/browser/ssl/https_upgrades_interceptor.cc"
 
 #undef MaybeCreateLoader
 #undef MaybeCreateLoaderForResponse
 #undef IsEnabled
 #undef IsLocalhost
+#undef url_is_typed_with_http_scheme

test/filters/browser_tests.filter

@@ -662,9 +662,11 @@
 -HttpsUpgradesBrowserTest.SlowHttps_ShouldInterstitial/*
 -HttpsUpgradesBrowserTest.URLsAutocompletedWithHttpSchemeAreUpgraded/*
 -HttpsUpgradesBrowserTest.URLsTypedWithHttpSchemeNoUpgrades/*
--HttpsUpgradesBrowserTest.UrlWithHttpScheme_ShouldUpgrade/*
+-HttpsUpgradesBrowserTest.URLsTypedWithHttpSchemeNoUpgradesAllowlist/HttpsFirstModeWithSiteEngagementAndHttpsUpgrades
+-HttpsUpgradesBrowserTest.URLsTypedWithHttpSchemeNoUpgradesAllowlist/HttpsUpgradesOnly
 -HttpsUpgradesBrowserTest.UrlWithHttpScheme_BrokenSSL_ShouldInterstitial/*
 -HttpsUpgradesBrowserTest.UrlWithHttpScheme_BrokenSSL_ShouldInterstitial_SiteEngagement/*
+-HttpsUpgradesBrowserTest.UrlWithHttpScheme_ShouldUpgrade/*
 -HttpsUpgradesBrowserTest.UrlWithHttpsScheme_ShouldLoad/*
 
 # These tests fail because we change the Learn More link in the HTTPS First