From 06faa3f6ac29c9cb9f01df4cbfe231bd3616e960 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Thu, 20 Nov 2025 12:45:53 +0100 Subject: [PATCH] Fix RemoveLastPathComponent edge case with absolute paths --- src/filesys.cpp | 40 +++++++++-------- src/filesys.h | 9 ++-- src/unittest/test_filesys.cpp | 81 ++++++++++++++++++----------------- 3 files changed, 68 insertions(+), 62 deletions(-) diff --git a/src/filesys.cpp b/src/filesys.cpp index 62352403f3..a2d8420452 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -610,14 +610,12 @@ bool RecursiveDeleteContent(const std::string &path) bool CreateAllDirs(const std::string &path) { - std::vector tocreate; - std::string basepath = path; - while(!PathExists(basepath)) - { + std::string basepath = path, removed; + while (!PathExists(basepath)) { tocreate.push_back(basepath); - basepath = RemoveLastPathComponent(basepath); - if(basepath.empty()) + basepath = RemoveLastPathComponent(basepath, &removed); + if (removed.empty()) break; } for(int i=tocreate.size()-1;i>=0;i--) @@ -748,25 +746,31 @@ std::string RemoveLastPathComponent(const std::string &path, size_t remaining = path.size(); - for(int i = 0; i < count; ++i){ - // strip a dir delimiter - while(remaining != 0 && IsDirDelimiter(path[remaining-1])) + for (int i = 0; i < count; ++i) { + // strip a dir delimiter, unless the path is empty + // because "" and "/" are not the same + // FIXME: same problem probably exists on win32 with "C:" + while (remaining > 1 && IsDirDelimiter(path[remaining-1])) remaining--; // strip a path component size_t component_end = remaining; - while(remaining != 0 && !IsDirDelimiter(path[remaining-1])) + while (remaining != 0 && !IsDirDelimiter(path[remaining-1])) remaining--; size_t component_start = remaining; - // strip a dir delimiter - while(remaining != 0 && IsDirDelimiter(path[remaining-1])) + // strip another delimiter + while (remaining > 1 && IsDirDelimiter(path[remaining-1])) remaining--; - if(removed){ + if (component_start == component_end) + break; // could not remove anything + if (removed) { std::string component = path.substr(component_start, component_end - component_start); - if(i) - *removed = component + DIR_DELIM + *removed; - else - *removed = component; + if (i) { + removed->insert(0, DIR_DELIM); + removed->insert(0, component); + } else { + *removed = std::move(component); + } } } return path.substr(0, remaining); @@ -804,7 +808,7 @@ std::string RemoveRelativePathComponents(std::string path) while (pos != 0 && IsDirDelimiter(path[pos-1])) pos--; if (component_start == 0) { - // We need to remove the delemiter too + // We need to remove the delimiter too path = path.substr(component_with_delim_end, std::string::npos); } else { path = path.substr(0, pos) + DIR_DELIM + diff --git a/src/filesys.h b/src/filesys.h index 73c9041a31..5ae1f1234e 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -116,12 +116,13 @@ bool MoveDir(const std::string &source, const std::string &target); // Ignores case differences and '/' vs. '\\' on Windows bool PathStartsWith(const std::string &path, const std::string &prefix); -// Remove last path component and the dir delimiter before and/or after it, -// returns "" if there is only one path component. -// removed: If non-NULL, receives the removed component(s). +// Remove last path component and the dir delimiter before and/or after it. +// If there's only one path component it will refuse to remove it (if absolute) +// or return "" (if relative). +// removed: If non-NULL, receives the removed components // count: Number of components to remove std::string RemoveLastPathComponent(const std::string &path, - std::string *removed = NULL, int count = 1); + std::string *removed = nullptr, int count = 1); // Remove "." and ".." path components and for every ".." removed, remove // the last normal path component before it. Unlike AbsolutePath, diff --git a/src/unittest/test_filesys.cpp b/src/unittest/test_filesys.cpp index 34666f6118..6d2cd6bb4f 100644 --- a/src/unittest/test_filesys.cpp +++ b/src/unittest/test_filesys.cpp @@ -49,6 +49,12 @@ void TestFileSys::runTests(IGameDef *gamedef) //////////////////////////////////////////////////////////////////////////////// +#if defined(_WIN32) +static constexpr bool win32 = true; +#else +static constexpr bool win32 = false; +#endif + // adjusts a POSIX path to system-specific conventions // -> changes '/' to DIR_DELIM // -> absolute paths start with "C:\\" on windows @@ -61,10 +67,10 @@ static std::string p(std::string path) } } - #ifdef _WIN32 +#ifdef _WIN32 if (path[0] == '\\') - path = "C:" + path; - #endif + path.insert(0, "C:"); +#endif return path; } @@ -75,11 +81,7 @@ void TestFileSys::testIsDirDelimiter() UASSERT(fs::IsDirDelimiter('/') == true); UASSERT(fs::IsDirDelimiter('A') == false); UASSERT(fs::IsDirDelimiter(0) == false); -#ifdef _WIN32 - UASSERT(fs::IsDirDelimiter('\\') == true); -#else - UASSERT(fs::IsDirDelimiter('\\') == false); -#endif + UASSERT(fs::IsDirDelimiter('\\') == win32); } @@ -127,33 +129,17 @@ void TestFileSys::testPathStartsWith() for (int i = 0; i < numpaths; i++) for (int j = 0; j < numpaths; j++){ - /*verbosestream<<"testing fs::PathStartsWith(\"" - <