Merge pull request #383 from kiwix/fix_path_windows

src/tools/pathTools.cpp

@@ -81,13 +81,19 @@ std::wstring Utf8ToWide(const std::string& str)
 bool isRelativePath(const std::string& path)
 {
 #ifdef _WIN32
-  return path.empty() || path.substr(1, 2) == ":\\" ? false : true;
+  if (path.size() < 3 ) {
+    return true;
+  }
+  if (path.substr(1, 2) == ":\\" || path.substr(0, 2) == "\\\\") {
+    return false;
+  }
+  return true;
 #else
   return path.empty() || path.substr(0, 1) == "/" ? false : true;
 #endif
 }
 
-std::vector<std::string> normalizeParts(std::vector<std::string> parts, bool absolute)
+std::vector<std::string> normalizeParts(std::vector<std::string>& parts, bool absolute)
 {
   std::vector<std::string> ret;
 #ifdef _WIN32
@@ -95,10 +101,35 @@ std::vector<std::string> normalizeParts(std::vector<std::string> parts, bool abs
   //Starts from there.
   auto it = find_if(parts.rbegin(), parts.rend(),
                     [](const std::string& p) ->bool
-                      { return p.length() == 2 && p[1] == ':'; });
+                      { return ((p.length() == 2 && p[1] == ':')
+                             || (p.length() > 2 && p[0] == '\\' && p[1] == '\\')); });
   if (it != parts.rend()) {
     parts.erase(parts.begin(), it.base()-1);
   }
+  // Special case for samba mount point starting with two "\\" ("\\\\samba\\foo")
+  if (parts.size() > 2  && parts[0].empty() && parts[1].empty()) {
+    parts.erase(parts.begin(), parts.begin()+2);
+    parts[0] = "\\\\" + parts[0];
+  }
+  // Special case if we have a samba drive not at first.
+  // Path is "..\\\\sambdadrive\\..\\.." So we will have an empty part.
+  auto previous_empty = false;
+  for (it = parts.rbegin(); it!=parts.rend(); it++) {
+    if(it->empty()) {
+      if (previous_empty) {
+        it++;
+        break;
+      } else {
+        previous_empty = true;
+      }
+    } else {
+      previous_empty = false;
+    }
+  }
+  if (it != parts.rend()) {
+    parts.erase(parts.begin(), it.base()-1);
+    parts[0] = "\\\\" + parts[0];
+  }
 #endif
 
   size_t index = 0;
@@ -144,8 +175,10 @@ std::vector<std::string> normalizeParts(std::vector<std::string> parts, bool abs
 
 std::string computeRelativePath(const std::string& path, const std::string& absolutePath)
 {
-  auto pathParts = normalizeParts(kiwix::split(path, SEPARATOR, false), false);
+  auto parts = kiwix::split(path, SEPARATOR, false);
-  auto absolutePathParts = kiwix::split(absolutePath, SEPARATOR, false);
+  auto pathParts = normalizeParts(parts, false);
+  parts = kiwix::split(absolutePath, SEPARATOR, false);
+  auto absolutePathParts = normalizeParts(parts, true);
 
   unsigned int commonCount = 0;
   while (commonCount < pathParts.size()
@@ -172,8 +205,10 @@ std::string computeAbsolutePath(const std::string& path, const std::string& rela
     absolutePath = getCurrentDirectory();
   }
 
-  auto absoluteParts = normalizeParts(kiwix::split(absolutePath, SEPARATOR, false), true);
+  auto parts = kiwix::split(absolutePath, SEPARATOR, false);
-  auto relativeParts = kiwix::split(relativePath, SEPARATOR, false);
+  auto absoluteParts = normalizeParts(parts, true);
+  parts = kiwix::split(relativePath, SEPARATOR, false);
+  auto relativeParts = normalizeParts(parts, false);
 
   absoluteParts.insert(absoluteParts.end(), relativeParts.begin(), relativeParts.end());
   auto ret = kiwix::join(normalizeParts(absoluteParts, true), SEPARATOR);
@@ -182,7 +217,8 @@ std::string computeAbsolutePath(const std::string& path, const std::string& rela
 
 std::string removeLastPathElement(const std::string& path)
 {
-  auto parts = normalizeParts(kiwix::split(path, SEPARATOR, false), false);
+  auto parts_ = kiwix::split(path, SEPARATOR, false);
+  auto parts = normalizeParts(parts_, false);
   if (!parts.empty()) {
     parts.pop_back();
   }
@@ -202,7 +238,8 @@ std::string appendToDirectory(const std::string& directoryPath, const std::strin
 
 std::string getLastPathElement(const std::string& path)
 {
-  auto parts = normalizeParts(kiwix::split(path, SEPARATOR), false);
+  auto parts_ = kiwix::split(path, SEPARATOR);
+  auto parts = normalizeParts(parts_, false);
   if (parts.empty()) {
     return "";
   }

test/library.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Tommi Maekitalo
+ * Copyright (C) 2019 Matthieu Gautier <mgautier@kymeria.fr>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -262,8 +262,13 @@ TEST_F(LibraryTest, filterCheck)
 TEST_F(LibraryTest, getBookByPath)
 {
   auto& book = lib.getBookById(lib.getBooksIds()[0]);
-  book.setPath("/some/abs/path.zim");
+#ifdef _WIN32
-  EXPECT_EQ(lib.getBookByPath("/some/abs/path.zim").getId(), book.getId());
+  auto path = "C:\\some\\abs\\path.zim";
+#else
+  auto path = "/some/abs/path.zim";
+#endif
+  book.setPath(path);
+  EXPECT_EQ(lib.getBookByPath(path).getId(), book.getId());
   EXPECT_THROW(lib.getBookByPath("non/existant/path.zim"), std::out_of_range);
 }
 };

test/pathTools.cpp

@@ -25,6 +25,7 @@
 #ifdef _WIN32
 # define S "\\"
 # define AS "c:"
+# define A_SAMBA "\\\\sambadir"
 #else
 # define S "/"
 # define AS ""
@@ -42,7 +43,10 @@
 #define A4(a, b, c, d) A1(P4(a, b, c, d))
 #define A5(a, b, c, d, e) A1(P5(a, b, c, d, e))
 
-std::vector<std::string> normalizeParts(std::vector<std::string> parts, bool absolute);
+std::vector<std::string> normalizeParts(std::vector<std::string>& parts, bool absolute);
+std::vector<std::string> nParts(std::vector<std::string> parts, bool absolute) {
+  return normalizeParts(parts, absolute);
+}
 #ifdef _WIN32
 std::wstring Utf8ToWide(const std::string& str);
 std::string WideToUtf8(const std::wstring& wstr);
@@ -54,7 +58,7 @@ namespace
 #define V std::vector<std::string>
 TEST(pathTools, normalizePartsAbsolute)
 {
-#define N(...) normalizeParts(__VA_ARGS__, true)
+#define N(...) nParts(__VA_ARGS__, true)
   ASSERT_EQ(N({}), V({}));
 #ifdef _WIN32
   ASSERT_EQ(N({"c:"}), V({"c:"}));
@@ -75,13 +79,14 @@ TEST(pathTools, normalizePartsAbsolute)
 
 #ifdef _WIN32
   ASSERT_EQ(N({"c:", "a", "b", ".", "c", "d:", "..", "foo"}), V({"d:", "foo"}));
+  ASSERT_EQ(N({"","","samba","a","b"}), V({"\\\\samba", "a", "b"}));
 #endif
 #undef N
 }
 
 TEST(pathTools, normalizePartsRelative)
 {
-#define N(...) normalizeParts(__VA_ARGS__, false)
+#define N(...) nParts(__VA_ARGS__, false)
   ASSERT_EQ(N({}), V({}));
   ASSERT_EQ(N({""}), V({}));
   ASSERT_EQ(N({"a"}), V({"a"}));
@@ -103,6 +108,10 @@ TEST(pathTools, isRelativePath)
   ASSERT_TRUE(isRelativePath(P4("foo","","bar","")));
   ASSERT_FALSE(isRelativePath(A1("foo")));
   ASSERT_FALSE(isRelativePath(A2("foo", "bar")));
+#ifdef _WIN32
+  ASSERT_FALSE(isRelativePath(P2(A_SAMBA, "foo")));
+  ASSERT_FALSE(isRelativePath(P3(A_SAMBA, "foo", "bar")));
+#endif
 }
 
 TEST(pathTools, computeAbsolutePath)
@@ -121,6 +130,12 @@ TEST(pathTools, computeAbsolutePath)
             A5("a","b","c","d","foo"));
   ASSERT_EQ(computeAbsolutePath(A5("a","b","c","d","e"), P5("..","..","..","g","foo")),
             A4("a","b","g","foo"));
+#ifdef _WIN32
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b",""), P2("..","foo")),
+            P3(A_SAMBA,"a","foo"));
+  ASSERT_EQ(computeAbsolutePath(P6(A_SAMBA,"a","b","c","d","e"), P5("..","..","..","g","foo")),
+            P5(A_SAMBA,"a","b","g","foo"));
+#endif
 }
 
 TEST(pathTools, computeRelativePath)
@@ -137,6 +152,13 @@ TEST(pathTools, computeRelativePath)
             P2("..","foo"));
   ASSERT_EQ(computeRelativePath(A5("a","b","c","d","e"), A4("a","b","g","foo")),
             P5("..","..","..","g","foo"));
+#ifdef _WIN32
+  ASSERT_EQ(computeRelativePath(P3(A_SAMBA,"a","b"), P3(A_SAMBA,"a","foo")),
+            P2("..","foo"));
+  ASSERT_EQ(computeRelativePath(P6(A_SAMBA,"a","b","c","d","e"), P5(A_SAMBA,"a","b","g","foo")),
+            P5("..","..","..","g","foo"));
+
+#endif
 }
 
 TEST(pathTools, removeLastPathElement)
@@ -179,6 +201,15 @@ TEST(pathTools, goUp)
             "c:");
   ASSERT_EQ(computeAbsolutePath(A3("a","b","c"), P4("..","..","..","..")),
             "c:");
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), ".."),
+            P3(A_SAMBA,"a", "b"));
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), P2("..","..")),
+            P2(A_SAMBA,"a"));
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), P3("..","..","..")),
+            A_SAMBA);
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), P4("..","..","..","..")),
+            A_SAMBA);
+
 #else
   ASSERT_EQ(computeAbsolutePath(A3("a","b","c"), P3("..","..","..")),
             "/");
@@ -195,6 +226,12 @@ TEST(pathTools, goUp)
             A1("foo"));
   ASSERT_EQ(computeAbsolutePath(A3("a","b","c"), P5("..","..","..","..","foo")),
             A1("foo"));
+#ifdef _WIN32
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), P4("..","..","..","foo")),
+            P2(A_SAMBA,"foo"));
+  ASSERT_EQ(computeAbsolutePath(P4(A_SAMBA,"a","b","c"), P5("..","..","..","..","foo")),
+            P2(A_SAMBA,"foo"));
+#endif
 }
 
 
@@ -203,11 +240,22 @@ TEST(pathTools, dirChange)
 {
   std::string p1("c:\\a\\b\\c");
   std::string p2("d:\\d\\e\\foo.xml");
+  {
     std::string relative_path = computeRelativePath(p1, p2);
-  ASSERT_EQ(relative_path, "d:\\d\\e\\foo.xml");
+    ASSERT_EQ(relative_path, p2);
     std::string abs_path = computeAbsolutePath(p1, relative_path);
     ASSERT_EQ(abs_path, p2);
     ASSERT_EQ(computeAbsolutePath(p1, "..\\..\\..\\..\\..\\d:\\d\\e\\foo.xml"), p2);
+  }
+  std::string ps("\\\\samba\\d\\e\\foo.xml");
+  {
+    std::string relative_path = computeRelativePath(p1, ps);
+    ASSERT_EQ(relative_path, ps);
+    std::string abs_path = computeAbsolutePath(p1, relative_path);
+    ASSERT_EQ(abs_path, ps);
+    // I'm not sure this test is valid on windows :/
+//    ASSERT_EQ(computeAbsolutePath(p1, "..\\..\\..\\..\\..\\\\samba\\d\\e\\foo.xml"), ps);
+  }
 }
 
 TEST(pathTools, Utf8ToWide)