Fix get_canonical/absolute_path() impl

base/fs.cpp

@@ -1,5 +1,5 @@
 // LAF Base Library
-// Copyright (c) 2021-2022 Igara Studio S.A.
+// Copyright (c) 2021-2024 Igara Studio S.A.
 // Copyright (c) 2001-2018 David Capello
 //
 // This file is released under the terms of the MIT license.
@@ -27,16 +27,18 @@
 
 namespace base {
 
+// On Windows we can use \ or / as path separators, but on Unix-like
+// platforms it's just /, as \ can be part of the file name.
 #if LAF_WINDOWS
-  const std::string::value_type path_separator = '\\';
+  const std::string::value_type* path_separators = "\\/";
 #else
-  const std::string::value_type path_separator = '/';
+  const std::string::value_type* path_separators = "/";
 #endif
 
 void make_all_directories(const std::string& path)
 {
   std::vector<std::string> parts;
-  split_string(path, parts, "/\\");
+  split_string(path, parts, path_separators);
 
   std::string intermediate;
   for (const std::string& component : parts) {
@@ -55,31 +57,6 @@ void make_all_directories(const std::string& path)
   }
 }
 
-std::string get_absolute_path(const std::string& filename)
-{
-  std::string fn = filename;
-  if (fn.size() > 2 &&
-#if LAF_WINDOWS
-      fn[1] != ':'
-#else
-      fn[0] != '/'
-#endif
-      ) {
-    fn = base::join_path(base::get_current_path(), fn);
-  }
-  fn = base::get_canonical_path(fn);
-  return fn;
-}
-
-bool is_path_separator(std::string::value_type chr)
-{
-  return (
-#if LAF_WINDOWS
-    chr == '\\' ||
-#endif
-    chr == '/');
-}
-
 std::string get_file_path(const std::string& filename)
 {
   std::string::const_reverse_iterator rit;
@@ -213,10 +190,10 @@ std::string get_file_title_with_path(const std::string& filename)
 std::string get_relative_path(const std::string& filename, const std::string& base_path)
 {
   std::vector<std::string> baseDirs;
-  split_string(base_path, baseDirs, "/\\");
+  split_string(base_path, baseDirs, path_separators);
 
   std::vector<std::string> toParts;
-  split_string(filename, toParts, "/\\");
+  split_string(filename, toParts, path_separators);
 
   // Find the common prefix
   auto itFrom = baseDirs.begin();
@@ -270,19 +247,79 @@ std::string remove_path_separator(const std::string& path)
 
 std::string fix_path_separators(const std::string& filename)
 {
-  std::string result(filename);
-
-  // Replace any separator with the system path separator.
-  std::replace_if(result.begin(), result.end(),
-                  is_path_separator, path_separator);
-
+  std::string result;
+  result.reserve(filename.size());
+  for (auto chr : filename) {
+    if (is_path_separator(chr)) {
+      if (result.empty() || !is_path_separator(result.back()))
+        result.push_back(path_separator);
+    }
+    else
+      result.push_back(chr);
+  }
   return result;
 }
 
-std::string normalize_path(const std::string& filename)
+// It tries to replicate the standard path::lexically_normal()
+// algorithm from https://en.cppreference.com/w/cpp/filesystem/path
+std::string normalize_path(const std::string& _path)
 {
-  std::string fn = base::get_canonical_path(filename);
-  fn = base::fix_path_separators(fn);
+  // Normal form of an empty path is an empty path.
+  if (_path.empty())
+    return std::string();
+
+  // Replace multiple slashes with a single path_separator.
+  std::string path = fix_path_separators(_path);
+
+  std::string fn;
+  if (!path.empty() && path[0] == path_separator)
+    fn.push_back(path_separator);
+
+  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;
+
+  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 (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;
 }
 

base/fs.h

@@ -17,8 +17,14 @@ namespace base {
 
   class Time;
 
-  // Default path separator (on Windows it is '\' and on Unix-like systems it is '/').
-  extern const std::string::value_type path_separator;
+  // Default path separator (on Windows it is '\' and on Unix-like
+  // systems it is '/').
+#if LAF_WINDOWS
+  static constexpr const std::string::value_type path_separator = '\\';
+#else
+  static constexpr const std::string::value_type path_separator = '/';
+#endif
+  extern const std::string::value_type* path_separators;
 
   bool is_file(const std::string& path);
   bool is_directory(const std::string& path);
@@ -48,18 +54,27 @@ namespace base {
   std::string get_lib_app_support_path();
 #endif
 
-  // If the given filename is a relative path, it converts the
-  // filename to an absolute one.
+  // Converts an existing file path to an absolute one, or returns an
+  // empty string if the file doesn't exist. It uses realpath() on
+  // POSIX-like systems and GetFullPathName() on Windows.
   std::string get_canonical_path(const std::string& path);
 
-  // TODO why get_canonical_path() is not enough?
-  std::string get_absolute_path(const std::string& filename);
+  // Returns the absolute path using lexical/string operations, and
+  // get_current_path() when needed. Doesn't require an existing file
+  // in "path".
+  std::string get_absolute_path(const std::string& path);
 
   paths list_files(const std::string& path);
 
   // Returns true if the given character is a valud path separator
   // (any of '\' or '/' characters).
-  bool is_path_separator(std::string::value_type chr);
+  inline constexpr bool is_path_separator(std::string::value_type chr) {
+    return (
+#if LAF_WINDOWS
+      chr == '\\' ||
+#endif
+      chr == '/');
+  }
 
   // Returns only the path (without the last trailing slash).
   std::string get_file_path(const std::string& filename);
@@ -89,9 +104,15 @@ namespace base {
   // Replaces all separators with the system separator.
   std::string fix_path_separators(const std::string& filename);
 
-  // Calls get_canonical_path() and fix_path_separators() for the
-  // given filename.
-  std::string normalize_path(const std::string& filename);
+  // Remove superfluous path elements ("/../" and "/./") and call
+  // fix_path_separators() for the given path.
+  std::string normalize_path(const std::string& path);
+
+  // Returns the absolute path normalized. This is the old behavior of
+  // get_absolute_path() on laf.
+  inline std::string get_absolute_normal_path(const std::string& path) {
+    return normalize_path(get_absolute_path(path));
+  }
 
   // Returns true if the filename contains one of the specified
   // extensions. The "extensions" parameter must be a set of possible

base/fs_tests.cpp

@@ -10,8 +10,32 @@
 #include "base/file_content.h"
 #include "base/fs.h"
 
+#include <cstdio>
+
+// We can use the <filesystem> in tests (because we don't distribute
+// tests in our target macOS 10.9 platform).
+#include <filesystem>
+namespace fs = std::filesystem;
+
 using namespace base;
 
+// We want to test against std::filesystem for future replacement of
+// some of our functions with the standard ones.
+TEST(FS, CurrentPath)
+{
+  // Compare with <filesystem>
+  EXPECT_EQ(fs::current_path(), get_current_path());
+  EXPECT_EQ(fs::path::preferred_separator, path_separator);
+}
+
+TEST(FS, FixPathSeparators)
+{
+  std::string sep(1, path_separator);
+  EXPECT_EQ(sep, fix_path_separators("/"));
+  EXPECT_EQ(sep, fix_path_separators("///"));
+  EXPECT_EQ("a" + sep + "b/", fix_path_separators("a///b/"));
+}
+
 TEST(FS, MakeDirectory)
 {
   EXPECT_FALSE(is_directory("a"));
@@ -191,6 +215,56 @@ TEST(FS, GetRelativePath)
 #endif
 }
 
+TEST(FS, GetAbsolutePath)
+{
+  const auto cp = get_current_path();
+
+  EXPECT_EQ(join_path(cp, "a"), get_absolute_path("a"));
+  EXPECT_EQ(join_path(cp, "./a"), get_absolute_path("./a"));
+  // Compare with <filesystem>
+  EXPECT_EQ(fs::absolute("."), get_absolute_path("."));
+  EXPECT_EQ(fs::absolute("./."), get_absolute_path("./."));
+  EXPECT_EQ(fs::absolute("./a/.."), get_absolute_path("./a/.."));
+  EXPECT_EQ(fs::absolute(".////."), get_absolute_path(".////."));
+
+#if LAF_WINDOWS
+  EXPECT_EQ("C:\\file", get_absolute_path("C:/path/../file"));
+#else
+  EXPECT_EQ("/path/../file", get_absolute_path("/path/../file"));
+#endif
+}
+
+TEST(FS, GetCanonicalPath)
+{
+  const auto cp = get_current_path();
+
+  EXPECT_EQ("", get_canonical_path("./non_existent_file"));
+  EXPECT_EQ(cp, get_canonical_path("."));
+
+  // Creates a file so get_canonical_path() returns its absolute path
+  {
+    FILE* f = std::fopen("_test_existing_file.txt", "w");
+    std::fclose(f);
+  }
+  EXPECT_EQ(join_path(cp, "_test_existing_file.txt"),
+            get_canonical_path("_test_existing_file.txt"));
+}
+
+TEST(FS, NormalizePath)
+{
+  for (auto sample : { "", ".", "./.", ".///./.", ".///./",
+                       "a/.", "a/", "./a", "a///b/./c",
+                       "..", "../..",
+                       "../../", ".././..", "./.././../.",
+                       "a/..", "../a/..", "../a/../..", "a/../..",
+                       "/a/../b"
+       }) {
+    EXPECT_EQ(fs::path(sample).lexically_normal(),
+              normalize_path(sample))
+      << "  sample=\"" << sample << "\"";
+  }
+}
+
 TEST(FS, JoinPath)
 {
   std::string sep;

base/fs_unix.h

@@ -209,10 +209,21 @@ std::string get_user_docs_folder()
 std::string get_canonical_path(const std::string& path)
 {
   char buffer[PATH_MAX];
+  errno = 0;
   // Ignore return value as realpath() returns nullptr anyway when the
   // resolved_path parameter is specified.
   realpath(path.c_str(), buffer);
-  return buffer;
+  if (errno != 0)
+    return std::string();
+  return buffer;                // No error, the file/dir exists
+}
+
+std::string get_absolute_path(const std::string& filename)
+{
+  std::string fn = filename;
+  if (!fn.empty() && fn[0] != '/')
+    fn = join_path(get_current_path(), fn);
+  return fn;
 }
 
 paths list_files(const std::string& path)

base/fs_win32.h

@@ -158,9 +158,23 @@ std::string get_user_docs_folder()
 
 std::string get_canonical_path(const std::string& path)
 {
+  std::string full = get_absolute_path(path);
+  DWORD attr = ::GetFileAttributes(from_utf8(full).c_str());
+  if (attr != INVALID_FILE_ATTRIBUTES)
+    return full;
+  else
+    return std::string();
+}
+
+std::string get_absolute_path(const std::string& path)
+{
+  std::string fn = path;
+  if (fn.size() > 2 && fn[1] != ':')
+    fn = base::join_path(base::get_current_path(), fn);
+
   TCHAR buffer[MAX_PATH+1];
   GetFullPathName(
-    from_utf8(path).c_str(),
+    from_utf8(fn).c_str(),
     sizeof(buffer)/sizeof(TCHAR),
     buffer,
     nullptr);