Fix join paths to remove duplicate separators (#209)

The joinPaths function would remove trailing slashes from the function arguments. This can cause downstream applications, like fuel tools using libzip, to treat a directory as a file. Signed-off-by: Nate Koenig <[email protected]>
gazebosim · May 3, 2021 · b85431f · b85431f
1 parent ee6fe52
commit b85431f
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 24 deletions.
diff --git a/src/Filesystem.cc b/src/Filesystem.cc
@@ -258,37 +258,27 @@ std::string ignition::common::joinPaths(const std::string &_path1,
                                         const std::string &_path2)
 {
   // avoid duplicated '/' at the beginning/end of the string
-  auto sanitizeSlashes = [](const std::string &_path, bool is_windows = false)
-  {
-    std::string result = _path;
-    if (is_windows && !result.empty() &&
-      (result[0] == '\\' || result[0] == '/'))
-    {
-        result.erase(0, 1);
-    }
-    if (!result.empty() &&
-      (result[result.length()-1] == '/' || result[result.length()-1] == '\\'))
-    {
-        result.erase(result.length()-1, 1);
-    }
-    return result;
-  };
   std::string path;
 #ifndef _WIN32
-  path = sanitizeSlashes(separator(sanitizeSlashes(_path1))
-    + sanitizeSlashes(_path2));
+  std::regex reg("/+");
+  path = std::regex_replace(separator(_path1) + _path2, reg, "/");
 #else  // _WIN32
   // std::string path1 = checkWindowsPath(_path1);
   // std::string path2 = checkWindowsPath(_path2);
   // +1 for directory separator, +1 for the ending \0 character
-  std::string path1 = sanitizeSlashes(_path1, true);
-  std::string path2 = sanitizeSlashes(_path2, true);
-  std::vector<CHAR> combined(path1.length() + path2.length() + 2);
+  std::regex reg("\\\\+");
+  std::vector<CHAR> combined(_path1.length() + _path2.length() + 2);
   // TODO(anyone): Switch to PathAllocCombine once switched to wide strings
-  if (::PathCombineA(combined.data(), path1.c_str(), path2.c_str()) != NULL)
-    path = sanitizeSlashes(checkWindowsPath(std::string(combined.data())));
+  if (::PathCombineA(combined.data(), _path1.c_str(), _path2.c_str()) != NULL)
+  {
+    path = std::regex_replace(
+        checkWindowsPath(std::string(combined.data())), reg, "\\");
+  }
   else
-    path = sanitizeSlashes(checkWindowsPath(separator(path1) + path2));
+  {
+    path = std::regex_replace(
+        checkWindowsPath(separator(_path1) + _path2), reg, "\\");
+  }
 #endif  // _WIN32
   return path;
 }

diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc
@@ -412,7 +412,42 @@ TEST_F(FilesystemTest, append)
   path = joinPaths("base", "/before", "after/");
 
 #ifndef _WIN32
-  EXPECT_EQ(path, "base//before/after");
+  EXPECT_EQ(path, "base/before/after/");
+#else
+  EXPECT_EQ(path, "base\\before\\after\\");
+#endif
+
+  path = joinPaths("base", "/before", "after///");
+#ifndef _WIN32
+  EXPECT_EQ(path, "base/before/after/");
+#else
+  EXPECT_EQ(path, "base\\before\\after\\");
+#endif
+
+  path = joinPaths("///base", "/before", "after");
+#ifndef _WIN32
+  EXPECT_EQ(path, "/base/before/after");
+#else
+  EXPECT_EQ(path, "\\base\\before\\after");
+#endif
+
+  path = joinPaths("/base", "/before", "after");
+#ifndef _WIN32
+  EXPECT_EQ(path, "/base/before/after");
+#else
+  EXPECT_EQ(path, "\\base\\before\\after");
+#endif
+
+  path = joinPaths("///base", "///before//", "/after///");
+#ifndef _WIN32
+  EXPECT_EQ(path, "/base/before/after/");
+#else
+  EXPECT_EQ(path, "\\base\\before\\after\\");
+#endif
+
+  path = joinPaths("base", "/before", "after");
+#ifndef _WIN32
+  EXPECT_EQ(path, "base/before/after");
 #else
   EXPECT_EQ(path, "base\\before\\after");
 #endif