From 9e3642d5e76af15ecdddf213850833d438fa4716 Mon Sep 17 00:00:00 2001
From: Alex Zolotarev <deathbaba@gmail.com>
Date: Tue, 4 Sep 2012 18:01:29 +0300
Subject: [PATCH] Correct unique bookmarks file name generation

---
 map/bookmark.cpp                 | 32 +++++++++++++++++++++++---------
 map/bookmark.hpp                 |  2 +-
 map/map_tests/bookmarks_test.cpp | 23 ++++++++++++++++++++---
 3 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/map/bookmark.cpp b/map/bookmark.cpp
index c080d0cf9d..0bd89e965d 100644
--- a/map/bookmark.cpp
+++ b/map/bookmark.cpp
@@ -6,6 +6,7 @@
 
 #include "../coding/file_reader.hpp"
 #include "../coding/parse_xml.hpp"  // LoadFromKML
+#include "../base/string_utils.hpp"
 
 #include "../base/stl_add.hpp"
 #include "../base/string_utils.hpp"
@@ -291,18 +292,31 @@ void BookmarkCategory::SaveToKML(ostream & s)
   s << kmlFooter;
 }
 
-string BookmarkCategory::GenerateUniqueFileName(const string & path, string name)
+static bool IsValidCharForPath(strings::UniChar c)
+{
+  static strings::UniChar const illegalChars[] = {':', '/', '\\', '<', '>', '\"', '|', '?', '*'};
+  for (size_t i = 0; i < ARRAY_SIZE(illegalChars); ++i)
+    if (c < ' ' || illegalChars[i] == c)
+      return false;
+  return true;
+}
+
+string BookmarkCategory::GenerateUniqueFileName(const string & path, string const & name)
 {
   // Remove not allowed symbols
-  char const illegalChars[] = ":/";
-  for (size_t i = 0; i < ARRAY_SIZE(illegalChars); ++i)
-    name.erase(std::remove(name.begin(), name.end(), illegalChars[i]), name.end());
-  if (name.empty())
-    name = "Bookmarks";
+  strings::UniString uniName = strings::MakeUniString(name);
+  size_t const charCount = uniName.size();
+  strings::UniString uniFixedName;
+  for (size_t i = 0; i < charCount; ++i)
+    if (IsValidCharForPath(uniName[i]))
+      uniFixedName.push_back(uniName[i]);
+
+  string const fixedName = uniFixedName.empty() ? "Bookmarks" : strings::ToUtf8(uniFixedName);
   size_t counter = 1;
-  while (Platform::IsFileExistsByFullPath(path + name))
-    name.append(strings::to_string(counter++));
-  return path + name + ".kml";
+  string suffix;
+  while (Platform::IsFileExistsByFullPath(path + fixedName + suffix))
+    suffix = strings::to_string(counter++);
+  return path + fixedName + suffix + ".kml";
 }
 
 bool BookmarkCategory::SaveToKMLFileAtPath(string const & path)
diff --git a/map/bookmark.hpp b/map/bookmark.hpp
index 3559aa9b5f..4adfb8b00d 100644
--- a/map/bookmark.hpp
+++ b/map/bookmark.hpp
@@ -66,7 +66,7 @@ public:
   /// @param[in] path directory name where to save
   bool SaveToKMLFileAtPath(string const & path);
 
-  static string GenerateUniqueFileName(const string & path, string name);
+  static string GenerateUniqueFileName(const string & path, string const & name);
 };
 
 /// Non-const category is needed to "edit" bookmark (actually, re-add it)
diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp
index 9fa336eb73..16d2e860a4 100644
--- a/map/map_tests/bookmarks_test.cpp
+++ b/map/map_tests/bookmarks_test.cpp
@@ -209,11 +209,28 @@ UNIT_TEST(Bookmarks_AddressInfo)
 
 UNIT_TEST(Bookmarks_UniqueFileName)
 {
-  char const * FILENAME = "SomeUniqueFileName";
+  string const FILENAME = "SomeUniqueFileName";
   {
     FileWriter file(FILENAME);
-    file.Write(FILENAME, strlen(FILENAME));
+    file.Write(FILENAME.data(), FILENAME.size());
   }
-  TEST_NOT_EQUAL(BookmarkCategory::GenerateUniqueFileName("", FILENAME), FILENAME, ());
+  string gen = BookmarkCategory::GenerateUniqueFileName("", FILENAME);
+  TEST_NOT_EQUAL(gen, FILENAME, ());
+  TEST_EQUAL(gen, FILENAME + "1.kml", ());
+
+  string const FILENAME1 = FILENAME + "1";
+  {
+    FileWriter file(FILENAME1);
+    file.Write(FILENAME1.data(), FILENAME1.size());
+  }
+  gen = BookmarkCategory::GenerateUniqueFileName("", FILENAME);
+  TEST_NOT_EQUAL(gen, FILENAME, ());
+  TEST_NOT_EQUAL(gen, FILENAME1, ());
+  TEST_EQUAL(gen, FILENAME + "2.kml", ());
+
   FileWriter::DeleteFileX(FILENAME);
+  FileWriter::DeleteFileX(FILENAME1);
+
+  gen = BookmarkCategory::GenerateUniqueFileName("", FILENAME);
+  TEST_EQUAL(gen, FILENAME + ".kml", ());
 }