Commit be8c96c0 authored by Mustafa Emre Acer's avatar Mustafa Emre Acer Committed by Chromium LUCI CQ

Default typed omnibox navigations to https: Fix ports and IP addresses

Hostnames typed without a scheme but with a non-default port shouldn't
be upgraded to HTTPS. In such a case, we can't simply change the scheme
to https:// and expect the site to load since it's likely that the HTTPS
version of the URL is also served from a non-default port.

This CL also fixes handling of IP addresses when there is a port.
Previously, we were checking whether the entered text was an IP address.
This breaks down if the IP address has a port attached to it such as
127.0.0.1:8080. This CL instead checks the canonicalized_url.

Bug: 1141691
Change-Id: Iffefa5c0ee939b361d8152c5cd417756841b5923
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623683
Auto-Submit: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842552}
parent fc3d2711
......@@ -154,8 +154,9 @@ void AutocompleteInput::Init(
type_ == metrics::OmniboxInputType::URL &&
scheme_ == base::ASCIIToUTF16(url::kHttpScheme) &&
!base::StartsWith(text_, scheme_, base::CompareCase::INSENSITIVE_ASCII) &&
!url::HostIsIPAddress(base::UTF16ToUTF8(text)) &&
!net::IsHostnameNonUnique(base::UTF16ToUTF8(text))) {
!url::HostIsIPAddress(canonicalized_url.host()) &&
!net::IsHostnameNonUnique(base::UTF16ToUTF8(text)) &&
canonicalized_url.port().empty()) {
// Use HTTPS as the default scheme for URLs that are typed without a scheme.
// Inputs of type UNKNOWN can still be valid URLs, but these will be mainly
// intranet hosts which we don't to upgrade to HTTPS so we only check the
......@@ -164,7 +165,11 @@ void AutocompleteInput::Init(
// - Non-unique hostnames such as intranet hosts
// - Single word hostnames (these are most likely non-unique).
// - IP addresses
// TODO(crbug.com/1141691): Add tests for these cases.
// - URLs with a specified port. If it's a non-standard HTTP port, we can't
// simply change the scheme to HTTPS and assume that these will load over
// HTTPS. URLs with HTTP port 80 get their port dropped so they will be
// upgraded (e.g. example.com:80 will load https://example.com).
DCHECK_EQ(url::kHttpScheme, canonicalized_url.scheme());
added_default_scheme_to_typed_url_ = true;
scheme_ = base::ASCIIToUTF16(url::kHttpsScheme);
GURL::Replacements replacements;
......
......@@ -382,15 +382,31 @@ TEST(AutocompleteInputTest, UpgradeTypedNavigationsToHttps) {
bool expected_added_default_scheme_to_typed_url;
} input_cases[]{
{ASCIIToUTF16("example.com"), GURL("https://example.com"), true},
// If the hostname has a port specified, the URL shouldn't be upgraded
// to HTTPS because we can't assume that the HTTPS site is served over the
// default SSL port. Port 80 is dropped in URLs so it's still upgraded.
{ASCIIToUTF16("example.com:80"), GURL("https://example.com"), true},
{ASCIIToUTF16("example.com:8080"), GURL("http://example.com:8080"),
false},
// Non-URL inputs shouldn't be upgraded.
{ASCIIToUTF16("example query"), GURL(), false},
// IP addresses shouldn't be upgraded.
{ASCIIToUTF16("127.0.0.1"), GURL("http://127.0.0.1"), false},
{ASCIIToUTF16("127.0.0.1:80"), GURL("http://127.0.0.1:80"), false},
{ASCIIToUTF16("127.0.0.1:8080"), GURL("http://127.0.0.1:8080"), false},
// Non-unique hostnames shouldn't be upgraded.
{ASCIIToUTF16("site.test"), GURL("http://site.test"), false},
// Fully typed URLs shouldn't be upgraded.
{ASCIIToUTF16("http://example.com"), GURL("http://example.com"), false},
{ASCIIToUTF16("HTTP://EXAMPLE.COM"), GURL("http://example.com"), false},
{ASCIIToUTF16("http://example.com:80"), GURL("http://example.com"),
false},
{ASCIIToUTF16("HTTP://EXAMPLE.COM:80"), GURL("http://example.com"),
false},
{ASCIIToUTF16("http://example.com:8080"), GURL("http://example.com:8080"),
false},
{ASCIIToUTF16("HTTP://EXAMPLE.COM:8080"), GURL("http://example.com:8080"),
false},
};
for (const test_data& input_case : input_cases) {
AutocompleteInput input(input_case.input, base::string16::npos,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment