Skip to content

Commit

Permalink
Opt-out URLs typed with http scheme from https upgrades.
Browse files Browse the repository at this point in the history
This only opts out cases where the full http url was typed (e.g. typing http://examp and then choosing example.com from the suggestions will still result in an upgrade, as will navigating to an http url via an http bookmark).

Plumbing for the new flag is somewhat duplicated with added_default_scheme_to_typed_url. This was on purpose (rather than adding a multi-state flag) since schemeless typed navigations upgrades (and thus the added_default_scheme_to_typed_url) will be obsolete when https upgrades fully launch.

Change-Id: I86c4f487b79aa3b4705f2345ddc25ca22a8b4eb4

(cherry picked from commit 67cf5a9)

Bug: 1447921
Change-Id: I86c4f487b79aa3b4705f2345ddc25ca22a8b4eb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4549863
Reviewed-by: Robbie Gibson <rkgibson@google.com>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Chris Thompson <cthomp@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149342}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4575041
Auto-Submit: Carlos IL <carlosil@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#138}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
carlosjoan91 authored and Chromium LUCI CQ committed May 30, 2023
1 parent 24fee4f commit 403eaca
Show file tree
Hide file tree
Showing 30 changed files with 209 additions and 17 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/chrome_content_browser_client.cc
Expand Up @@ -6006,7 +6006,8 @@ ChromeContentBrowserClient::WillCreateURLLoaderRequestInterceptors(

if (base::FeatureList::IsEnabled(features::kHttpsFirstModeV2)) {
auto https_upgrades_interceptor =
HttpsUpgradesInterceptor::MaybeCreateInterceptor(frame_tree_node_id);
HttpsUpgradesInterceptor::MaybeCreateInterceptor(frame_tree_node_id,
navigation_ui_data);
if (https_upgrades_interceptor) {
interceptors.push_back(std::move(https_upgrades_interceptor));
}
Expand Down
6 changes: 5 additions & 1 deletion chrome/browser/renderer_host/chrome_navigation_ui_data.cc
Expand Up @@ -53,11 +53,14 @@ std::unique_ptr<ChromeNavigationUIData>
ChromeNavigationUIData::CreateForMainFrameNavigation(
content::WebContents* web_contents,
WindowOpenDisposition disposition,
bool is_using_https_as_default_scheme) {
bool is_using_https_as_default_scheme,
bool url_is_typed_with_http_scheme) {
auto navigation_ui_data = std::make_unique<ChromeNavigationUIData>();
navigation_ui_data->disposition_ = disposition;
navigation_ui_data->is_using_https_as_default_scheme_ =
is_using_https_as_default_scheme;
navigation_ui_data->url_is_typed_with_http_scheme_ =
url_is_typed_with_http_scheme;

#if BUILDFLAG(ENABLE_EXTENSIONS)
int tab_id = extension_misc::kUnknownTabId;
Expand All @@ -81,6 +84,7 @@ std::unique_ptr<content::NavigationUIData> ChromeNavigationUIData::Clone() {

copy->disposition_ = disposition_;
copy->is_using_https_as_default_scheme_ = is_using_https_as_default_scheme_;
copy->url_is_typed_with_http_scheme_ = url_is_typed_with_http_scheme_;

#if BUILDFLAG(ENABLE_EXTENSIONS)
if (extension_data_)
Expand Down
11 changes: 10 additions & 1 deletion chrome/browser/renderer_host/chrome_navigation_ui_data.h
Expand Up @@ -49,7 +49,8 @@ class ChromeNavigationUIData : public content::NavigationUIData {
static std::unique_ptr<ChromeNavigationUIData> CreateForMainFrameNavigation(
content::WebContents* web_contents,
WindowOpenDisposition disposition,
bool is_using_https_as_default_scheme);
bool is_using_https_as_default_scheme,
bool url_is_typed_with_http_scheme);

// Creates a new ChromeNavigationUIData that is a deep copy of the original.
// Any changes to the original after the clone is created will not be
Expand Down Expand Up @@ -83,6 +84,9 @@ class ChromeNavigationUIData : public content::NavigationUIData {
bool is_using_https_as_default_scheme() const {
return is_using_https_as_default_scheme_;
}
bool url_is_typed_with_http_scheme() const {
return url_is_typed_with_http_scheme_;
}

absl::optional<base::Uuid> bookmark_id() { return bookmark_id_; }
void set_bookmark_id(absl::optional<base::Uuid> id) { bookmark_id_ = id; }
Expand All @@ -109,6 +113,11 @@ class ChromeNavigationUIData : public content::NavigationUIData {
// observed and fall back to using http scheme if necessary.
bool is_using_https_as_default_scheme_ = false;

// True if the navigation was initiated by typing in the omnibox, and the
// typed text had an explicit http scheme. This is used to opt-out of https
// upgrades.
bool url_is_typed_with_http_scheme_ = false;

// Id of the bookmark which started this navigation.
absl::optional<base::Uuid> bookmark_id_ = absl::nullopt;
};
Expand Down
68 changes: 68 additions & 0 deletions chrome/browser/ssl/https_upgrades_browsertest.cc
Expand Up @@ -20,10 +20,15 @@
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
#include "components/omnibox/browser/omnibox_edit_model_delegate.h"
#include "components/omnibox/browser/omnibox_view.h"
#include "components/prefs/pref_service.h"
#include "components/security_interstitials/content/stateful_ssl_host_state_delegate.h"
#include "components/security_interstitials/core/https_only_mode_metrics.h"
Expand Down Expand Up @@ -1990,6 +1995,69 @@ IN_PROC_BROWSER_TEST_P(HttpsUpgradesBrowserTest,
contents));
}

// Tests that URLs typed with an explicit http:// scheme are opted out from
// upgrades.
IN_PROC_BROWSER_TEST_P(HttpsUpgradesBrowserTest,
URLsTypedWithHttpSchemeNoUpgrades) {
if (!IsHttpUpgradingEnabled()) {
return;
}
GURL http_url = http_server()->GetURL("foo.com", "/simple.html");
GURL https_url = https_server()->GetURL("foo.com", "/simple.html");
auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
OmniboxEditModelDelegate* edit_model_delegate = browser()
->window()
->GetLocationBar()
->GetOmniboxView()
->model()
->delegate();

// Simulate the full URL was typed with an http scheme.
content::TestNavigationObserver nav_observer(contents, 1);
edit_model_delegate->OnAutocompleteAccept(
http_url, nullptr, WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, AutocompleteMatchType::URL_WHAT_YOU_TYPED,
base::TimeTicks(), false, true, std::u16string(), AutocompleteMatch(),
AutocompleteMatch(), IDNA2008DeviationCharacter::kNone);
nav_observer.Wait();

if (IsHttpsFirstModePrefEnabled()) {
// Typed http URLs don't opt out of upgrades in HFM.
EXPECT_EQ(https_url, contents->GetLastCommittedURL());
} else {
EXPECT_EQ(http_url, contents->GetLastCommittedURL());
}
}

// Tests that URLs with an explicit http:// scheme are upgraded if they were
// autocompleted.
IN_PROC_BROWSER_TEST_P(HttpsUpgradesBrowserTest,
URLsAutocompletedWithHttpSchemeAreUpgraded) {
if (!IsHttpUpgradingEnabled()) {
return;
}
GURL http_url = http_server()->GetURL("foo.com", "/simple.html");
GURL https_url = https_server()->GetURL("foo.com", "/simple.html");
auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
OmniboxEditModelDelegate* edit_model_delegate = browser()
->window()
->GetLocationBar()
->GetOmniboxView()
->model()
->delegate();

// Simulate the full URL was autocompleted with an http scheme.
content::TestNavigationObserver nav_observer(contents, 1);
edit_model_delegate->OnAutocompleteAccept(
http_url, nullptr, WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, AutocompleteMatchType::NAVSUGGEST,
base::TimeTicks(), false, false, std::u16string(), AutocompleteMatch(),
AutocompleteMatch(), IDNA2008DeviationCharacter::kNone);
nav_observer.Wait();

EXPECT_EQ(https_url, contents->GetLastCommittedURL());
}

// A simple test fixture that ensures the kHttpsFirstModeV2 feature is enabled
// and constructs a HistogramTester (so that it gets initialized before browser
// startup). Used for testing pref tracking logic.
Expand Down
26 changes: 21 additions & 5 deletions chrome/browser/ssl/https_upgrades_interceptor.cc
Expand Up @@ -11,6 +11,7 @@
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/renderer_host/chrome_navigation_ui_data.h"
#include "chrome/browser/ssl/https_only_mode_tab_helper.h"
#include "chrome/browser/ssl/https_upgrades_util.h"
#include "chrome/common/chrome_features.h"
Expand Down Expand Up @@ -157,7 +158,9 @@ using security_interstitials::https_only_mode::NavigationRequestSecurityLevel;

// static
std::unique_ptr<HttpsUpgradesInterceptor>
HttpsUpgradesInterceptor::MaybeCreateInterceptor(int frame_tree_node_id) {
HttpsUpgradesInterceptor::MaybeCreateInterceptor(
int frame_tree_node_id,
content::NavigationUIData* navigation_ui_data) {
auto* web_contents =
content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
// Could be null if the FrameTreeNode's RenderFrameHost is shutting down.
Expand All @@ -175,15 +178,17 @@ HttpsUpgradesInterceptor::MaybeCreateInterceptor(int frame_tree_node_id) {
bool https_first_mode_enabled =
base::FeatureList::IsEnabled(features::kHttpsFirstModeV2) && prefs &&
prefs->GetBoolean(prefs::kHttpsOnlyModeEnabled);
return std::make_unique<HttpsUpgradesInterceptor>(frame_tree_node_id,
https_first_mode_enabled);
return std::make_unique<HttpsUpgradesInterceptor>(
frame_tree_node_id, https_first_mode_enabled, navigation_ui_data);
}

HttpsUpgradesInterceptor::HttpsUpgradesInterceptor(
int frame_tree_node_id,
bool http_interstitial_enabled_by_pref)
bool http_interstitial_enabled_by_pref,
content::NavigationUIData* navigation_ui_data)
: frame_tree_node_id_(frame_tree_node_id),
http_interstitial_enabled_by_pref_(http_interstitial_enabled_by_pref) {}
http_interstitial_enabled_by_pref_(http_interstitial_enabled_by_pref),
navigation_ui_data_(navigation_ui_data) {}

HttpsUpgradesInterceptor::~HttpsUpgradesInterceptor() = default;

Expand Down Expand Up @@ -286,6 +291,17 @@ void HttpsUpgradesInterceptor::MaybeCreateLoader(
}
}

// If the URL was typed with an explicit http:// URL, it is opted-out from
// upgrades.
ChromeNavigationUIData* chrome_navigation_ui_data =
static_cast<ChromeNavigationUIData*>(navigation_ui_data_);
if (chrome_navigation_ui_data &&
chrome_navigation_ui_data->url_is_typed_with_http_scheme() &&
!IsInterstitialEnabled(*interstitial_state_)) {
std::move(callback).Run({});
return;
}

// Check whether this host would be upgraded to HTTPS by HSTS. This requires a
// Mojo call to the network service, so set up a callback to continue the rest
// of the MaybeCreateLoader() logic (passing along the necessary state). The
Expand Down
9 changes: 7 additions & 2 deletions chrome/browser/ssl/https_upgrades_interceptor.h
Expand Up @@ -30,6 +30,7 @@ class ThrottlingURLLoader;

namespace content {
class BrowserContext;
class NavigationUIData;
class WebContents;
} // namespace content

Expand All @@ -47,10 +48,12 @@ class HttpsUpgradesInterceptor : public content::URLLoaderRequestInterceptor,
public network::mojom::URLLoader {
public:
static std::unique_ptr<HttpsUpgradesInterceptor> MaybeCreateInterceptor(
int frame_tree_node_id);
int frame_tree_node_id,
content::NavigationUIData* navigation_ui_data_);

HttpsUpgradesInterceptor(int frame_tree_node_id,
bool http_interstitial_enabled);
bool http_interstitial_enabled,
content::NavigationUIData* navigation_ui_data_);
~HttpsUpgradesInterceptor() override;

HttpsUpgradesInterceptor(const HttpsUpgradesInterceptor&) = delete;
Expand Down Expand Up @@ -142,6 +145,8 @@ class HttpsUpgradesInterceptor : public content::URLLoaderRequestInterceptor,
// The owning client. Used for serving redirects.
mojo::Remote<network::mojom::URLLoaderClient> client_;

// Owned by NavigationURLLoaderImpl, which should outlive the interceptor.
raw_ptr<content::NavigationUIData> navigation_ui_data_;
SEQUENCE_CHECKER(sequence_checker_);

base::WeakPtrFactory<HttpsUpgradesInterceptor> weak_factory_{this};
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/browser_commands.cc
Expand Up @@ -774,6 +774,8 @@ base::WeakPtr<content::NavigationHandle> OpenCurrentURL(Browser* browser) {
params.input_start = location_bar->GetMatchSelectionTimestamp();
params.is_using_https_as_default_scheme =
location_bar->IsInputTypedUrlWithoutScheme();
params.url_typed_with_http_scheme =
location_bar->IsInputTypedUrlWithHttpScheme();
auto result = Navigate(&params);

#if BUILDFLAG(ENABLE_EXTENSIONS)
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/browser_focus_uitest.cc
Expand Up @@ -637,7 +637,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, NavigateFromOmniboxIntoNewTab) {
edit_model_delegate->OnAutocompleteAccept(
url2, nullptr, WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui::PAGE_TRANSITION_TYPED, AutocompleteMatchType::URL_WHAT_YOU_TYPED,
base::TimeTicks(), false, std::u16string(), AutocompleteMatch(),
base::TimeTicks(), false, false, std::u16string(), AutocompleteMatch(),
AutocompleteMatch(), IDNA2008DeviationCharacter::kNone);

// Make sure the second tab is selected.
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/browser_navigator.cc
Expand Up @@ -472,7 +472,8 @@ base::WeakPtr<content::NavigationHandle> LoadURLInContents(
load_url_params.navigation_ui_data =
ChromeNavigationUIData::CreateForMainFrameNavigation(
target_contents, params->disposition,
params->is_using_https_as_default_scheme);
params->is_using_https_as_default_scheme,
params->url_typed_with_http_scheme);
}

if (params->post_data) {
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/browser_navigator_params.h
Expand Up @@ -340,6 +340,10 @@ struct NavigateParams {
// observed and fall back to using http scheme if necessary.
bool is_using_https_as_default_scheme = false;

// True if the navigation was initiated by typing in the omnibox and the typed
// text had an explicit http scheme.
bool url_typed_with_http_scheme = false;

// Indicates if the page load occurs during a non-optimal performance state.
// This value is only suggested based upon the load context, and can be
// overridden by other factors.
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/location_bar/location_bar.h
Expand Up @@ -32,6 +32,7 @@ class LocationBar {
virtual ui::PageTransition GetPageTransition() const = 0;
virtual base::TimeTicks GetMatchSelectionTimestamp() const = 0;
virtual bool IsInputTypedUrlWithoutScheme() const = 0;
virtual bool IsInputTypedUrlWithHttpScheme() const = 0;

// Focuses the location bar. User-initiated focuses (like pressing Ctrl+L)
// should have |is_user_initiated| set to true. In those cases, we want to
Expand Down
Expand Up @@ -41,6 +41,7 @@ void ChromeOmniboxEditModelDelegate::OnAutocompleteAccept(
AutocompleteMatchType::Type match_type,
base::TimeTicks match_selection_timestamp,
bool destination_url_entered_without_scheme,
bool destination_url_entered_with_http_scheme,
const std::u16string& text,
const AutocompleteMatch& match,
const AutocompleteMatch& alternative_nav_match,
Expand All @@ -56,6 +57,8 @@ void ChromeOmniboxEditModelDelegate::OnAutocompleteAccept(
match_selection_timestamp_ = match_selection_timestamp;
destination_url_entered_without_scheme_ =
destination_url_entered_without_scheme;
destination_url_entered_with_http_scheme_ =
destination_url_entered_with_http_scheme;

if (browser_) {
auto navigation = chrome::OpenCurrentURL(browser_);
Expand Down
Expand Up @@ -37,6 +37,7 @@ class ChromeOmniboxEditModelDelegate : public OmniboxEditModelDelegate {
AutocompleteMatchType::Type type,
base::TimeTicks match_selection_timestamp,
bool destination_url_entered_without_scheme,
bool destination_url_entered_with_http_scheme,
const std::u16string& text,
const AutocompleteMatch& match,
const AutocompleteMatch& alternative_nav_match,
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/views/location_bar/location_bar_view.cc
Expand Up @@ -1162,6 +1162,10 @@ bool LocationBarView::IsInputTypedUrlWithoutScheme() const {
return destination_url_entered_without_scheme();
}

bool LocationBarView::IsInputTypedUrlWithHttpScheme() const {
return destination_url_entered_with_http_scheme();
}

WindowOpenDisposition LocationBarView::GetWindowOpenDisposition() const {
return disposition();
}
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/views/location_bar/location_bar_view.h
Expand Up @@ -319,6 +319,7 @@ class LocationBarView : public LocationBar,
// LocationBar:
GURL GetDestinationURL() const override;
bool IsInputTypedUrlWithoutScheme() const override;
bool IsInputTypedUrlWithHttpScheme() const override;
WindowOpenDisposition GetWindowOpenDisposition() const override;
ui::PageTransition GetPageTransition() const override;
base::TimeTicks GetMatchSelectionTimestamp() const override;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/webui/realbox/realbox_handler.cc
Expand Up @@ -882,6 +882,7 @@ void RealboxHandler::OnAutocompleteAccept(
AutocompleteMatchType::Type match_type,
base::TimeTicks match_selection_timestamp,
bool destination_url_entered_without_scheme,
bool destination_url_entered_with_http_scheme,
const std::u16string& text,
const AutocompleteMatch& match,
const AutocompleteMatch& alternative_nav_match,
Expand All @@ -893,6 +894,8 @@ void RealboxHandler::OnAutocompleteAccept(
match_selection_timestamp_ = match_selection_timestamp;
destination_url_entered_without_scheme_ =
destination_url_entered_without_scheme;
destination_url_entered_with_http_scheme_ =
destination_url_entered_with_http_scheme;

web_contents_->OpenURL(content::OpenURLParams(
destination_url_, content::Referrer(), disposition_, transition_, false));
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/webui/realbox/realbox_handler.h
Expand Up @@ -112,6 +112,7 @@ class RealboxHandler : public omnibox::mojom::PageHandler,
AutocompleteMatchType::Type match_type,
base::TimeTicks match_selection_timestamp,
bool destination_url_entered_without_scheme,
bool destination_url_entered_with_http_scheme,
const std::u16string& text,
const AutocompleteMatch& match,
const AutocompleteMatch& alternative_nav_match,
Expand Down
4 changes: 4 additions & 0 deletions chrome/test/base/test_browser_window.cc
Expand Up @@ -73,6 +73,10 @@ bool TestBrowserWindow::TestLocationBar::IsInputTypedUrlWithoutScheme() const {
return false;
}

bool TestBrowserWindow::TestLocationBar::IsInputTypedUrlWithHttpScheme() const {
return false;
}

// TestBrowserWindow ----------------------------------------------------------

TestBrowserWindow::TestBrowserWindow() {}
Expand Down
1 change: 1 addition & 0 deletions chrome/test/base/test_browser_window.h
Expand Up @@ -289,6 +289,7 @@ class TestBrowserWindow : public BrowserWindow {
// LocationBar:
GURL GetDestinationURL() const override;
bool IsInputTypedUrlWithoutScheme() const override;
bool IsInputTypedUrlWithHttpScheme() const override;
WindowOpenDisposition GetWindowOpenDisposition() const override;
ui::PageTransition GetPageTransition() const override;
base::TimeTicks GetMatchSelectionTimestamp() const override;
Expand Down
3 changes: 2 additions & 1 deletion components/omnibox/browser/actions/omnibox_action.cc
Expand Up @@ -131,7 +131,8 @@ void OmniboxAction::OpenURL(OmniboxAction::ExecutionContext& context,
.Run(url, nullptr, context.disposition_, ui::PAGE_TRANSITION_GENERATED,
/*match_type=*/AutocompleteMatchType::URL_WHAT_YOU_TYPED,
context.match_selection_timestamp_,
/*destination_url_entered_without_scheme=*/false, u"",
/*destination_url_entered_without_scheme=*/false,
/*destination_url_entered_with_http_scheme=*/false, u"",
AutocompleteMatch(), AutocompleteMatch(),
IDNA2008DeviationCharacter::kNone);
}
1 change: 1 addition & 0 deletions components/omnibox/browser/actions/omnibox_action.h
Expand Up @@ -115,6 +115,7 @@ class OmniboxAction : public base::RefCounted<OmniboxAction> {
AutocompleteMatchType::Type match_type,
base::TimeTicks match_selection_timestamp,
bool destination_url_entered_without_scheme,
bool destination_url_entered_with_http_scheme,
const std::u16string&,
const AutocompleteMatch&,
const AutocompleteMatch&,
Expand Down

0 comments on commit 403eaca

Please sign in to comment.