Use absolute paths when loading XML files (#4751) (#4830)

* Use absolute paths when loading XML files (#4751) * Refs #10173: Add XMLLoadFile unittest Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Use absolute paths when loading XML files Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Fix windows.h INTERFACE name collision Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Avoid MINMAX windows warning as error in unittests Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Use DEFAULT_FASTDDS_PROFILES for windows Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Enable internal debug in test Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Force log info in the test Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #10173: Correct windows FASTDDS_DEFAULT PROFILES env and include fastdds/log instead of fastrtps Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> (cherry picked from commit 0919ff2) # Conflicts: # include/fastrtps/xmlparser/XMLParserCommon.h # src/cpp/rtps/xmlparser/XMLParser.cpp # src/cpp/rtps/xmlparser/XMLParserCommon.cpp # src/cpp/rtps/xmlparser/XMLProfileManager.cpp # test/unittest/xmlparser/CMakeLists.txt * Fix conflicts Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Fix conflicts Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Avoid minmax warning in windows Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com> Co-authored-by: Mario Dominguez <mariodominguez@eprosima.com>
eProsima · May 30, 2024 · de8aeb5 · de8aeb5
1 parent 42e6632
commit de8aeb5
Show file tree

Hide file tree

Showing 9 changed files with 385 additions and 11 deletions.
diff --git a/include/fastrtps/xmlparser/XMLParser.h b/include/fastrtps/xmlparser/XMLParser.h
@@ -108,6 +108,18 @@ class XMLParser
             const std::string& filename,
             up_base_node_t& root);
 
+    /**
+     * Load a XML file.
+     * @param filename Name for the file to be loaded.
+     * @param root Root node.
+     * @param is_default Is the default XML file.
+     * @return XMLP_ret::XML_OK on success, XMLP_ret::XML_ERROR in other case.
+     */
+    static XMLP_ret loadXML(
+            const std::string& filename,
+            up_base_node_t& root,
+            bool is_default);
+
     /**
      * Load a XML data from buffer.
      * @param data XML data to load.

diff --git a/include/fastrtps/xmlparser/XMLProfileManager.h b/include/fastrtps/xmlparser/XMLProfileManager.h
@@ -69,6 +69,18 @@ class XMLProfileManager
     RTPS_DllAPI static XMLP_ret loadXMLFile(
             const std::string& filename);
 
+    /**
+     * Load a profiles XML file.
+     * @param filename Name for the file to be loaded.
+     * @param is_default Flag to indicate if the file is a default profiles file.
+     * @return XMLP_ret::XML_OK if all profiles are correct, XMLP_ret::XML_NOK if some are and some are not,
+     *         XMLP_ret::XML_ERROR in other case.
+     */
+
+    static XMLP_ret loadXMLFile(
+            const std::string& filename,
+            bool is_default);
+
     /**
      * Load a profiles XML string.
      * @param data Buffer containing the data.

diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp
@@ -18,6 +18,11 @@
 #include <iostream>
 #include <string>
 #include <unordered_map>
+#ifdef _WIN32
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif // _WIN32
 
 #include <fastrtps/xmlparser/XMLParserCommon.h>
 #include <fastrtps/xmlparser/XMLTree.h>
@@ -43,7 +48,32 @@ namespace xmlparser {
 XMLP_ret XMLParser::loadDefaultXMLFile(
         up_base_node_t& root)
 {
-    return loadXML(DEFAULT_FASTRTPS_PROFILES, root);
+    // Use absolute path to ensure that the file is loaded only once
+#ifdef _WIN32
+    char current_directory[MAX_PATH];
+    if (GetCurrentDirectory(MAX_PATH, current_directory) == 0)
+    {
+        EPROSIMA_LOG_ERROR(XMLPARSER, "GetCurrentDirectory failed " << GetLastError());
+    }
+    else
+    {
+        strcat_s(current_directory, MAX_PATH, DEFAULT_FASTRTPS_PROFILES);
+        return loadXML(current_directory, root, true);
+    }
+#else
+    char current_directory[PATH_MAX];
+    if (getcwd(current_directory, PATH_MAX) == NULL)
+    {
+        EPROSIMA_LOG_ERROR(XMLPARSER, "getcwd failed " << std::strerror(errno));
+    }
+    else
+    {
+        strcat(current_directory, "/");
+        strcat(current_directory, DEFAULT_FASTRTPS_PROFILES);
+        return loadXML(current_directory, root, true);
+    }
+#endif // _WIN32
+    return XMLP_ret::XML_ERROR;
 }
 
 XMLP_ret XMLParser::parseXML(
@@ -1663,6 +1693,14 @@ XMLP_ret XMLParser::parseXMLConsumer(
 XMLP_ret XMLParser::loadXML(
         const std::string& filename,
         up_base_node_t& root)
+{
+    return loadXML(filename, root, false);
+}
+
+XMLP_ret XMLParser::loadXML(
+        const std::string& filename,
+        up_base_node_t& root,
+        bool is_default)
 {
     if (filename.empty())
     {
@@ -1673,7 +1711,7 @@ XMLP_ret XMLParser::loadXML(
     tinyxml2::XMLDocument xmlDoc;
     if (tinyxml2::XMLError::XML_SUCCESS != xmlDoc.LoadFile(filename.c_str()))
     {
-        if (filename != std::string(DEFAULT_FASTRTPS_PROFILES))
+        if (!is_default)
         {
             EPROSIMA_LOG_ERROR(XMLPARSER, "Error opening '" << filename << "'");
         }

diff --git a/src/cpp/rtps/xmlparser/XMLProfileManager.cpp b/src/cpp/rtps/xmlparser/XMLProfileManager.cpp
@@ -20,7 +20,9 @@
 #include <cstdlib>
 #ifdef _WIN32
 #include <windows.h>
-#endif // ifdef _WIN32
+#else
+#include <unistd.h>
+#endif // _WIN32
 
 using namespace eprosima::fastrtps;
 using namespace ::xmlparser;
@@ -166,10 +168,21 @@ void XMLProfileManager::loadDefaultXMLFile()
     // Try to load the default XML file set with an environment variable.
 #ifdef _WIN32
     char file_path[MAX_PATH];
+    char absolute_path[MAX_PATH];
+    char current_directory[MAX_PATH];
+    char** filename = {nullptr};
     size_t size = MAX_PATH;
     if (getenv_s(&size, file_path, size, DEFAULT_FASTRTPS_ENV_VARIABLE) == 0 && size > 0)
     {
-        loadXMLFile(file_path);
+        // Use absolute path to ensure the file is loaded only once
+        if (GetFullPathName(file_path, MAX_PATH, absolute_path, filename) == 0)
+        {
+            EPROSIMA_LOG_ERROR(XMLPARSER, "GetFullPathName failed " << GetLastError());
+        }
+        else
+        {
+            loadXMLFile(absolute_path);
+        }
     }
 
     // Should take into account '\0'
@@ -179,21 +192,49 @@ void XMLProfileManager::loadDefaultXMLFile()
     // Try to load the default XML file if variable does not exist or is not set to '1'
     if (!(getenv_s(&size, skip_xml, size, SKIP_DEFAULT_XML_FILE) == 0 && skip_xml[0] == '1'))
     {
-        loadXMLFile(DEFAULT_FASTRTPS_PROFILES);
+        // Try to load the default XML file.
+        if (GetCurrentDirectory(MAX_PATH, current_directory) == 0)
+        {
+            EPROSIMA_LOG_ERROR(XMLPARSER, "GetCurrentDirectory failed " << GetLastError());
+        }
+        else
+        {
+            strcat_s(current_directory, MAX_PATH, "\\");
+            strcat_s(current_directory, MAX_PATH, DEFAULT_FASTRTPS_PROFILES);
+            loadXMLFile(current_directory, true);
+        }
     }
 #else
+    char absolute_path[PATH_MAX];
 
     if (const char* file_path = std::getenv(DEFAULT_FASTRTPS_ENV_VARIABLE))
     {
-        loadXMLFile(file_path);
+        char* res = realpath(file_path, absolute_path);
+        if (res)
+        {
+            loadXMLFile(absolute_path);
+        }
+        else
+        {
+            EPROSIMA_LOG_ERROR(XMLPARSER, "realpath failed " << std::strerror(errno));
+        }
     }
 
     const char* skip_xml = std::getenv(SKIP_DEFAULT_XML_FILE);
 
     // Try to load the default XML file if variable does not exist or is not set to '1'
     if (!(skip_xml != nullptr && skip_xml[0] == '1'))
     {
-        loadXMLFile(DEFAULT_FASTRTPS_PROFILES);
+        if (getcwd(absolute_path, PATH_MAX) == NULL)
+        {
+            EPROSIMA_LOG_ERROR(XMLPARSER, "getcwd failed " << std::strerror(errno));
+        }
+        else
+        {
+            strcat(absolute_path, "/");
+            strcat(absolute_path, DEFAULT_FASTRTPS_PROFILES);
+            loadXMLFile(absolute_path, true);
+        }
     }
 
 #endif // ifdef _WIN32
@@ -279,6 +320,13 @@ XMLP_ret XMLProfileManager::loadXMLNode(
 
 XMLP_ret XMLProfileManager::loadXMLFile(
         const std::string& filename)
+{
+    return loadXMLFile(filename, false);
+}
+
+XMLP_ret XMLProfileManager::loadXMLFile(
+        const std::string& filename,
+        bool is_default)
 {
     if (filename.empty())
     {
@@ -294,10 +342,10 @@ XMLP_ret XMLProfileManager::loadXMLFile(
     }
 
     up_base_node_t root_node;
-    XMLP_ret loaded_ret = XMLParser::loadXML(filename, root_node);
+    XMLP_ret loaded_ret = XMLParser::loadXML(filename, root_node, is_default);
     if (!root_node || loaded_ret != XMLP_ret::XML_OK)
     {
-        if (filename != std::string(DEFAULT_FASTRTPS_PROFILES))
+        if (!is_default)
         {
             EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing '" << filename << "'");
         }

diff --git a/test/unittest/dds/status/CMakeLists.txt b/test/unittest/dds/status/CMakeLists.txt
@@ -152,6 +152,7 @@ target_compile_definitions(ListenerTests PRIVATE
     $<$<AND:$<NOT:$<BOOL:${WIN32}>>,$<STREQUAL:"${CMAKE_BUILD_TYPE}","Debug">>:__DEBUG>
     $<$<BOOL:${INTERNAL_DEBUG}>:__INTERNALDEBUG> # Internal debug activated.
     $<$<BOOL:${WIN32}>:_ENABLE_ATOMIC_ALIGNMENT_FIX>
+    $<$<BOOL:${MSVC}>:NOMINMAX> # avoid conflic with std::min & std::max in visual studio
     )
 target_include_directories(ListenerTests PRIVATE
     ${Asio_INCLUDE_DIR}

diff --git a/test/unittest/dynamic_types/CMakeLists.txt b/test/unittest/dynamic_types/CMakeLists.txt
@@ -110,6 +110,7 @@ add_executable(DynamicTypesTests ${DYNAMIC_TYPES_TEST_SOURCE})
 target_compile_definitions(DynamicTypesTests PRIVATE
     BOOST_ASIO_STANDALONE
     ASIO_STANDALONE
+    $<$<BOOL:${MSVC}>:NOMINMAX> # avoid conflict with std::min & std::max in visual studio
     $<$<AND:$<NOT:$<BOOL:${WIN32}>>,$<STREQUAL:"${CMAKE_BUILD_TYPE}","Debug">>:__DEBUG>
     $<$<BOOL:${INTERNAL_DEBUG}>:__INTERNALDEBUG> # Internal debug activated.
     )

diff --git a/test/unittest/logging/mock/MockConsumer.h b/test/unittest/logging/mock/MockConsumer.h
@@ -60,10 +60,20 @@ class MockConsumer : public StdoutConsumer
 
     template<typename Pred>
     void wait(
-            Pred pred)
+            Pred pred,
+            std::chrono::seconds timeout = std::chrono::seconds::zero())
     {
         std::unique_lock<std::mutex> lock(mMutex);
-        cv_.wait(lock, pred);
+
+        if (timeout == std::chrono::seconds::zero())
+        {
+            cv_.wait(lock, pred);
+        }
+        else
+        {
+            cv_.wait_for(lock, timeout, pred);
+        }
+
     }
 
     void wait_for_at_least_entries(
@@ -75,6 +85,16 @@ class MockConsumer : public StdoutConsumer
                        });
     }
 
+    void wait_for_at_least_entries_for(
+            size_t num_entries,
+            std::chrono::seconds timeout)
+    {
+        return wait([this, num_entries]() -> bool
+                       {
+                           return mEntriesConsumed.size() >= num_entries;
+                       }, timeout);
+    }
+
     void clear_entries()
     {
         std::unique_lock<std::mutex> guard(mMutex);