diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-22 23:06:12 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-22 23:06:12 +0000 |
commit | de29433580baeecc204edc9ca0c2aa47c51aa93a (patch) | |
tree | ec55f655f63f7e09fb373c1e7d784c2e721c6119 | |
parent | 2c1639589b5932b565c9d420cb79b56a4212a706 (diff) | |
download | chromium_src-de29433580baeecc204edc9ca0c2aa47c51aa93a.zip chromium_src-de29433580baeecc204edc9ca0c2aa47c51aa93a.tar.gz chromium_src-de29433580baeecc204edc9ca0c2aa47c51aa93a.tar.bz2 |
Do some cleanup of file path name handling.
This started trying to cleanup DownloadManager::GenerateFilename which asserts
if your system locale isn't UTF-8 (I ran into this when mine got messed up).
The solution is to have GetSuggestedFilename return a FilePath rather than
calling FromWStringHack.
The rest of the patch is a result of trying to write GetSuggestedFilename in a
reasonable way. I changed ReplaceIllegalCharacters to work on a
FilePath::StringType.
Some places in the code calling these functions got cleaner, some got messier.
I think overall the ones that got messier are the ones doing sketchy things
with paths and the ones that got cleaner are the ones doing things more
properly.
The only code here that gets called a nontrivial number of times is the
weburlloader, and I think the new code does about the same number of string
conversions overall (though on certain platforms the number will be higher or
lower).
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/271056
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29832 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | app/os_exchange_data_provider_win.cc | 6 | ||||
-rw-r--r-- | base/i18n/file_util_icu.cc | 67 | ||||
-rw-r--r-- | base/i18n/file_util_icu.h | 8 | ||||
-rw-r--r-- | base/i18n/file_util_icu_unittest.cc | 46 | ||||
-rw-r--r-- | chrome/browser/bookmarks/bookmark_utils.cc | 2 | ||||
-rw-r--r-- | chrome/browser/cocoa/web_drag_source.mm | 3 | ||||
-rw-r--r-- | chrome/browser/download/download_manager.cc | 9 | ||||
-rw-r--r-- | chrome/browser/download/save_package.cc | 16 | ||||
-rw-r--r-- | chrome/browser/shell_integration_linux.cc | 10 | ||||
-rw-r--r-- | chrome/browser/views/new_profile_dialog.cc | 14 | ||||
-rw-r--r-- | chrome/browser/views/tab_contents/tab_contents_view_win.cc | 10 | ||||
-rw-r--r-- | net/base/net_util.cc | 72 | ||||
-rw-r--r-- | net/base/net_util.h | 25 | ||||
-rw-r--r-- | net/base/net_util_unittest.cc | 67 | ||||
-rw-r--r-- | printing/printed_document.cc | 8 | ||||
-rw-r--r-- | printing/printing_context_win.cc | 2 | ||||
-rw-r--r-- | webkit/glue/weburlloader_impl.cc | 9 |
17 files changed, 223 insertions, 151 deletions
diff --git a/app/os_exchange_data_provider_win.cc b/app/os_exchange_data_provider_win.cc index 997cdee..81db40e 100644 --- a/app/os_exchange_data_provider_win.cc +++ b/app/os_exchange_data_provider_win.cc @@ -703,15 +703,15 @@ static void CreateValidFileNameFromTitle(const GURL& url, std::wstring* validated) { if (title.empty()) { if (url.is_valid()) { - *validated = net::GetSuggestedFilename( - url, std::string(), std::string(), std::wstring()); + *validated = net::GetSuggestedFilename(url, std::string(), + std::string(), "").ToWStringHack(); } else { // Nothing else can be done, just use a default. *validated = l10n_util::GetString(IDS_APP_UNTITLED_SHORTCUT_FILE_NAME); } } else { *validated = title; - file_util::ReplaceIllegalCharacters(validated, '-'); + file_util::ReplaceIllegalCharactersInPath(validated, '-'); } static const wchar_t extension[] = L".url"; static const size_t max_length = MAX_PATH - arraysize(extension); diff --git a/base/i18n/file_util_icu.cc b/base/i18n/file_util_icu.cc index 0bc9db6..4d33e3a 100644 --- a/base/i18n/file_util_icu.cc +++ b/base/i18n/file_util_icu.cc @@ -124,50 +124,47 @@ bool IsFilenameLegal(const string16& file_name) { return Singleton<IllegalCharacters>()->containsNone(file_name); } -void ReplaceIllegalCharacters(std::wstring* file_name, int replace_char) { +void ReplaceIllegalCharactersInPath(FilePath::StringType* file_name, + char replace_char) { DCHECK(file_name); - DCHECK(!(Singleton<IllegalCharacters>()->contains(replace_char)) && - replace_char < 0x10000); + DCHECK(!(Singleton<IllegalCharacters>()->contains(replace_char))); // Remove leading and trailing whitespace. TrimWhitespace(*file_name, TRIM_ALL, file_name); - if (IsFilenameLegal(WideToUTF16(*file_name))) - return; + IllegalCharacters* illegal = Singleton<IllegalCharacters>::get(); + int cursor = 0; // The ICU macros expect an int. + while (cursor < static_cast<int>(file_name->size())) { + int char_begin = cursor; + uint32 code_point; +#if defined(OS_MACOSX) + // Mac uses UTF-8 encoding for filenames. + U8_NEXT(file_name->data(), cursor, static_cast<int>(file_name->length()), + code_point); +#elif defined(OS_WIN) + // Windows uses UTF-16 encoding for filenames. + U16_NEXT(file_name->data(), cursor, static_cast<int>(file_name->length()), + code_point); +#elif defined(OS_LINUX) + // Linux doesn't actually define an encoding. It basically allows anything + // except for a few special ASCII characters. + unsigned char cur_char = static_cast<unsigned char>((*file_name)[cursor++]); + if (cur_char >= 0x80) + continue; + code_point = cur_char; +#else + NOTREACHED(); +#endif - std::wstring::size_type i = 0; - std::wstring::size_type length = file_name->size(); - const wchar_t* wstr = file_name->data(); -#if defined(WCHAR_T_IS_UTF16) - // Using |span| method of UnicodeSet might speed things up a bit, but - // it's not likely to matter here. - std::wstring temp; - temp.reserve(length); - while (i < length) { - UChar32 ucs4; - std::wstring::size_type prev = i; - U16_NEXT(wstr, i, length, ucs4); - if (Singleton<IllegalCharacters>()->contains(ucs4)) { - temp.push_back(replace_char); - } else if (ucs4 < 0x10000) { - temp.push_back(ucs4); - } else { - temp.push_back(wstr[prev]); - temp.push_back(wstr[prev + 1]); + if (illegal->contains(code_point)) { + file_name->replace(char_begin, cursor - char_begin, 1, replace_char); + // We just made the potentially multi-byte/word char into one that only + // takes one byte/word, so need to adjust the cursor to point to the next + // character again. + cursor = char_begin + 1; } } - file_name->swap(temp); -#elif defined(WCHAR_T_IS_UTF32) - while (i < length) { - if (Singleton<IllegalCharacters>()->contains(wstr[i])) { - (*file_name)[i] = replace_char; - } - ++i; - } -#else -#error wchar_t* should be either UTF-16 or UTF-32 -#endif } bool LocaleAwareCompareFilenames(const FilePath& a, const FilePath& b) { diff --git a/base/i18n/file_util_icu.h b/base/i18n/file_util_icu.h index c309a9e..54ddb08 100644 --- a/base/i18n/file_util_icu.h +++ b/base/i18n/file_util_icu.h @@ -6,6 +6,7 @@ #include <string> +#include "base/file_path.h" #include "base/string16.h" class FilePath; @@ -18,12 +19,13 @@ bool IsFilenameLegal(const string16& file_name); // Replaces characters in 'file_name' that are illegal for file names with // 'replace_char'. 'file_name' must not be a full or relative path, but just the -// file name component. Any leading or trailing whitespace in 'file_name' is -// removed. +// file name component (since slashes are considered illegal). Any leading or +// trailing whitespace in 'file_name' is removed. // Example: // file_name == "bad:file*name?.txt", changed to: "bad-file-name-.txt" when // 'replace_char' is '-'. -void ReplaceIllegalCharacters(std::wstring* file_name, int replace_char); +void ReplaceIllegalCharactersInPath(FilePath::StringType* file_name, + char replace_char); // Compares two filenames using the current locale information. This can be // used to sort directory listings. It behaves like "operator<" for use in diff --git a/base/i18n/file_util_icu_unittest.cc b/base/i18n/file_util_icu_unittest.cc index aebcd0df..b46fe55 100644 --- a/base/i18n/file_util_icu_unittest.cc +++ b/base/i18n/file_util_icu_unittest.cc @@ -6,6 +6,7 @@ #include "base/file_util.h" #include "base/path_service.h" +#include "base/utf_string_conversions.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -34,9 +35,34 @@ class FileUtilICUTest : public PlatformTest { FilePath test_dir_; }; +#if defined(OS_LINUX) + +// Linux disallows some evil ASCII characters, but passes all non-ASCII. +static const struct goodbad_pair { + const char* bad_name; + const char* good_name; +} kIllegalCharacterCases[] = { + {"bad*file:name?.jpg", "bad-file-name-.jpg"}, + {"**********::::.txt", "--------------.txt"}, + {"\xe9\xf0zzzz.\xff", "\xe9\xf0zzzz.\xff"}, +}; + +TEST_F(FileUtilICUTest, ReplaceIllegalCharacersInPathLinuxTest) { + for (size_t i = 0; i < arraysize(kIllegalCharacterCases); ++i) { + std::string bad_name(kIllegalCharacterCases[i].bad_name); + file_util::ReplaceIllegalCharactersInPath(&bad_name, '-'); + EXPECT_EQ(kIllegalCharacterCases[i].good_name, bad_name); + } +} + +#else + +// For Mac & Windows, which both do Unicode validation on filenames. These +// characters are given as wide strings since its more convenient to specify +// unicode characters. For Mac they should be converted to UTF-8. static const struct goodbad_pair { - std::wstring bad_name; - std::wstring good_name; + const wchar_t* bad_name; + const wchar_t* good_name; } kIllegalCharacterCases[] = { {L"bad*file:name?.jpg", L"bad-file-name-.jpg"}, {L"**********::::.txt", L"--------------.txt"}, @@ -46,7 +72,7 @@ static const struct goodbad_pair { #if defined(OS_WIN) {L"bad*file\\name.jpg", L"bad-file-name.jpg"}, {L"\t bad*file\\name/.jpg ", L"bad-file-name-.jpg"}, -#elif defined(OS_POSIX) +#elif defined(OS_MACOSX) {L"bad*file?name.jpg", L"bad-file-name.jpg"}, {L"\t bad*file?name/.jpg ", L"bad-file-name-.jpg"}, #endif @@ -61,11 +87,19 @@ static const struct goodbad_pair { {L"bad\uFDD0file\uFDEFname.jpg ", L"bad-file-name.jpg"}, }; -TEST_F(FileUtilICUTest, ReplaceIllegalCharactersTest) { - for (unsigned int i = 0; i < arraysize(kIllegalCharacterCases); ++i) { +TEST_F(FileUtilICUTest, ReplaceIllegalCharactersInPathTest) { + for (size_t i = 0; i < arraysize(kIllegalCharacterCases); ++i) { +#if defined(OS_WIN) std::wstring bad_name(kIllegalCharacterCases[i].bad_name); - file_util::ReplaceIllegalCharacters(&bad_name, L'-'); + file_util::ReplaceIllegalCharactersInPath(&bad_name, '-'); EXPECT_EQ(kIllegalCharacterCases[i].good_name, bad_name); +#elif defined(OS_MACOSX) + std::string bad_name(WideToUTF8(kIllegalCharacterCases[i].bad_name)); + file_util::ReplaceIllegalCharactersInPath(&bad_name, '-'); + EXPECT_EQ(WideToUTF8(kIllegalCharacterCases[i].good_name), bad_name); +#endif } } +#endif + diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc index b11d0b9..3af7910 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -403,7 +403,7 @@ bool CanPasteFromClipboard(const BookmarkNode* node) { std::string GetNameForURL(const GURL& url) { if (url.is_valid()) { return WideToUTF8(net::GetSuggestedFilename( - url, std::string(), std::string(), std::wstring())); + url, std::string(), std::string(), "").ToWStringHack()); } else { return l10n_util::GetStringUTF8(IDS_APP_UNTITLED_SHORTCUT_FILE_NAME); } diff --git a/chrome/browser/cocoa/web_drag_source.mm b/chrome/browser/cocoa/web_drag_source.mm index 1d82218..3790e0c 100644 --- a/chrome/browser/cocoa/web_drag_source.mm +++ b/chrome/browser/cocoa/web_drag_source.mm @@ -52,8 +52,7 @@ FilePath GetFileNameFromDragData( if (file_name.empty()) { // Retrieve the name from the URL. - file_name = FilePath::FromWStringHack( - net::GetSuggestedFilename(drop_data.url, "", "", L"")); + file_name = net::GetSuggestedFilename(drop_data.url, "", "", ""); } file_name = file_name.ReplaceExtension([SysUTF16ToNSString( diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc index c83056f..fb68114 100644 --- a/chrome/browser/download/download_manager.cc +++ b/chrome/browser/download/download_manager.cc @@ -1212,11 +1212,10 @@ void DownloadManager::GenerateExtension( void DownloadManager::GenerateFilename(DownloadCreateInfo* info, FilePath* generated_name) { - *generated_name = FilePath::FromWStringHack( - net::GetSuggestedFilename(GURL(info->url), - info->content_disposition, - info->referrer_charset, - L"download")); + *generated_name = net::GetSuggestedFilename(GURL(info->url), + info->content_disposition, + info->referrer_charset, + "download"); DCHECK(!generated_name->empty()); GenerateSafeFilename(info->mime_type, generated_name); diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc index ac4a39e..97487c0 100644 --- a/chrome/browser/download/save_package.cc +++ b/chrome/browser/download/save_package.cc @@ -65,7 +65,7 @@ namespace { // Default name which will be used when we can not get proper name from // resource URL. -const wchar_t kDefaultSaveName[] = L"saved_resource"; +const char kDefaultSaveName[] = "saved_resource"; const FilePath::CharType kDefaultHtmlExtension[] = #if defined(OS_WIN) @@ -336,8 +336,8 @@ bool SavePackage::GenerateFilename(const std::string& disposition, FilePath::StringType* generated_name) { // TODO(jungshik): Figure out the referrer charset when having one // makes sense and pass it to GetSuggestedFilename. - FilePath file_path = FilePath::FromWStringHack( - net::GetSuggestedFilename(url, disposition, "", kDefaultSaveName)); + FilePath file_path = net::GetSuggestedFilename(url, disposition, "", + kDefaultSaveName); DCHECK(!file_path.empty()); FilePath::StringType pure_file_name = @@ -1033,13 +1033,9 @@ FilePath SavePackage::GetSuggestedNameForSaveAs(const FilePath& name, if (can_save_as_complete) name_with_proper_ext = EnsureHtmlExtension(name_with_proper_ext); - std::wstring file_name = name_with_proper_ext.ToWStringHack(); - // TODO(port): we need a version of ReplaceIllegalCharacters() that takes - // FilePaths. - file_util::ReplaceIllegalCharacters(&file_name, L' '); - TrimWhitespace(file_name, TRIM_ALL, &file_name); - - return FilePath::FromWStringHack(file_name); + FilePath::StringType file_name = name_with_proper_ext.value(); + file_util::ReplaceIllegalCharactersInPath(&file_name, ' '); + return FilePath(file_name); } FilePath SavePackage::EnsureHtmlExtension(const FilePath& name) { diff --git a/chrome/browser/shell_integration_linux.cc b/chrome/browser/shell_integration_linux.cc index 4d6601a..3766249 100644 --- a/chrome/browser/shell_integration_linux.cc +++ b/chrome/browser/shell_integration_linux.cc @@ -303,17 +303,15 @@ bool ShellIntegration::IsFirefoxDefaultBrowser() { FilePath ShellIntegration::GetDesktopShortcutFilename(const GURL& url) { // Use a prefix, because xdg-desktop-menu requires it. - std::wstring filename_wide = - std::wstring(chrome::kBrowserProcessExecutableName) + L"-" + - UTF8ToWide(url.spec()); - file_util::ReplaceIllegalCharacters(&filename_wide, '_'); + std::string filename = + WideToUTF8(chrome::kBrowserProcessExecutableName) + "-" + url.spec(); + file_util::ReplaceIllegalCharactersInPath(&filename, '_'); FilePath desktop_path; if (!PathService::Get(chrome::DIR_USER_DESKTOP, &desktop_path)) return FilePath(); - FilePath filepath = desktop_path.Append( - FilePath::FromWStringHack(filename_wide)); + FilePath filepath = desktop_path.Append(filename); FilePath alternative_filepath(filepath.value() + ".desktop"); for (size_t i = 1; i < 100; ++i) { if (file_util::PathExists(FilePath(alternative_filepath))) { diff --git a/chrome/browser/views/new_profile_dialog.cc b/chrome/browser/views/new_profile_dialog.cc index f0cfded..54ac154 100644 --- a/chrome/browser/views/new_profile_dialog.cc +++ b/chrome/browser/views/new_profile_dialog.cc @@ -65,12 +65,14 @@ bool NewProfileDialog::IsDialogButtonEnabled( MessageBoxFlags::DialogButton button) const { if (button == MessageBoxFlags::DIALOGBUTTON_OK) { std::wstring profile_name = message_box_view_->GetInputText(); - // TODO(munjal): Refactor the function ReplaceIllegalCharacters in - // file_util to something that just checks if there are illegal chars - // since that's what we really need. Also, replaceIllegalChars seems to - // be expensive since it builds a list of illegal characters for each call. - // So at the least fix that. - file_util::ReplaceIllegalCharacters(&profile_name, L'_'); + +#if defined(OS_POSIX) + std::string profile_name_narrow = WideToUTF8(profile_name); + file_util::ReplaceIllegalCharactersInPath(&profile_name_narrow, '_'); + profile_name = UTF8ToWide(profile_name_narrow); +#else + file_util::ReplaceIllegalCharactersInPath(&profile_name, '_'); +#endif return !profile_name.empty() && profile_name == message_box_view_->GetInputText(); } diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.cc b/chrome/browser/views/tab_contents/tab_contents_view_win.cc index 227425f..e1aa377 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.cc @@ -138,10 +138,12 @@ void TabContentsViewWin::StartDragging(const WebDropData& drop_data, file_name = file_name.BaseName().RemoveExtension(); if (file_name.value().empty()) { // Retrieve the name from the URL. - std::wstring fn = net::GetSuggestedFilename(drop_data.url, "", "", L""); - if ((fn.size() + drop_data.file_extension.size() + 1) > MAX_PATH) - fn = fn.substr(0, MAX_PATH - drop_data.file_extension.size() - 2); - file_name = FilePath::FromWStringHack(fn); + file_name = net::GetSuggestedFilename(drop_data.url, "", "", ""); + if (file_name.value().size() + drop_data.file_extension.size() + 1 > + MAX_PATH) { + file_name = FilePath(file_name.value().substr( + 0, MAX_PATH - drop_data.file_extension.size() - 2)); + } } file_name = file_name.ReplaceExtension(drop_data.file_extension); data.SetFileContents(file_name.value(), drop_data.file_contents); diff --git a/net/base/net_util.cc b/net/base/net_util.cc index f75184f..49ded14 100644 --- a/net/base/net_util.cc +++ b/net/base/net_util.cc @@ -749,6 +749,18 @@ std::wstring FormatViewSourceUrl(const GURL& url, return result; } +// Converts a UTF-8 string to a FilePath string type. +// +// This is inline with the hope that the function will be "free" on non-Windows +// platforms. +inline FilePath::StringType UTF8ToFilePathString(const std::string& utf8) { +#if defined(OS_WIN) + return FilePath::StringType(UTF8ToUTF16(utf8)); +#else + return utf8; +#endif +} + } // namespace namespace net { @@ -805,19 +817,19 @@ std::string GetSpecificHeader(const std::string& headers, return GetSpecificHeaderT(headers, name); } -std::wstring GetFileNameFromCD(const std::string& header, - const std::string& referrer_charset) { +std::string GetFileNameFromCD(const std::string& header, + const std::string& referrer_charset) { std::string param_value = GetHeaderParamValue(header, "filename"); if (param_value.empty()) { // Some servers use 'name' parameter. param_value = GetHeaderParamValue(header, "name"); } if (param_value.empty()) - return std::wstring(); + return std::string(); std::string decoded; if (DecodeParamValue(param_value, referrer_charset, &decoded)) - return UTF8ToWide(decoded); - return std::wstring(); + return decoded; + return std::string(); } std::wstring GetHeaderParamValue(const std::wstring& field, @@ -1032,60 +1044,72 @@ std::wstring StripWWW(const std::wstring& text) { text.substr(www.length()) : text; } -std::wstring GetSuggestedFilename(const GURL& url, - const std::string& content_disposition, - const std::string& referrer_charset, - const std::wstring& default_name) { +FilePath GetSuggestedFilename(const GURL& url, + const std::string& content_disposition, + const std::string& referrer_charset, + const char* default_name) { // TODO(rolandsteiner): as pointed out by darin in the code review, this is // hardly ideal. "download" should be translated, or another solution found. // (cf. http://code.google.com/p/chromium/issues/detail?id=25289) - const wchar_t kFinalFallbackName[] = L"download"; + const char kFinalFallbackName[] = "download"; // about: and data: URLs don't have file names, but esp. data: URLs may // contain parts that look like ones (i.e., contain a slash). // Therefore we don't attempt to divine a file name out of them. - if (url.SchemeIs("about") || url.SchemeIs("data")) - return default_name.empty() ? std::wstring(kFinalFallbackName) - : default_name; + if (url.SchemeIs("about") || url.SchemeIs("data")) { + return FilePath(UTF8ToFilePathString( + default_name && default_name[0] ? default_name : kFinalFallbackName)); + } - std::wstring filename = GetFileNameFromCD(content_disposition, - referrer_charset); + std::string filename = GetFileNameFromCD(content_disposition, + referrer_charset); if (!filename.empty()) { // Remove any path information the server may have sent, take the name // only. - filename = file_util::GetFilenameFromPath(filename); +#if defined(OS_WIN) + filename = UTF16ToUTF8(FilePath(UTF8ToUTF16(filename)).BaseName().value()); +#else + filename = FilePath(filename).BaseName().value(); +#endif + // Next, remove "." from the beginning and end of the file name to avoid // tricks with hidden files, "..", and "." - TrimString(filename, L".", &filename); + TrimString(filename, ".", &filename); } if (filename.empty()) { if (url.is_valid()) { - filename = UnescapeAndDecodeUTF8URLComponent( + filename = UnescapeURLComponent( url.ExtractFileName(), UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS); } } // Trim '.' once more. - TrimString(filename, L".", &filename); + TrimString(filename, ".", &filename); + // If there's no filename or it gets trimed to be empty, use // the URL hostname or default_name if (filename.empty()) { - if (!default_name.empty()) { + if (default_name && default_name[0]) { filename = default_name; } else if (url.is_valid()) { // Some schemes (e.g. file) do not have a hostname. Even though it's // not likely to reach here, let's hardcode the last fallback name. // TODO(jungshik) : Decode a 'punycoded' IDN hostname. (bug 1264451) - filename = url.host().empty() ? std::wstring(kFinalFallbackName) - : UTF8ToWide(url.host()); + filename = url.host().empty() ? std::string(kFinalFallbackName) + : url.host(); } else { NOTREACHED(); } } - file_util::ReplaceIllegalCharacters(&filename, '-'); - return filename; +#if defined(OS_WIN) + FilePath::StringType file_path_string = UTF8ToWide(filename); +#else + std::string& file_path_string = filename; +#endif + file_util::ReplaceIllegalCharactersInPath(&file_path_string, '-'); + return FilePath(file_path_string); } bool IsPortAllowedByDefault(int port) { diff --git a/net/base/net_util.h b/net/base/net_util.h index 302a55f..1ad4ac2 100644 --- a/net/base/net_util.h +++ b/net/base/net_util.h @@ -1,9 +1,9 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. -#ifndef NET_BASE_NET_UTIL_H__ -#define NET_BASE_NET_UTIL_H__ +#ifndef NET_BASE_NET_UTIL_H_ +#define NET_BASE_NET_UTIL_H_ #include "build/build_config.h" @@ -126,8 +126,8 @@ std::string GetHeaderParamValue(const std::string& field, // other caller is a unit test. Need to figure out expose this function only to // net_util_unittest. // -std::wstring GetFileNameFromCD(const std::string& header, - const std::string& referrer_charset); +std::string GetFileNameFromCD(const std::string& header, + const std::string& referrer_charset); // Converts the given host name to unicode characters, APPENDING them to the // the given output string. This can be called for any host name, if the @@ -194,14 +194,17 @@ std::wstring StripWWW(const std::wstring& text); // Gets the filename from the raw Content-Disposition header (as read from the // network). Otherwise uses the last path component name or hostname from -// |url|. Note: it's possible for the suggested filename to be empty (e.g., +// |url|. If there is no filename or it can't be used, the given default name +// will be used if specified. + +// Note: it's possible for the suggested filename to be empty (e.g., // file:///). referrer_charset is used as one of charsets // to interpret a raw 8bit string in C-D header (after interpreting // as UTF-8 fails). See the comment for GetFilenameFromCD for more details. -std::wstring GetSuggestedFilename(const GURL& url, - const std::string& content_disposition, - const std::string& referrer_charset, - const std::wstring& default_name); +FilePath GetSuggestedFilename(const GURL& url, + const std::string& content_disposition, + const std::string& referrer_charset, + const char* default_name); // Checks the given port against a list of ports which are restricted by // default. Returns true if the port is allowed, false if it is restricted. @@ -259,4 +262,4 @@ void SetExplicitlyAllowedPorts(const std::wstring& allowed_ports); } // namespace net -#endif // NET_BASE_NET_UTIL_H__ +#endif // NET_BASE_NET_UTIL_H_ diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc index eddbd16..f8faedf 100644 --- a/net/base/net_util_unittest.cc +++ b/net/base/net_util_unittest.cc @@ -353,7 +353,7 @@ struct SuggestedFilenameCase { const char* url; const char* content_disp_header; const char* referrer_charset; - const wchar_t* default_filename; + const char* default_filename; const wchar_t* expected_filename; }; @@ -770,8 +770,8 @@ TEST(NetUtilTest, GetFileNameFromCD) { }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { EXPECT_EQ(tests[i].expected, - net::GetFileNameFromCD(tests[i].header_field, - tests[i].referrer_charset)); + UTF8ToWide(net::GetFileNameFromCD(tests[i].header_field, + tests[i].referrer_charset))); } } @@ -853,32 +853,32 @@ TEST(NetUtilTest, GetSuggestedFilename) { {"http://www.google.com/", "Content-disposition: attachment; filename=test.html", "", - L"", + "", L"test.html"}, {"http://www.google.com/", "Content-disposition: attachment; filename=\"test.html\"", "", - L"", + "", L"test.html"}, {"http://www.google.com/path/test.html", "Content-disposition: attachment", "", - L"", + "", L"test.html"}, {"http://www.google.com/path/test.html", "Content-disposition: attachment;", "", - L"", + "", L"test.html"}, {"http://www.google.com/", "", "", - L"", + "", L"www.google.com"}, {"http://www.google.com/test.html", "", "", - L"", + "", L"test.html"}, // Now that we use googleurl's ExtractFileName, this case falls back // to the hostname. If this behavior is not desirable, we'd better @@ -886,114 +886,119 @@ TEST(NetUtilTest, GetSuggestedFilename) { {"http://www.google.com/path/", "", "", - L"", + "", L"www.google.com"}, {"http://www.google.com/path", "", "", - L"", + "", L"path"}, {"file:///", "", "", - L"", + "", L"download"}, {"non-standard-scheme:", "", "", - L"", + "", L"download"}, {"http://www.google.com/", "Content-disposition: attachment; filename =\"test.html\"", "", - L"download", + "download", L"test.html"}, {"http://www.google.com/", "", "", - L"download", + "download", L"download"}, {"http://www.google.com/", "Content-disposition: attachment; filename=\"../test.html\"", "", - L"", + "", L"test.html"}, {"http://www.google.com/", "Content-disposition: attachment; filename=\"..\"", "", - L"download", + "download", L"download"}, {"http://www.google.com/test.html", "Content-disposition: attachment; filename=\"..\"", "", - L"download", + "download", L"test.html"}, // Below is a small subset of cases taken from GetFileNameFromCD test above. {"http://www.google.com/", "Content-Disposition: attachment; filename=\"%EC%98%88%EC%88%A0%20" "%EC%98%88%EC%88%A0.jpg\"", "", - L"", + "", L"\uc608\uc220 \uc608\uc220.jpg"}, {"http://www.google.com/%EC%98%88%EC%88%A0%20%EC%98%88%EC%88%A0.jpg", "", "", - L"download", + "download", L"\uc608\uc220 \uc608\uc220.jpg"}, {"http://www.google.com/", "Content-disposition: attachment;", "", - L"\uB2E4\uC6B4\uB85C\uB4DC", + "\xEB\x8B\xA4\xEC\x9A\xB4\xEB\xA1\x9C\xEB\x93\x9C", L"\uB2E4\uC6B4\uB85C\uB4DC"}, {"http://www.google.com/", "Content-Disposition: attachment; filename=\"=?EUC-JP?Q?=B7=DD=BD=" "D13=2Epng?=\"", "", - L"download", + "download", L"\u82b8\u88533.png"}, {"http://www.example.com/images?id=3", "Content-Disposition: attachment; filename=caf\xc3\xa9.png", "iso-8859-1", - L"", + "", L"caf\u00e9.png"}, {"http://www.example.com/images?id=3", "Content-Disposition: attachment; filename=caf\xe5.png", "windows-1253", - L"", + "", L"caf\u03b5.png"}, {"http://www.example.com/file?id=3", "Content-Disposition: attachment; name=\xcf\xc2\xd4\xd8.zip", "GBK", - L"", + "", L"\u4e0b\u8f7d.zip"}, // Invalid C-D header. Extracts filename from url. {"http://www.google.com/test.html", "Content-Disposition: attachment; filename==?iiso88591?Q?caf=EG?=", "", - L"", + "", L"test.html"}, // about: and data: URLs {"about:chrome", "", "", - L"", + "", L"download"}, {"data:,looks/like/a.path", "", "", - L"", + "", L"download"}, {"data:text/plain;base64,VG8gYmUgb3Igbm90IHRvIGJlLg=", "", "", - L"", + "", L"download"}, }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) { - std::wstring filename = net::GetSuggestedFilename( + FilePath filename = net::GetSuggestedFilename( GURL(test_cases[i].url), test_cases[i].content_disp_header, test_cases[i].referrer_charset, test_cases[i].default_filename); - EXPECT_EQ(std::wstring(test_cases[i].expected_filename), filename); +#if defined(OS_WIN) + EXPECT_EQ(std::wstring(test_cases[i].expected_filename), filename.value()) +#else + EXPECT_EQ(WideToUTF8(test_cases[i].expected_filename), filename.value()) +#endif + << "Iteration " << i << ": " << test_cases[i].url; } } diff --git a/printing/printed_document.cc b/printing/printed_document.cc index aa9ab5b..785b846 100644 --- a/printing/printed_document.cc +++ b/printing/printed_document.cc @@ -263,7 +263,13 @@ void PrintedDocument::DebugDump(const PrintedPage& page) { filename += L"_"; filename += StringPrintf(L"%02d", page.page_number()); filename += L"_.emf"; - file_util::ReplaceIllegalCharacters(&filename, '_'); +#if defined(OS_WIN) + file_util::ReplaceIllegalCharactersInPath(&filename, '_'); +#else + std::string narrow_filename = WideToUTF8(filename); + file_util::ReplaceIllegalCharactersInPath(&narrow_filename, '_'); + filename = UTF8ToWide(narrow_filename); +#endif std::wstring path(g_debug_dump_info->debug_dump_path); file_util::AppendToPath(&path, filename); #if defined(OS_WIN) diff --git a/printing/printing_context_win.cc b/printing/printing_context_win.cc index 99232a9..6c49da2 100644 --- a/printing/printing_context_win.cc +++ b/printing/printing_context_win.cc @@ -284,7 +284,7 @@ PrintingContext::Result PrintingContext::NewDocument( filename += document_name; filename += L"_"; filename += L"buffer.prn"; - file_util::ReplaceIllegalCharacters(&filename, '_'); + file_util::ReplaceIllegalCharactersInPath(&filename, '_'); file_util::AppendToPath(&debug_dump_path, filename); di.lpszOutput = debug_dump_path.c_str(); } diff --git a/webkit/glue/weburlloader_impl.cc b/webkit/glue/weburlloader_impl.cc index 8884bdc..510be0e 100644 --- a/webkit/glue/weburlloader_impl.cc +++ b/webkit/glue/weburlloader_impl.cc @@ -167,8 +167,13 @@ void PopulateURLResponse( // pass it to GetSuggestedFilename. std::string value; if (headers->EnumerateHeader(NULL, "content-disposition", &value)) { - response->setSuggestedFileName(WideToUTF16Hack( - net::GetSuggestedFilename(url, value, "", std::wstring()))); +#if defined(OS_WIN) + response->setSuggestedFileName( + net::GetSuggestedFilename(url, value, "", "").value()); +#else + response->setSuggestedFileName(UTF8ToUTF16( + net::GetSuggestedFilename(url, value, "", "").value())); +#endif } Time time_val; |