diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-02 02:37:40 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-02 02:37:40 +0000 |
commit | 79845effcd569cb61784e8a8c221f839e6e23525 (patch) | |
tree | d86e942097765499876b48a5d764592e2b9c376f | |
parent | 9902c201466fe4a701f43a124f4d22b6829c87e6 (diff) | |
download | chromium_src-79845effcd569cb61784e8a8c221f839e6e23525.zip chromium_src-79845effcd569cb61784e8a8c221f839e6e23525.tar.gz chromium_src-79845effcd569cb61784e8a8c221f839e6e23525.tar.bz2 |
Strip the trailing slash from URLs like "http://google.com/". This especially helps when the scheme has also been stripped, as it makes the hostname look less unbalanced. We're careful to avoid stripping the slash when doing so would confuse the omnibox.
This also moves to more aggressive stripping and/or unescaping in several places. In general, it seems like we should be as aggressive as is feasible.
BUG=43587
TEST=Visit google.com. There should be no slash in the address bar.
Review URL: http://codereview.chromium.org/2389002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48691 0039d316-1c4b-4281-b951-d872f2087c98
20 files changed, 169 insertions, 87 deletions
diff --git a/app/text_elider_unittest.cc b/app/text_elider_unittest.cc index 0eacfd2..30c494f 100644 --- a/app/text_elider_unittest.cc +++ b/app/text_elider_unittest.cc @@ -91,7 +91,7 @@ TEST(TextEliderTest, TestMoreEliding) { {"", L""}, {"http://foo.bar..example.com...hello/test/filename.html", L"foo.bar..example.com...hello/" + kEllipsisStr + L"/filename.html"}, - {"http://foo.bar../", L"foo.bar../"}, + {"http://foo.bar../", L"foo.bar.."}, {"http://xn--1lq90i.cn/foo", L"\x5317\x4eac.cn/foo"}, {"http://me:mypass@secrethost.com:99/foo?bar#baz", L"secrethost.com:99/foo?bar#baz"}, @@ -251,8 +251,8 @@ TEST(TextEliderTest, ElideTextLongStrings) { // Verifies display_url is set correctly. TEST(TextEliderTest, SortedDisplayURL) { - gfx::SortedDisplayURL d_url(GURL("http://www.google.com/"), std::wstring()); - EXPECT_EQ("www.google.com/", UTF16ToASCII(d_url.display_url())); + gfx::SortedDisplayURL d_url(GURL("http://www.google.com"), std::wstring()); + EXPECT_EQ("www.google.com", UTF16ToASCII(d_url.display_url())); } // Verifies DisplayURL::Compare works correctly. diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 035a1ea..d92677e 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -90,8 +90,6 @@ AutocompleteInput::Type AutocompleteInput::Parse( const std::wstring& desired_tld, url_parse::Parsed* parts, std::wstring* scheme) { - DCHECK(parts); - const size_t first_non_white = text.find_first_not_of(kWhitespaceWide, 0); if (first_non_white == std::wstring::npos) return INVALID; // All whitespace. @@ -106,6 +104,9 @@ AutocompleteInput::Type AutocompleteInput::Parse( // use the URLFixerUpper here because we want to be smart about what we // consider a scheme. For example, we shouldn't consider www.google.com:80 // to have a scheme. + url_parse::Parsed local_parts; + if (!parts) + parts = &local_parts; const std::wstring parsed_scheme(URLFixerUpper::SegmentURL(text, parts)); if (scheme) *scheme = parsed_scheme; @@ -344,6 +345,19 @@ void AutocompleteInput::ParseForEmphasizeComponents( } } +// static +std::wstring AutocompleteInput::FormattedStringWithEquivalentMeaning( + const GURL& url, + const std::wstring& formatted_url) { + if (!net::CanStripTrailingSlash(url)) + return formatted_url; + const std::wstring url_with_path(formatted_url + L"/"); + return (AutocompleteInput::Parse(formatted_url, std::wstring(), NULL, NULL) == + AutocompleteInput::Parse(url_with_path, std::wstring(), NULL, NULL)) ? + formatted_url : url_with_path; +} + + bool AutocompleteInput::Equals(const AutocompleteInput& other) const { return (text_ == other.text_) && (type_ == other.type_) && @@ -591,16 +605,13 @@ void AutocompleteProvider::UpdateStarredStateOfMatches() { i->starred = bookmark_model->IsBookmarked(GURL(i->destination_url)); } -std::wstring AutocompleteProvider::StringForURLDisplay( - const GURL& url, - bool check_accept_lang, - bool trim_http) const { - std::wstring languages = (check_accept_lang && profile_) ? - profile_->GetPrefs()->GetString(prefs::kAcceptLanguages) : std::wstring(); - const net::FormatUrlTypes format_types = trim_http ? - net::kFormatUrlOmitAll : net::kFormatUrlOmitUsernamePassword; - return net::FormatUrl(url, languages, format_types, UnescapeRule::SPACES, - NULL, NULL, NULL); +std::wstring AutocompleteProvider::StringForURLDisplay(const GURL& url, + bool check_accept_lang, + bool trim_http) const { + return net::FormatUrl(url, (check_accept_lang && profile_) ? + profile_->GetPrefs()->GetString(prefs::kAcceptLanguages) : std::wstring(), + net::kFormatUrlOmitAll & ~(trim_http ? 0 : net::kFormatUrlOmitHTTP), + UnescapeRule::SPACES, NULL, NULL, NULL); } // AutocompleteResult --------------------------------------------------------- diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index e7766dd..4f85cf4 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -190,7 +190,8 @@ class AutocompleteInput { static std::string TypeToString(Type type); // Parses |text| and returns the type of input this will be interpreted as. - // The components of the input are stored in the output parameter |parts|. + // The components of the input are stored in the output parameter |parts|, if + // it is non-NULL. static Type Parse(const std::wstring& text, const std::wstring& desired_tld, url_parse::Parsed* parts, @@ -205,6 +206,16 @@ class AutocompleteInput { url_parse::Component* scheme, url_parse::Component* host); + // Code that wants to format URLs with a format flag including + // net::kFormatUrlOmitTrailingSlashOnBareHostname risk changing the meaning if + // the result is then parsed as AutocompleteInput. Such code can call this + // function with the URL and its formatted string, and it will return a + // formatted string with the same meaning as the original URL (i.e. it will + // re-append a slash if necessary). + static std::wstring FormattedStringWithEquivalentMeaning( + const GURL& url, + const std::wstring& formatted_url); + // User-provided text to be completed. const std::wstring& text() const { return text_; } diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index de74891..976e2e0 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -158,9 +158,8 @@ AutocompleteMatch::Type AutocompleteEditModel::CurrentTextType() const { bool AutocompleteEditModel::GetURLForText(const std::wstring& text, GURL* url) const { - url_parse::Parsed parts; const AutocompleteInput::Type type = AutocompleteInput::Parse( - UserTextFromDisplayText(text), std::wstring(), &parts, NULL); + UserTextFromDisplayText(text), std::wstring(), NULL, NULL); if (type != AutocompleteInput::URL) return false; diff --git a/chrome/browser/autocomplete/history_contents_provider.cc b/chrome/browser/autocomplete/history_contents_provider.cc index 5ad20c5..deb35d2 100644 --- a/chrome/browser/autocomplete/history_contents_provider.cc +++ b/chrome/browser/autocomplete/history_contents_provider.cc @@ -207,9 +207,11 @@ AutocompleteMatch HistoryContentsProvider::ResultToMatch( // Also show star in popup. AutocompleteMatch match(this, score, false, MatchInTitle(result) ? AutocompleteMatch::HISTORY_TITLE : AutocompleteMatch::HISTORY_BODY); - match.fill_into_edit = StringForURLDisplay(result.url(), true, trim_http_); + match.contents = StringForURLDisplay(result.url(), true, trim_http_); + match.fill_into_edit = + AutocompleteInput::FormattedStringWithEquivalentMeaning(result.url(), + match.contents); match.destination_url = result.url(); - match.contents = match.fill_into_edit; match.contents_class.push_back( ACMatchClassification(0, ACMatchClassification::URL)); match.description = result.title(); diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc index 8880a12..703b909 100644 --- a/chrome/browser/autocomplete/history_url_provider.cc +++ b/chrome/browser/autocomplete/history_url_provider.cc @@ -256,21 +256,26 @@ AutocompleteMatch HistoryURLProvider::SuggestExactInput( const GURL& url = input.canonicalized_url(); if (url.is_valid()) { match.destination_url = url; - match.fill_into_edit = StringForURLDisplay(url, false, false); + + // Trim off "http://" if the user didn't type it. + // NOTE: We use TrimHttpPrefix() here rather than StringForURLDisplay() to + // strip the scheme as we need to know the offset so we can adjust the + // |match_location| below. StringForURLDisplay() and TrimHttpPrefix() have + // slightly different behavior as well (the latter will strip even without + // two slashes after the scheme). + std::wstring display_string(StringForURLDisplay(url, false, false)); + const size_t offset = trim_http ? TrimHttpPrefix(&display_string) : 0; + match.fill_into_edit = + AutocompleteInput::FormattedStringWithEquivalentMeaning(url, + display_string); // NOTE: Don't set match.input_location (to allow inline autocompletion) // here, it's surprising and annoying. - // Trim off "http://" if the user didn't type it. - // Double NOTE: we use TrimHttpPrefix here rather than StringForURLDisplay - // to strip the http as we need to know the offset so we can adjust the - // match_location below. StringForURLDisplay and TrimHttpPrefix have - // slightly different behavior when stripping http as well. - const size_t offset = trim_http ? TrimHttpPrefix(&match.fill_into_edit) : 0; // Try to highlight "innermost" match location. If we fix up "w" into // "www.w.com", we want to highlight the fifth character, not the first. // This relies on match.destination_url being the non-prefix-trimmed version // of match.contents. - match.contents = match.fill_into_edit; + match.contents = display_string; const Prefix* best_prefix = BestPrefix(match.destination_url, input.text()); // Because of the vagaries of GURL, it's possible for match.destination_url // to not contain the user's input at all. In this case don't mark anything @@ -832,13 +837,14 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch( DCHECK(match.destination_url.is_valid()); size_t inline_autocomplete_offset = history_match.input_location + params->input.text().length(); - const net::FormatUrlTypes format_types = - (params->trim_http && !history_match.match_in_scheme) ? - net::kFormatUrlOmitAll : net::kFormatUrlOmitUsernamePassword; - match.fill_into_edit = net::FormatUrl(info.url(), - match_type == WHAT_YOU_TYPED ? std::wstring() : params->languages, - format_types, UnescapeRule::SPACES, NULL, NULL, - &inline_autocomplete_offset); + const net::FormatUrlTypes format_types = net::kFormatUrlOmitAll & + ~((params->trim_http && !history_match.match_in_scheme) ? + 0 : net::kFormatUrlOmitHTTP); + match.fill_into_edit = + AutocompleteInput::FormattedStringWithEquivalentMeaning(info.url(), + net::FormatUrl(info.url(), match_type == WHAT_YOU_TYPED ? + std::wstring() : params->languages, format_types, UnescapeRule::SPACES, + NULL, NULL, &inline_autocomplete_offset)); if (!params->input.prevent_inline_autocomplete()) match.inline_autocomplete_offset = inline_autocomplete_offset; DCHECK((match.inline_autocomplete_offset == std::wstring::npos) || diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index beda1f3..ba550a6 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -754,7 +754,9 @@ AutocompleteMatch SearchProvider::NavigationToMatch( // suggestion, non-Search results will suddenly appear. if (input_.type() == AutocompleteInput::FORCED_QUERY) match.fill_into_edit.assign(L"?"); - match.fill_into_edit.append(match.contents); + match.fill_into_edit.append( + AutocompleteInput::FormattedStringWithEquivalentMeaning(navigation.url, + match.contents)); // TODO(pkasting): http://b/1112879 These should perhaps be // inline-autocompletable? diff --git a/chrome/browser/cocoa/status_bubble_mac_unittest.mm b/chrome/browser/cocoa/status_bubble_mac_unittest.mm index 6f4dab4..6a15f8b 100644 --- a/chrome/browser/cocoa/status_bubble_mac_unittest.mm +++ b/chrome/browser/cocoa/status_bubble_mac_unittest.mm @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -141,7 +141,7 @@ TEST_F(StatusBubbleMacTest, SetURL) { EXPECT_TRUE([GetURLText() isEqualToString:@"foopy://"]); bubble_->SetURL(GURL("http://www.cnn.com"), L""); EXPECT_TRUE(IsVisible()); - EXPECT_TRUE([GetURLText() isEqualToString:@"www.cnn.com/"]); + EXPECT_TRUE([GetURLText() isEqualToString:@"www.cnn.com"]); } // Test hiding bubble that's already hidden. @@ -160,23 +160,23 @@ TEST_F(StatusBubbleMacTest, SetStatusAndURL) { bubble_->SetStatus(L"Status"); EXPECT_TRUE(IsVisible()); EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"Status"]); - bubble_->SetURL(GURL("http://www.nytimes.com/"), L""); + bubble_->SetURL(GURL("http://www.nytimes.com"), L""); EXPECT_TRUE(IsVisible()); - EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com/"]); + EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com"]); bubble_->SetURL(GURL(), L""); EXPECT_TRUE(IsVisible()); EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"Status"]); bubble_->SetStatus(L""); EXPECT_FALSE(IsVisible()); - bubble_->SetURL(GURL("http://www.nytimes.com/"), L""); + bubble_->SetURL(GURL("http://www.nytimes.com"), L""); EXPECT_TRUE(IsVisible()); - EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com/"]); + EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com"]); bubble_->SetStatus(L"Status"); EXPECT_TRUE(IsVisible()); EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"Status"]); bubble_->SetStatus(L""); EXPECT_TRUE(IsVisible()); - EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com/"]); + EXPECT_TRUE([GetBubbleViewText() isEqualToString:@"www.nytimes.com"]); bubble_->SetURL(GURL(), L""); EXPECT_FALSE(IsVisible()); } diff --git a/chrome/browser/gtk/options/passwords_exceptions_page_gtk.cc b/chrome/browser/gtk/options/passwords_exceptions_page_gtk.cc index 9405fe1..a5bf522 100644 --- a/chrome/browser/gtk/options/passwords_exceptions_page_gtk.cc +++ b/chrome/browser/gtk/options/passwords_exceptions_page_gtk.cc @@ -115,8 +115,7 @@ void PasswordsExceptionsPageGtk::SetExceptionList( exception_list_.resize(result.size()); for (size_t i = 0; i < result.size(); ++i) { exception_list_[i] = *result[i]; - std::wstring formatted = net::FormatUrl(result[i]->origin, languages, - net::kFormatUrlOmitAll, UnescapeRule::NONE, NULL, NULL, NULL); + std::wstring formatted = net::FormatUrl(result[i]->origin, languages); std::string site = WideToUTF8(formatted); GtkTreeIter iter; gtk_list_store_insert_with_values(exception_list_store_, &iter, (gint) i, diff --git a/chrome/browser/gtk/options/passwords_page_gtk.cc b/chrome/browser/gtk/options/passwords_page_gtk.cc index 9ecf89f..90cd7b6 100644 --- a/chrome/browser/gtk/options/passwords_page_gtk.cc +++ b/chrome/browser/gtk/options/passwords_page_gtk.cc @@ -159,8 +159,7 @@ void PasswordsPageGtk::SetPasswordList( password_list_.resize(result.size()); for (size_t i = 0; i < result.size(); ++i) { password_list_[i] = *result[i]; - std::wstring formatted = net::FormatUrl(result[i]->origin, languages, - net::kFormatUrlOmitAll, UnescapeRule::NONE, NULL, NULL, NULL); + std::wstring formatted = net::FormatUrl(result[i]->origin, languages); std::string site = WideToUTF8(formatted); std::string user = UTF16ToUTF8(result[i]->username_value); GtkTreeIter iter; diff --git a/chrome/browser/gtk/options/url_picker_dialog_gtk.cc b/chrome/browser/gtk/options/url_picker_dialog_gtk.cc index 860a33f..dd51798 100644 --- a/chrome/browser/gtk/options/url_picker_dialog_gtk.cc +++ b/chrome/browser/gtk/options/url_picker_dialog_gtk.cc @@ -198,11 +198,12 @@ std::string UrlPickerDialogGtk::GetURLForPath(GtkTreePath* path) const { } std::wstring languages = profile_->GetPrefs()->GetString(prefs::kAcceptLanguages); - // Because the url_field_ is user-editable, we set the URL with - // username:password and escaped path and query. + // Because this gets parsed by FixupURL(), it's safe to omit the scheme or + // trailing slash, and unescape most characters, but we need to not drop any + // username/password, or unescape anything that changes the meaning. std::wstring formatted = net::FormatUrl(url_table_model_->GetURL(row), - languages, net::kFormatUrlOmitNothing, UnescapeRule::NONE, NULL, NULL, - NULL); + languages, net::kFormatUrlOmitAll & ~net::kFormatUrlOmitUsernamePassword, + UnescapeRule::SPACES, NULL, NULL, NULL); return WideToUTF8(formatted); } diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc index 658cbee..b60db13 100644 --- a/chrome/browser/tab_contents/navigation_controller_unittest.cc +++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc @@ -1623,8 +1623,8 @@ TEST_F(NavigationControllerTest, SameSubframe) { // Test view source redirection is reflected in title bar. TEST_F(NavigationControllerTest, ViewSourceRedirect) { const char kUrl[] = "view-source:http://redirect.to/google.com"; - const char kResult[] = "http://google.com/"; - const char kExpected[] = "view-source:http://google.com/"; + const char kResult[] = "http://google.com"; + const char kExpected[] = "view-source:http://google.com"; const GURL url(kUrl); const GURL result_url(kResult); diff --git a/chrome/browser/tab_contents/navigation_entry_unittest.cc b/chrome/browser/tab_contents/navigation_entry_unittest.cc index d742d3e..4ecd1df 100644 --- a/chrome/browser/tab_contents/navigation_entry_unittest.cc +++ b/chrome/browser/tab_contents/navigation_entry_unittest.cc @@ -57,8 +57,8 @@ TEST_F(NavigationEntryTest, NavigationEntryURLs) { // Setting URL affects virtual_url and GetTitleForDisplay entry1_.get()->set_url(GURL("http://www.google.com")); EXPECT_EQ(GURL("http://www.google.com"), entry1_.get()->url()); - EXPECT_EQ(GURL("http://www.google.com/"), entry1_.get()->virtual_url()); - EXPECT_EQ(ASCIIToUTF16("http://www.google.com/"), + EXPECT_EQ(GURL("http://www.google.com"), entry1_.get()->virtual_url()); + EXPECT_EQ(ASCIIToUTF16("www.google.com"), entry1_.get()->GetTitleForDisplay(NULL)); // Title affects GetTitleForDisplay diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index 9e95563..682aac6 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -233,7 +233,7 @@ TEST_F(TabContentsTest, UpdateTitle) { // Test view source mode for the new tabs page. TEST_F(TabContentsTest, NTPViewSource) { - const char kUrl[] = "view-source:chrome://newtab/"; + const char kUrl[] = "view-source:chrome://newtab"; const GURL kGURL(kUrl); process()->sink().ClearMessages(); diff --git a/chrome/browser/toolbar_model.cc b/chrome/browser/toolbar_model.cc index a591e5c..740bc3e9 100644 --- a/chrome/browser/toolbar_model.cc +++ b/chrome/browser/toolbar_model.cc @@ -53,8 +53,12 @@ std::wstring ToolbarModel::GetText() const { url = GURL(url.scheme() + ":"); } } - return net::FormatUrl(url, languages, net::kFormatUrlOmitAll, - UnescapeRule::NORMAL, NULL, NULL, NULL); + // Note that we can't unescape spaces here, because if the user copies this + // and pastes it into another program, that program may think the URL ends at + // the space. + return AutocompleteInput::FormattedStringWithEquivalentMeaning(url, + net::FormatUrl(url, languages, net::kFormatUrlOmitAll, + UnescapeRule::NORMAL, NULL, NULL, NULL)); } ToolbarModel::SecurityLevel ToolbarModel::GetSecurityLevel() const { diff --git a/chrome/browser/views/bookmark_editor_view.cc b/chrome/browser/views/bookmark_editor_view.cc index 616000b..f0aa7e9 100644 --- a/chrome/browser/views/bookmark_editor_view.cc +++ b/chrome/browser/views/bookmark_editor_view.cc @@ -279,9 +279,12 @@ void BookmarkEditorView::Init() { std::wstring languages = profile_ ? profile_->GetPrefs()->GetString(prefs::kAcceptLanguages) : std::wstring(); - // The following URL is user-editable, so we don't strip anything from it. + // Because this gets parsed by FixupURL(), it's safe to omit the scheme or + // trailing slash, and unescape most characters, but we need to not drop any + // username/password, or unescape anything that changes the meaning. url_text = net::FormatUrl(details_.existing_node->GetURL(), languages, - net::kFormatUrlOmitNothing, UnescapeRule::NONE, NULL, NULL, NULL); + net::kFormatUrlOmitAll & ~net::kFormatUrlOmitUsernamePassword, + UnescapeRule::SPACES, NULL, NULL, NULL); } url_tf_.SetText(url_text); url_tf_.SetController(this); diff --git a/chrome/browser/views/url_picker.cc b/chrome/browser/views/url_picker.cc index 3d275d4..116075f 100644 --- a/chrome/browser/views/url_picker.cc +++ b/chrome/browser/views/url_picker.cc @@ -221,10 +221,13 @@ void UrlPicker::OnSelectionChanged() { if (selection >= 0 && selection < url_table_model_->RowCount()) { std::wstring languages = profile_->GetPrefs()->GetString(prefs::kAcceptLanguages); - // Because the url_field_ is user-editable, we don't strip anything. + // Because this gets parsed by FixupURL(), it's safe to omit the scheme or + // trailing slash, and unescape most characters, but we need to not drop any + // username/password, or unescape anything that changes the meaning. std::wstring formatted = net::FormatUrl(url_table_model_->GetURL(selection), - languages, net::kFormatUrlOmitNothing, UnescapeRule::NONE, NULL, NULL, - NULL); + languages, + net::kFormatUrlOmitAll & ~net::kFormatUrlOmitUsernamePassword, + UnescapeRule::SPACES, NULL, NULL, NULL); url_field_->SetText(formatted); GetDialogClientView()->UpdateDialogButtons(); } diff --git a/net/base/net_util.cc b/net/base/net_util.cc index 99b21b8..66c2dba 100644 --- a/net/base/net_util.cc +++ b/net/base/net_util.cc @@ -774,11 +774,12 @@ std::wstring FormatViewSourceUrl(const GURL& url, namespace net { -const FormatUrlType kFormatUrlOmitNothing = 0; -const FormatUrlType kFormatUrlOmitUsernamePassword = 1 << 0; -const FormatUrlType kFormatUrlOmitHTTP = 1 << 1; +const FormatUrlType kFormatUrlOmitNothing = 0; +const FormatUrlType kFormatUrlOmitUsernamePassword = 1 << 0; +const FormatUrlType kFormatUrlOmitHTTP = 1 << 1; +const FormatUrlType kFormatUrlOmitTrailingSlashOnBareHostname = 1 << 2; const FormatUrlType kFormatUrlOmitAll = kFormatUrlOmitUsernamePassword | - kFormatUrlOmitHTTP; + kFormatUrlOmitHTTP | kFormatUrlOmitTrailingSlashOnBareHostname; std::set<int> explicitly_allowed_ports; @@ -1441,21 +1442,17 @@ std::wstring FormatUrl(const GURL& url, true), 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(); - // 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.'. + const wchar_t kHTTP[] = L"http://"; + const char kFTP[] = "ftp."; + // URLFixerUpper::FixupURL() treats "ftp.foo.com" as ftp://ftp.foo.com. This + // means that if we trim "http://" off a URL whose host starts with "ftp." and + // the user inputs this into any field subject to fixup (which is basically + // all input fields), the meaning would be changed. (In fact, often the + // formatted URL is directly pre-filled into an input field.) For this reason + // we avoid stripping "http://" in this case. 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)))); + (format_types & kFormatUrlOmitHTTP) && (url_string == kHTTP) && + (url.host().compare(0, arraysize(kFTP) - 1, kFTP) != 0); new_parsed->scheme = parsed.scheme; @@ -1522,8 +1519,11 @@ std::wstring FormatUrl(const GURL& url, } // Path and query both get the same general unescape & convert treatment. - AppendFormattedComponent(spec, parsed.path, unescape_rules, &url_string, - &new_parsed->path, offset_for_adjustment); + if (!(format_types & kFormatUrlOmitTrailingSlashOnBareHostname) || + !CanStripTrailingSlash(url)) { + AppendFormattedComponent(spec, parsed.path, unescape_rules, &url_string, + &new_parsed->path, offset_for_adjustment); + } if (parsed.query.is_valid()) url_string.push_back('?'); AppendFormattedComponent(spec, parsed.query, unescape_rules, &url_string, @@ -1561,6 +1561,7 @@ std::wstring FormatUrl(const GURL& url, // If we need to strip out http do it after the fact. This way we don't need // to worry about how offset_for_adjustment is interpreted. + const size_t kHTTPSize = arraysize(kHTTP) - 1; if (omit_http && !url_string.compare(0, kHTTPSize, kHTTP)) { url_string = url_string.substr(kHTTPSize); if (*offset_for_adjustment != std::wstring::npos) { @@ -1582,6 +1583,13 @@ std::wstring FormatUrl(const GURL& url, return url_string; } +bool CanStripTrailingSlash(const GURL& url) { + // Omit the path only for standard, non-file URLs with nothing but "/" after + // the hostname. + return url.IsStandard() && !url.SchemeIsFile() && !url.has_query() && + !url.has_ref() && url.path() == "/"; +} + GURL SimplifyUrlForRequest(const GURL& url) { DCHECK(url.is_valid()); GURL::Replacements replacements; diff --git a/net/base/net_util.h b/net/base/net_util.h index 5c3e37e..a66b45c 100644 --- a/net/base/net_util.h +++ b/net/base/net_util.h @@ -49,6 +49,10 @@ extern const FormatUrlType kFormatUrlOmitUsernamePassword; // If the scheme is 'http://', it's removed. extern const FormatUrlType kFormatUrlOmitHTTP; +// Omits the path if it is just a slash and there is no query or ref. This is +// meaningful for non-file "standard" URLs. +extern const FormatUrlType kFormatUrlOmitTrailingSlashOnBareHostname; + // Convenience for omitting all unecessary types. extern const FormatUrlType kFormatUrlOmitAll; @@ -293,13 +297,19 @@ std::wstring FormatUrl(const GURL& url, size_t* prefix_end, size_t* offset_for_adjustment); -// This is a convenience for FormatUrl with -// format_types=kFormatUrlOmitUsernamePassword and unescape=SPACES. +// This is a convenience function for FormatUrl() with +// format_types = kFormatUrlOmitAll and unescape = SPACES. This is the typical +// set of flags for "URLs to display to the user". You should be cautious about +// using this for URLs which will be parsed or sent to other applications. inline std::wstring FormatUrl(const GURL& url, const std::wstring& languages) { - return FormatUrl(url, languages, kFormatUrlOmitUsernamePassword, - UnescapeRule::SPACES, NULL, NULL, NULL); + return FormatUrl(url, languages, kFormatUrlOmitAll, UnescapeRule::SPACES, + NULL, NULL, NULL); } +// Returns whether FormatUrl() would strip a trailing slash from |url|, given a +// format flag including kFormatUrlOmitTrailingSlashOnBareHostname. +bool CanStripTrailingSlash(const GURL& url); + // Strip the portions of |url| that aren't core to the network request. // - user name / password // - reference section diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc index 3e1661d..44cdd06 100644 --- a/net/base/net_util_unittest.cc +++ b/net/base/net_util_unittest.cc @@ -1400,6 +1400,30 @@ TEST(NetUtilTest, FormatUrl) { "http://ftp.google.com/", L"en", net::kFormatUrlOmitHTTP, UnescapeRule::NORMAL, L"http://ftp.google.com/", 7}, + + // -------- omit trailing lash on bare hostname -------- + {"omit slash when it's the entire path", + "http://www.google.com/", L"en", + net::kFormatUrlOmitTrailingSlashOnBareHostname, UnescapeRule::NORMAL, + L"http://www.google.com", 7}, + {"omit slash when there's a ref", + "http://www.google.com/#ref", L"en", + net::kFormatUrlOmitTrailingSlashOnBareHostname, UnescapeRule::NORMAL, + L"http://www.google.com/#ref", 7}, + {"omit slash when there's a query", + "http://www.google.com/?", L"en", + net::kFormatUrlOmitTrailingSlashOnBareHostname, UnescapeRule::NORMAL, + L"http://www.google.com/?", 7}, + {"omit slash when it's not the entire path", + "http://www.google.com/foo", L"en", + net::kFormatUrlOmitTrailingSlashOnBareHostname, UnescapeRule::NORMAL, + L"http://www.google.com/foo", 7}, + {"omit slash for nonstandard URLs", + "data:/", L"en", net::kFormatUrlOmitTrailingSlashOnBareHostname, + UnescapeRule::NORMAL, L"data:/", 5}, + {"omit slash for file URLs", + "file:///", L"en", net::kFormatUrlOmitTrailingSlashOnBareHostname, + UnescapeRule::NORMAL, L"file:///", 7}, }; for (size_t i = 0; i < arraysize(tests); ++i) { |