Changes FormatURL to not strip http if the host starts with ftp or https[!a-z]. This

is necessitated by the omnibox treating ftp.www.com as
ftp://www.com as well as security problems.

I also changed FormatURL to always reset the supplied
url_parse::Parsed. I did this as in writing my test I uncovered a long
standing bug where a component wasn't getting reset. It seems much
safer to always reset than rely on all code paths setting every
component.

BUG=41585, 41652
TEST=make sure the omnibox doesn't strip http off urls that start with
ftp.

Review URL: http://codereview.chromium.org/1574034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44943 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 62 insertions, 2 deletions

diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 099746b..b251ee1 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -1392,6 +1392,8 @@ std::wstring FormatUrl(const GURL& url,
   url_parse::Parsed parsed_temp;
   if (!new_parsed)
     new_parsed = &parsed_temp;
+  else
+    *new_parsed = url_parse::Parsed();
   size_t offset_temp = std::wstring::npos;
   if (!offset_for_adjustment)
     offset_for_adjustment = &offset_temp;
@@ -1432,9 +1434,20 @@ std::wstring FormatUrl(const GURL& url,
       std::back_inserter(url_string));
 
   const wchar_t* const kHTTP = L"http://";
+  const char* const kFTP = "ftp.";
   const size_t kHTTPSize = std::wstring(kHTTP).size();
-  bool omit_http = ((format_types & kFormatUrlOmitHTTP) != 0 &&
-                    url_string == kHTTP);
+  // The omnibox treats ftp.foo.com as ftp://foo.com. This means that if we
+  // trimmed http off a string that starts with http://ftp and the user tried to
+  // reload the page the user would end up with a scheme of ftp://. For example,
+  // 'http://ftp.foo.com' -> 'ftp.foo.com' -> 'ftp://foo.com'. For this reason
+  // don't strip http off url's whose scheme is http and the host starts with
+  // 'ftp.'.
+  bool omit_http =
+      ((format_types & kFormatUrlOmitHTTP) != 0 &&
+       url_string == kHTTP && (!parsed.host.is_valid() ||
+                               (parsed.host.is_nonempty() &&
+                                spec.compare(parsed.host.begin,
+                                             std::string(kFTP).size(), kFTP))));
 
   new_parsed->scheme = parsed.scheme;
 
diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc
index 4c1ae52..30ab0b99 100644
--- a/net/base/net_util_unittest.cc
+++ b/net/base/net_util_unittest.cc
@@ -1389,6 +1389,11 @@ TEST(NetUtilTest, FormatUrl) {
      "https://www.google.com/", L"en", net::kFormatUrlOmitHTTP,
      UnescapeRule::NORMAL, L"https://www.google.com/",
      8},
+
+    {"omit http starts with ftp.",
+     "http://ftp.google.com/", L"en", net::kFormatUrlOmitHTTP,
+     UnescapeRule::NORMAL, L"http://ftp.google.com/",
+     7},
   };
 
   for (size_t i = 0; i < arraysize(tests); ++i) {
@@ -1491,6 +1496,36 @@ TEST(NetUtilTest, FormatUrlParsed) {
   EXPECT_EQ(L"/a", formatted.substr(parsed.path.begin, parsed.path.len));
   EXPECT_EQ(L"b=c", formatted.substr(parsed.query.begin, parsed.query.len));
   EXPECT_EQ(L"d", formatted.substr(parsed.ref.begin, parsed.ref.len));
+
+  // omit http starts with ftp case.
+  formatted = net::FormatUrl(
+      GURL("http://ftp.host:8000/a?b=c#d"),
+      L"", net::kFormatUrlOmitHTTP, UnescapeRule::NORMAL, &parsed, NULL, NULL);
+  EXPECT_EQ(L"http://ftp.host:8000/a?b=c#d", formatted);
+  EXPECT_TRUE(parsed.scheme.is_valid());
+  EXPECT_FALSE(parsed.username.is_valid());
+  EXPECT_FALSE(parsed.password.is_valid());
+  EXPECT_EQ(L"http", formatted.substr(parsed.scheme.begin, parsed.scheme.len));
+  EXPECT_EQ(L"ftp.host", formatted.substr(parsed.host.begin, parsed.host.len));
+  EXPECT_EQ(L"8000", formatted.substr(parsed.port.begin, parsed.port.len));
+  EXPECT_EQ(L"/a", formatted.substr(parsed.path.begin, parsed.path.len));
+  EXPECT_EQ(L"b=c", formatted.substr(parsed.query.begin, parsed.query.len));
+  EXPECT_EQ(L"d", formatted.substr(parsed.ref.begin, parsed.ref.len));
+
+  // omit http starts with 'f' case.
+  formatted = net::FormatUrl(
+      GURL("http://f/"),
+      L"", net::kFormatUrlOmitHTTP, UnescapeRule::NORMAL, &parsed, NULL, NULL);
+  EXPECT_EQ(L"f/", formatted);
+  EXPECT_FALSE(parsed.scheme.is_valid());
+  EXPECT_FALSE(parsed.username.is_valid());
+  EXPECT_FALSE(parsed.password.is_valid());
+  EXPECT_FALSE(parsed.port.is_valid());
+  EXPECT_TRUE(parsed.path.is_valid());
+  EXPECT_FALSE(parsed.query.is_valid());
+  EXPECT_FALSE(parsed.ref.is_valid());
+  EXPECT_EQ(L"f", formatted.substr(parsed.host.begin, parsed.host.len));
+  EXPECT_EQ(L"/", formatted.substr(parsed.path.begin, parsed.path.len));
 }
 
 TEST(NetUtilTest, FormatUrlAdjustOffset) {
@@ -1629,6 +1664,18 @@ TEST(NetUtilTest, FormatUrlAdjustOffset) {
     EXPECT_EQ(omit_http_cases[i].output_offset, offset);
   }
 
+  const AdjustOffsetCase omit_http_start_with_ftp[] = {
+    {0, 0},
+    {3, 3},
+    {8, 8},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(omit_http_start_with_ftp); ++i) {
+    size_t offset = omit_http_start_with_ftp[i].input_offset;
+    net::FormatUrl(GURL("http://ftp.google.com"), L"en",
+        net::kFormatUrlOmitHTTP, UnescapeRule::NORMAL, NULL, NULL, &offset);
+    EXPECT_EQ(omit_http_start_with_ftp[i].output_offset, offset);
+  }
+
   const AdjustOffsetCase omit_all_cases[] = {
     {12, 0},
     {13, 1},