From cdfbe87f5b632dca6caa07e881519641e76fa7f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Fri, 16 Apr 2021 16:30:23 +0200 Subject: [PATCH] Avoid duplication of / in joinPaths (Windows) (#201) * Avoid duplication of / in joinPaths (Windows) Signed-off-by: ahcorde * Remove / in Linux and MacOS Signed-off-by: ahcorde * add test and doc Signed-off-by: Louise Poubel * Fixed test Signed-off-by: ahcorde * Improved joinPaths method Signed-off-by: ahcorde * Add quick fix Signed-off-by: ahcorde * Add quick fix Signed-off-by: ahcorde * Make linters happy Signed-off-by: ahcorde Co-authored-by: Louise Poubel --- include/ignition/common/Filesystem.hh | 3 ++- src/Filesystem.cc | 31 ++++++++++++++++++++++----- src/Filesystem_TEST.cc | 8 +++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/include/ignition/common/Filesystem.hh b/include/ignition/common/Filesystem.hh index 2db110802..42e95b327 100644 --- a/include/ignition/common/Filesystem.hh +++ b/include/ignition/common/Filesystem.hh @@ -126,7 +126,8 @@ namespace ignition /// \param[in] _path1 the left portion of the path /// \param[in] _path2 the right portion of the path /// \return Joined path. The function can do simplifications such as - /// elimination of ../ (but is not guaranteed to do so). + /// elimination of ../ and removal of duplicate // (but is not guaranteed to + /// do so). std::string IGNITION_COMMON_VISIBLE joinPaths(const std::string &_path1, const std::string &_path2); diff --git a/src/Filesystem.cc b/src/Filesystem.cc index b1c962c4f..f3539baae 100644 --- a/src/Filesystem.cc +++ b/src/Filesystem.cc @@ -257,19 +257,40 @@ 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 + 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 - return separator(_path1) + _path2; + path = sanitizeSlashes(separator(sanitizeSlashes(_path1)) + + sanitizeSlashes(_path2)); #else // _WIN32 // std::string path1 = checkWindowsPath(_path1); // std::string path2 = checkWindowsPath(_path2); // +1 for directory separator, +1 for the ending \0 character - std::vector combined(_path1.length() + _path2.length() + 2); + std::string path1 = sanitizeSlashes(_path1, true); + std::string path2 = sanitizeSlashes(_path2, true); + std::vector 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) - return checkWindowsPath(std::string(combined.data())); + if (::PathCombineA(combined.data(), path1.c_str(), path2.c_str()) != NULL) + path = sanitizeSlashes(checkWindowsPath(std::string(combined.data()))); else - return checkWindowsPath(separator(_path1) + _path1); + path = sanitizeSlashes(checkWindowsPath(separator(path1) + path2)); #endif // _WIN32 + return path; } ///////////////////////////////////////////////// diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index b07ddf3fb..257753705 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -408,6 +408,14 @@ TEST_F(FilesystemTest, append) EXPECT_EQ(path, separator("tmp") + separator("hello") + separator("there") + "again"); + + path = joinPaths("base", "/before", "after/"); + +#ifndef _WIN32 + EXPECT_EQ(path, "base//before/after"); +#else + EXPECT_EQ(path, "base\\before\\after"); +#endif } /////////////////////////////////////////////////