Skip to content

Commit

Permalink
Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fal…
Browse files Browse the repository at this point in the history
…lback (#18141) (#18179)

* Migrate to HttpsFirstMode v2 (#17856)

Migrate HTTPS by Default feature to use HttpsFirstMode v2

* Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141)
  • Loading branch information
arthuredelstein authored Apr 28, 2023
1 parent e369d71 commit 0fbe592
Show file tree
Hide file tree
Showing 12 changed files with 251 additions and 217 deletions.
2 changes: 1 addition & 1 deletion app/brave_main_delegate_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) {
&features::kCopyLinkToText,
#endif
&features::kDigitalGoodsApi,
&features::kHttpsFirstModeV2,
&features::kFedCm,
&features::kFedCmIframeSupport,
&features::kFedCmUserInfo,
Expand Down Expand Up @@ -227,6 +226,7 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, EnabledFeatures) {
&blink::features::kPrefetchPrivacyChanges,
&blink::features::kReducedReferrerGranularity,
&blink::features::kReduceUserAgentMinorVersion,
&features::kHttpsFirstModeV2,
#if BUILDFLAG(IS_WIN)
&features::kWinrtGeolocationImplementation,
#endif
Expand Down
56 changes: 37 additions & 19 deletions browser/brave_shields/https_upgrade_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/interstitials/security_interstitial_page_test_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ssl/https_only_mode_upgrade_interceptor.h"
#include "chrome/browser/ssl/https_upgrades_interceptor.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/chrome_test_utils.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -42,22 +43,36 @@ enum class PageResult { kHttp, kHttps, kInterstitial };
struct TestCase {
bool init_secure;
const char* domain;
const char* path;
ControlType control_type;
PageResult expected_result;
};

constexpr char kSimple[] = "/simple.html";
// Nonexistent page results in a 404:
constexpr char kNonexistent[] = "/nonexistent.html";

constexpr TestCase kTestCases[] = {
{false, "insecure1.test", ControlType::ALLOW, PageResult::kHttp},
{false, "insecure2.test", ControlType::BLOCK_THIRD_PARTY,
{false, "insecure1.test", kSimple, ControlType::ALLOW, PageResult::kHttp},
{false, "insecure2.test", kSimple, ControlType::BLOCK_THIRD_PARTY,
PageResult::kHttp},
{false, "insecure3.test", kSimple, ControlType::BLOCK,
PageResult::kInterstitial},
{false, "broken1.test", kNonexistent, ControlType::ALLOW,
PageResult::kHttp},
{false, "broken2.test", kNonexistent, ControlType::BLOCK_THIRD_PARTY,
PageResult::kHttp},
{false, "insecure3.test", ControlType::BLOCK, PageResult::kInterstitial},
{false, "upgradable1.test", ControlType::ALLOW, PageResult::kHttp},
{false, "upgradable2.test", ControlType::BLOCK_THIRD_PARTY,
{false, "broken3.test", kNonexistent, ControlType::BLOCK,
PageResult::kInterstitial},
{false, "upgradable1.test", kSimple, ControlType::ALLOW, PageResult::kHttp},
{false, "upgradable2.test", kSimple, ControlType::BLOCK_THIRD_PARTY,
PageResult::kHttps},
{false, "upgradable3.test", ControlType::BLOCK, PageResult::kHttps},
{true, "secure1.test", ControlType::ALLOW, PageResult::kHttps},
{true, "secure2.test", ControlType::BLOCK_THIRD_PARTY, PageResult::kHttps},
{true, "secure3.test", ControlType::BLOCK, PageResult::kHttps}};
{false, "upgradable3.test", kSimple, ControlType::BLOCK,
PageResult::kHttps},
{true, "secure1.test", kSimple, ControlType::ALLOW, PageResult::kHttps},
{true, "secure2.test", kSimple, ControlType::BLOCK_THIRD_PARTY,
PageResult::kHttps},
{true, "secure3.test", kSimple, ControlType::BLOCK, PageResult::kHttps}};

base::FilePath GetTestDataDir() {
return base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest"));
Expand All @@ -71,7 +86,8 @@ class HttpsUpgradeBrowserTest : public PlatformBrowserTest {
~HttpsUpgradeBrowserTest() override = default;

void SetUp() override {
feature_list_.InitAndEnableFeature(kBraveHttpsByDefault);
feature_list_.InitWithFeatures(
{features::kHttpsFirstModeV2, kBraveHttpsByDefault}, {});
PlatformBrowserTest::SetUp();
}

Expand Down Expand Up @@ -102,10 +118,8 @@ class HttpsUpgradeBrowserTest : public PlatformBrowserTest {
ASSERT_TRUE(http_server_.Start());
ASSERT_TRUE(https_server_.Start());

HttpsOnlyModeUpgradeInterceptor::SetHttpsPortForTesting(
https_server()->port());
HttpsOnlyModeUpgradeInterceptor::SetHttpPortForTesting(
http_server()->port());
HttpsUpgradesInterceptor::SetHttpsPortForTesting(https_server()->port());
HttpsUpgradesInterceptor::SetHttpPortForTesting(http_server()->port());
}

void SetUpCommandLine(base::CommandLine* command_line) override {
Expand Down Expand Up @@ -138,15 +152,19 @@ class HttpsUpgradeBrowserTest : public PlatformBrowserTest {
<< "test_case.control_type: " << test_case.control_type);
GURL initial_url =
test_case.init_secure
? https_server()->GetURL(test_case.domain, "/simple.html")
: http_server()->GetURL(test_case.domain, "/simple.html");
? https_server()->GetURL(test_case.domain, test_case.path)
: http_server()->GetURL(test_case.domain, test_case.path);
brave_shields::SetBraveShieldsEnabled(ContentSettings(), shields_enabled,
initial_url, nullptr);
brave_shields::SetHttpsUpgradeControlType(
ContentSettings(), test_case.control_type,
global_setting ? GURL() : initial_url,
g_browser_process->local_state());
AttemptToNavigateToURL(initial_url);
// Run navigation twice to ensure that the behavior doesn't
// change after first run.
for (int i = 0; i < 2; ++i) {
AttemptToNavigateToURL(initial_url);
}
return initial_url;
}

Expand Down Expand Up @@ -195,7 +213,7 @@ IN_PROC_BROWSER_TEST_F(HttpsUpgradeBrowserTest, CheckUpgrades) {
GURL final_url =
(test_case.expected_result == PageResult::kHttp ? http_server()
: https_server())
->GetURL(test_case.domain, "/simple.html");
->GetURL(test_case.domain, test_case.path);
EXPECT_EQ(final_url, Contents()->GetLastCommittedURL());
}
}
Expand Down
142 changes: 0 additions & 142 deletions chromium_src/chrome/browser/ssl/https_only_mode_navigation_throttle.cc

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,37 +1,12 @@
/* 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/. */
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "chrome/browser/ssl/https_only_mode_upgrade_interceptor.h"

#include "brave/browser/brave_browser_process.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "components/prefs/pref_service.h"
#include "net/base/url_util.h"

class GURL;

namespace content {
class BrowserContext;
} // namespace content

namespace {

bool ShouldUpgradeToHttps(content::BrowserContext* context, const GURL& url) {
if (!brave_shields::IsHttpsByDefaultFeatureEnabled()) {
return false;
}
HostContentSettingsMap* map =
HostContentSettingsMapFactory::GetForProfile(context);
return brave_shields::ShouldUpgradeToHttps(
map, url, g_brave_browser_process->https_upgrade_exceptions_service());
}

} // namespace

namespace net {
namespace {

Expand All @@ -47,12 +22,7 @@ bool IsLocalhostOrOnion(const GURL& url) {
} // namespace net

#define IsLocalhost(URL) IsLocalhostOrOnion(URL)
#define GetBoolean(PREF_NAME) \
GetBooleanOr( \
PREF_NAME, \
ShouldUpgradeToHttps(browser_context, tentative_resource_request.url))

#include "src/chrome/browser/ssl/https_only_mode_upgrade_interceptor.cc"

#undef IsLocalHost
#undef GetBoolean
Loading

0 comments on commit 0fbe592

Please sign in to comment.