Merge pull request #1820 from AggieAir/pr-fix-fs-canonical

core: fix some bugs in fs_canonical
mavlink · Jun 30, 2022 · b7b500c · b7b500c
2 parents aadcd28 + 0709325
commit b7b500c
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 44 deletions.
diff --git a/src/mavsdk/core/CMakeLists.txt b/src/mavsdk/core/CMakeLists.txt
@@ -126,22 +126,23 @@ install(DIRECTORY
 
 list(APPEND UNIT_TEST_SOURCES
     ${PROJECT_SOURCE_DIR}/mavsdk/core/callback_list_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavsdk_time_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavsdk_math_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavlink_channels_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/unittests_main.cpp
-    # TODO: add this again
-    #${PROJECT_SOURCE_DIR}/mavsdk/core/http_loader_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/timeout_handler_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/call_every_handler_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/curl_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/cli_arg_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/curl_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/locked_queue_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/safe_queue_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/fs_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/geometry_test.cpp
+    # TODO: add this again
+    #${PROJECT_SOURCE_DIR}/mavsdk/core/http_loader_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavsdk_math_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/mavsdk_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavsdk_time_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/mavlink_channels_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/mavlink_mission_transfer_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/mavlink_statustext_handler_test.cpp
-    ${PROJECT_SOURCE_DIR}/mavsdk/core/geometry_test.cpp
     ${PROJECT_SOURCE_DIR}/mavsdk/core/ringbuffer_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/safe_queue_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/timeout_handler_test.cpp
+    ${PROJECT_SOURCE_DIR}/mavsdk/core/unittests_main.cpp
 )
 set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)
diff --git a/src/mavsdk/core/fs.cpp b/src/mavsdk/core/fs.cpp
@@ -62,53 +62,49 @@ std::string fs_filename(const std::string& path)
 
 std::string fs_canonical(const std::string& path)
 {
-    std::stack<std::string> st;
-    std::string dir;
-    std::string res{};
-
-    if (path.rfind(path_separator, 0) != 0) {
-        char buffer[PATH_MAX];
-        if (getcwd(buffer, sizeof(buffer)) != nullptr) {
-            res = std::string(buffer);
-        }
-    } else {
-        res = path_separator;
-    }
+    std::deque<std::string> st;
+    std::string res;
 
     int len = path.length();
 
-    for (int i = 0; i < len; i++) {
-        dir.clear();
-        while (path[i] == '/') {
-            i++;
-        }
-        while (i < len && path[i] != '/') {
-            dir.push_back(path[i]);
-            i++;
+    int index = 0;
+    while (index < len) {
+        int next_index = path.find(path_separator, index);
+        if (next_index == -1) {
+            next_index = path.size();
         }
+
+        std::string dir = path.substr(index, next_index - index);
+
         if (dir.compare("..") == 0 && !st.empty()) {
-            st.pop();
-        } else if (dir.compare(".") == 0) {
-            continue;
-        } else if (dir.length() != 0) {
-            st.push(dir);
+            st.pop_back();
+        } else if (dir.length() != 0 && dir.compare(".") != 0) {
+            st.push_back(dir);
         }
+
+        index = next_index + path_separator.size();
     }
 
-    std::stack<std::string> st1;
-    while (!st.empty()) {
-        st1.push(st.top());
-        st.pop();
+    if (path.rfind(path_separator, path_separator.size() - 1) != 0) {
+        char buffer[PATH_MAX];
+        if (getcwd(buffer, sizeof(buffer)) != nullptr) {
+            res.append(buffer);
+            if (!st.empty()) {
+                res.append(path_separator);
+            }
+        }
+    } else {
+        res.append(path_separator);
     }
 
-    while (!st1.empty()) {
-        std::string temp = st1.top();
-        if (st1.size() != 1) {
-            res.append(temp + "/");
+    while (!st.empty()) {
+        std::string temp = st.front();
+        if (st.size() != 1) {
+            res.append(temp + path_separator);
         } else {
             res.append(temp);
         }
-        st1.pop();
+        st.pop_front();
     }
 
     return res;

diff --git a/src/mavsdk/core/fs_test.cpp b/src/mavsdk/core/fs_test.cpp
@@ -0,0 +1,79 @@
+#include <gtest/gtest.h>
+#include <unistd.h>
+#include "fs.h"
+
+#if !defined(WINDOWS)
+#define PATH_MAX 4096
+
+TEST(Filesystem, AbsolutePathUnchanged)
+{
+    const std::string path = "/sbin/init";
+    ASSERT_EQ(path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RemoveFinalSlash)
+{
+    const std::string path = "/usr/local/include/mavsdk/";
+    const std::string canonical_path = "/usr/local/include/mavsdk";
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RemoveDuplicateSlashes)
+{
+    const std::string path = "//////opt/////ros////foxy";
+    const std::string canonical_path = "/opt/ros/foxy";
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RemoveInternalDots)
+{
+    const std::string path = "/sys/./class/./input";
+    const std::string canonical_path = "/sys/class/input";
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, ResolveDoubleDots)
+{
+    const std::string path = "/bin/../dev/../etc/crontab/../shadow";
+    const std::string canonical_path = "/etc/shadow";
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RelativePathEmpty)
+{
+    char cwd[PATH_MAX];
+    getcwd(cwd, PATH_MAX);
+    const std::string path = "";
+    const std::string canonical_path = std::string(cwd);
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RelativePathDot)
+{
+    char cwd[PATH_MAX];
+    getcwd(cwd, PATH_MAX);
+    const std::string path = ".";
+    const std::string canonical_path = std::string(cwd);
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RelativePathBare)
+{
+    char cwd[PATH_MAX];
+    getcwd(cwd, PATH_MAX);
+    const std::string path = "src/mavsdk/core/fs_test.cpp";
+    const std::string canonical_path = std::string(cwd) + mavsdk::path_separator + path;
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(path));
+}
+
+TEST(Filesystem, RelativePathDotSlash)
+{
+    char cwd[PATH_MAX];
+    getcwd(cwd, PATH_MAX);
+    const std::string bare_path = "src/mavsdk/plugins/mavlink_passthrough";
+    const std::string dotslash_path = "./" + bare_path;
+    const std::string canonical_path = std::string(cwd) + mavsdk::path_separator + bare_path;
+    ASSERT_EQ(canonical_path, mavsdk::fs_canonical(dotslash_path));
+}
+
+#endif