Fixes bug in file_util::ReplaceExtension that could chop off path

elements. This was causing some folks bookmarks not to get saved!

BUG=1946
TEST=covered by unit tests

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

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

2 files changed, 39 insertions, 5 deletions

diff --git a/base/file_util.cc b/base/file_util.cc
index 07fb71c..0d6fab1 100644
--- a/base/file_util.cc
+++ b/base/file_util.cc
@@ -194,14 +194,36 @@ void ReplaceIllegalCharacters(std::wstring* file_name, int replace_char) {
 #endif
 }
 
+// Appends the extension to file adding a '.' if extension doesn't contain one.
+// This does nothing if extension is empty or '.'. This is used internally by
+// ReplaceExtension.
+static void AppendExtension(const std::wstring& extension,
+                            std::wstring* file) {
+  if (!extension.empty() && extension != L".") {
+    if (extension[0] != L'.')
+      file->append(L".");
+    file->append(extension);
+  }
+}
+
 void ReplaceExtension(std::wstring* file_name, const std::wstring& extension) {
   const std::wstring::size_type last_dot = file_name->rfind(L'.');
-  std::wstring result = file_name->substr(0, last_dot);
-  if (!extension.empty() && extension != L".") {
-    if (extension.at(0) != L'.')
-      result.append(L".");
-    result.append(extension);
+  if (last_dot == std::wstring::npos) {
+    // No extension, just append the supplied extension.
+    AppendExtension(extension, file_name);
+    return;
   }
+  const std::wstring::size_type last_separator =
+      file_name->rfind(kPathSeparator);
+  if (last_separator != std::wstring::npos && last_dot < last_separator) {
+    // File name doesn't have extension, but one of the directories does; don't
+    // replace it, just append the supplied extension. For example
+    // 'c:\tmp.bar\foo'.
+    AppendExtension(extension, file_name);
+    return;
+  }
+  std::wstring result = file_name->substr(0, last_dot);
+  AppendExtension(extension, &result);
   file_name->swap(result);
 }
 
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index e5cea3a..c56579a 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -819,6 +819,18 @@ TEST_F(FileUtilTest, ReplaceExtensionTest) {
   }
 }
 
+// Make sure ReplaceExtension doesn't replace an extension that occurs as one of
+// the directory names of the path.
+TEST_F(FileUtilTest, ReplaceExtensionTestWithPathSeparators) {
+  std::wstring path;
+  file_util::AppendToPath(&path, L"foo.bar");
+  file_util::AppendToPath(&path, L"foo");
+  // '/foo.bar/foo' with extension '.baz'
+  std::wstring result_path = path;
+  file_util::ReplaceExtension(&result_path, L".baz");
+  EXPECT_EQ(path + L".baz", result_path);
+}
+
 TEST_F(FileUtilTest, FileEnumeratorTest) {
   // Test an empty directory.
   file_util::FileEnumerator f0(test_dir_, true,