Fix join paths again (#215)

* Remove the regex so that path separators in the middle of a string are not altered Signed-off-by: Nate Koenig <[email protected]> * windows debugging Signed-off-by: Nate Koenig <[email protected]> * Fixing windows Signed-off-by: Nate Koenig <[email protected]> * One more windows fix Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]>
gazebosim · May 10, 2021 · d96242a · d96242a
1 parent 767ff48
commit d96242a
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 14 deletions.
diff --git a/src/Filesystem.cc b/src/Filesystem.cc
@@ -257,27 +257,63 @@ std::string checkWindowsPath(const std::string _path)
 std::string ignition::common::joinPaths(const std::string &_path1,
                                         const std::string &_path2)
 {
-  // avoid duplicated '/' at the beginning/end of the string
+
+  /// This function is used to avoid duplicated path separators at the
+  /// beginning/end of the string, and between the two paths being joined.
+  /// \param[in] _path This is the string to sanitize.
+  /// \param[in] _stripLeading True if the leading separator should be
+  /// removed.
+  auto sanitizeSlashes = [](const std::string &_path,
+                            bool _stripLeading = false)
+  {
+    // Shortcut
+    if (_path.empty())
+      return _path;
+
+    std::string result = _path;
+
+    // Use the appropriate character for each platform.
+#ifndef _WIN32
+    char replacement = '/';
+#else
+    char replacement = '\\';
+#endif
+
+    // Sanitize the start of the path.
+    size_t index = 0;
+    size_t leadingIndex = _stripLeading ? 0 : 1;
+    for (; result[index] == replacement; ++index)
+    {
+    }
+    if (index > leadingIndex)
+      result.erase(leadingIndex, index-leadingIndex);
+
+    // Sanitize the end of the path.
+    index = result.length()-1;
+    for (; result[index] == replacement; --index)
+    {
+    }
+    index += 1;
+    if (index < result.length()-1)
+        result.erase(index+1);
+    return result;
+  };
+
   std::string path;
 #ifndef _WIN32
-  std::regex reg("/+");
-  path = std::regex_replace(separator(_path1) + _path2, reg, "/");
+  path = sanitizeSlashes(sanitizeSlashes(separator(_path1)) +
+      sanitizeSlashes(_path2, true));
 #else  // _WIN32
-  // std::string path1 = checkWindowsPath(_path1);
-  // std::string path2 = checkWindowsPath(_path2);
-  // +1 for directory separator, +1 for the ending \0 character
-  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)
+  std::string path1 = sanitizeSlashes(checkWindowsPath(_path1));
+  std::string path2 = sanitizeSlashes(checkWindowsPath(_path2), true);
+  std::vector<CHAR> combined(path1.length() + path2.length() + 2);
+  if (::PathCombineA(combined.data(), path1.c_str(), path2.c_str()) != NULL)
   {
-    path = std::regex_replace(
-        checkWindowsPath(std::string(combined.data())), reg, "\\");
+    path = sanitizeSlashes(checkWindowsPath(std::string(combined.data())));
   }
   else
   {
-    path = std::regex_replace(
-        checkWindowsPath(separator(_path1) + _path2), reg, "\\");
+    path = sanitizeSlashes(checkWindowsPath(separator(path1) + path2));
   }
 #endif  // _WIN32
   return path;

diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc
@@ -451,6 +451,10 @@ TEST_F(FilesystemTest, append)
 #else
   EXPECT_EQ(path, "base\\before\\after");
 #endif
+
+  // Make sure that the slashes in the middle of string are not altered.
+  path = joinPaths("https://fuel.ignitionrobotics.org", "/models", "box");
+  EXPECT_EQ(path, "https://fuel.ignitionrobotics.org/models/box");
 }
 
 /////////////////////////////////////////////////