Skip to content

Commit

Permalink
Fix base::get_absolute_path() fixing base::normalize_path() function
Browse files Browse the repository at this point in the history
base::get_absolute_path() resulted in bugs when more than two or more
double dots were present in the input path.
The source of the error was the base::normalize_path() function
which has been fixed.
Also added tests in 'fs_tests.cpp'.
This fix is part of the solution of aseprite/aseprite#4851.
  • Loading branch information
Gasparoken committed Dec 18, 2024
1 parent c84b89b commit bd69dea
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 48 deletions.
83 changes: 35 additions & 48 deletions base/fs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,36 @@ std::string normalize_path(const std::string& _path)
// Replace multiple slashes with a single path_separator.
std::string path = fix_path_separators(_path);

std::string fn;
fn.reserve(path.size());
std::vector<std::string> parts;
split_string(path, parts, path_separators);

// Add the first separator for absolute paths.
if (!path.empty() && path[0] == path_separator) {
fn.push_back(path_separator);
std::vector<std::string> fn_parts;
bool has_root = (!path.empty() && path[0] == path_separator);
bool last_dot = (parts.back() == ".");
bool last_slash = parts.back().empty();
if (last_slash || last_dot)
parts.pop_back();

for (const auto& part : parts) {
// Skip each dot part.
if (part.empty() || part == ".")
continue;
else if (part == "..") {
if (has_root && fn_parts.empty())
continue;
if (!fn_parts.empty() && fn_parts.back() != "..")
fn_parts.pop_back();
else
fn_parts.push_back(part);
}
else
fn_parts.push_back(part);
}

// Reconstruct the filename 'fn' from 'fn_parts'
std::string fn;
if (has_root) {
fn.push_back(path_separator);
#if LAF_WINDOWS
// Add the second separator for network paths.
if (path.size() >= 2 && path[1] == path_separator) {
Expand All @@ -296,51 +319,15 @@ std::string normalize_path(const std::string& _path)
#endif
}

std::vector<std::string> parts;
split_string(path, parts, path_separators);

// Last element generates a final dot or slash in normalized path.
bool last_dot = false;
for (const auto& part : fn_parts)
fn = join_path(fn, part);

auto n = int(parts.size());
for (int i = 0; i < n; ++i) {
const auto& part = parts[i];

// Remove each dot part.
if (part == ".") {
last_dot = true;

if (i + 1 == n)
break;

fn = join_path(fn, std::string());
continue;
}

if (!part.empty())
last_dot = false;
if (!fn.empty() &&
parts.back() != ".." &&
(last_slash || last_dot))
fn.push_back(path_separator);

if (part != ".." && i + 1 < n && parts[i + 1] == "..") {
// Skip this "part/.."
++i;
last_dot = true;
}
else if (!part.empty()) {
fn = join_path(fn, part);
}
else
last_dot = true;
}
if (last_dot) {
if (fn.empty())
fn = ".";
else if (fn.back() != path_separator &&
// Don't include trailing slash for ".." filename
get_file_name(fn) != "..") {
fn.push_back(path_separator);
}
}
return fn;
return (fn.empty() ? "." : fn);
}

bool has_file_extension(const std::string& filename, const base::paths& extensions)
Expand Down
7 changes: 7 additions & 0 deletions base/fs_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,14 @@ TEST(FS, GetAbsolutePath)

#if LAF_WINDOWS
EXPECT_EQ("C:\\file", get_absolute_path("C:/path/../file"));
EXPECT_EQ("C:\\user\\file", get_absolute_path("C:/user/name/path/../../file"));
EXPECT_EQ("C:\\file", get_absolute_path("C:/user/name/path/../../../file"));
EXPECT_EQ("C:\\path\\file", get_absolute_path("C:/user/name/..//.././path/tag/../file"));
#else
EXPECT_EQ("/file", get_absolute_path("/path/../file"));
EXPECT_EQ("/user/file", get_absolute_path("/user/name/path/../../file"));
EXPECT_EQ("/file", get_absolute_path("/user/name/path/../../../file"));
EXPECT_EQ("/path/file", get_absolute_path("/user/name/..//.././path/tag/../file"));
#endif
}

Expand Down Expand Up @@ -288,6 +294,7 @@ TEST(FS, NormalizePath)
EXPECT_EQ(".", normalize_path("a/.."));
EXPECT_EQ("..", normalize_path("../a/.."));
EXPECT_EQ(".." + sep + "..", normalize_path("../a/../.."));
EXPECT_EQ(".." + sep + "..", normalize_path("../a/../../"));
EXPECT_EQ("..", normalize_path("a/../.."));
EXPECT_EQ(sep + "b", normalize_path("/a/../b"));

Expand Down

0 comments on commit bd69dea

Please sign in to comment.