Skip to content

Commit

Permalink
Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error co…
Browse files Browse the repository at this point in the history
…de (#18141)
  • Loading branch information
arthuredelstein committed Apr 27, 2023
1 parent da8f56c commit 8851863
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 12 deletions.
38 changes: 26 additions & 12 deletions browser/brave_shields/https_upgrade_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,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", kSimple, ControlType::BLOCK,
PageResult::kHttps},
{true, "secure1.test", kSimple, ControlType::ALLOW, PageResult::kHttps},
{true, "secure2.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}};
{true, "secure3.test", kSimple, ControlType::BLOCK, PageResult::kHttps}};

base::FilePath GetTestDataDir() {
return base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest"));
Expand Down Expand Up @@ -138,8 +152,8 @@ 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(
Expand Down Expand Up @@ -199,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
20 changes: 20 additions & 0 deletions chromium_src/chrome/browser/ssl/https_upgrades_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,25 @@
} \
void HttpsUpgradesInterceptor::MaybeCreateLoader_ChromiumImpl(__VA_ARGS__)

// Force pages that have upgraded to HTTPS to fall back to HTTP if we receive
// an HTTP response code ()>= 400) on upgrade.
#define MaybeCreateLoaderForResponse(...) \
MaybeCreateLoaderForResponse(__VA_ARGS__) { \
network::URLLoaderCompletionStatus modified_status(status); \
if (!(*response_head).is_null()) { \
auto headers = (*response_head)->headers; \
if (headers && headers->response_code() >= 400) { \
modified_status.error_code = net::ERR_HTTP_RESPONSE_CODE_FAILURE; \
} \
} \
return MaybeCreateLoaderForResponse_ChromiumImpl( \
modified_status, request, response_head, response_body, loader, \
client_receiver, url_loader, skip_other_interceptors, \
will_return_unsafe_redirect); \
} \
bool HttpsUpgradesInterceptor::MaybeCreateLoaderForResponse_ChromiumImpl( \
__VA_ARGS__)

#define IsEnabled(FLAG) \
IsEnabled(FLAG.name == features::kHttpsUpgrades.name \
? net::features::kBraveHttpsByDefault \
Expand All @@ -37,4 +56,5 @@
#include "src/chrome/browser/ssl/https_upgrades_interceptor.cc"

#undef MaybeCreateLoader
#undef MaybeCreateLoaderForResponse
#undef IsEnabled
5 changes: 5 additions & 0 deletions chromium_src/chrome/browser/ssl/https_upgrades_interceptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@
MaybeCreateLoader_ChromiumImpl(__VA_ARGS__); \
void MaybeCreateLoader(__VA_ARGS__)

#define MaybeCreateLoaderForResponse(...) \
MaybeCreateLoaderForResponse_ChromiumImpl(__VA_ARGS__); \
bool MaybeCreateLoaderForResponse(__VA_ARGS__)

#include "src/chrome/browser/ssl/https_upgrades_interceptor.h" // IWYU pragma: export

#undef MaybeCreateLoader
#undef MaybeCreateLoaderForResponse

#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_SSL_HTTPS_UPGRADES_INTERCEPTOR_H_

0 comments on commit 8851863

Please sign in to comment.