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>
eProsima · May 22, 2024 · 0919ff2 · 0919ff2
1 parent 2552fa9
commit 0919ff2
Show file tree

Hide file tree

Showing 10 changed files with 377 additions and 18 deletions.
diff --git a/src/cpp/xmlparser/XMLParser.cpp b/src/cpp/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 <tinyxml2.h>
 
@@ -49,7 +54,32 @@ using namespace eprosima::fastdds::xml::detail;
 XMLP_ret XMLParser::loadDefaultXMLFile(
         up_base_node_t& root)
 {
-    return loadXML(DEFAULT_FASTDDS_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_FASTDDS_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_FASTDDS_PROFILES);
+        return loadXML(current_directory, root, true);
+    }
+#endif // _WIN32
+    return XMLP_ret::XML_ERROR;
 }
 
 XMLP_ret XMLParser::parseXML(
@@ -421,7 +451,7 @@ XMLP_ret XMLParser::validateXMLTransportElements(
                 strcmp(name, MAX_INITIAL_PEERS_RANGE) == 0 ||
                 strcmp(name, WHITE_LIST) == 0 ||
                 strcmp(name, NETMASK_FILTER) == 0 ||
-                strcmp(name, INTERFACES) == 0 ||
+                strcmp(name, NETWORK_INTERFACES) == 0 ||
                 strcmp(name, TTL) == 0 ||
                 strcmp(name, NON_BLOCKING_SEND) == 0 ||
                 strcmp(name, UDP_OUTPUT_PORT) == 0 ||
@@ -606,7 +636,7 @@ XMLP_ret XMLParser::parseXMLSocketTransportData(
                     p_aux1 != nullptr; p_aux1 = p_aux1->NextSiblingElement())
             {
                 address = p_aux1->Name();
-                if (strcmp(address, ADDRESS) == 0 || strcmp(address, INTERFACE) == 0)
+                if (strcmp(address, ADDRESS) == 0 || strcmp(address, NETWORK_INTERFACE) == 0)
                 {
                     std::string text = get_element_text(p_aux1);
                     if (!text.empty())
@@ -641,7 +671,7 @@ XMLP_ret XMLParser::parseXMLSocketTransportData(
                 return XMLP_ret::XML_ERROR;
             }
         }
-        else if (strcmp(name, INTERFACES) == 0)
+        else if (strcmp(name, NETWORK_INTERFACES) == 0)
         {
             if (XMLP_ret::XML_OK != parseXMLInterfaces(p_aux0, p_transport))
             {
@@ -2057,6 +2087,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())
     {
@@ -2067,7 +2105,7 @@ XMLP_ret XMLParser::loadXML(
     tinyxml2::XMLDocument xmlDoc;
     if (tinyxml2::XMLError::XML_SUCCESS != xmlDoc.LoadFile(filename.c_str()))
     {
-        if (filename != std::string(DEFAULT_FASTDDS_PROFILES))
+        if (!is_default)
         {
             EPROSIMA_LOG_ERROR(XMLPARSER, "Error opening '" << filename << "'");
         }

diff --git a/src/cpp/xmlparser/XMLParser.h b/src/cpp/xmlparser/XMLParser.h
@@ -121,6 +121,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/src/cpp/xmlparser/XMLParserCommon.cpp b/src/cpp/xmlparser/XMLParserCommon.cpp
@@ -48,9 +48,9 @@ const char* SEND_BUFFER_SIZE = "sendBufferSize";
 const char* TTL = "TTL";
 const char* NON_BLOCKING_SEND = "non_blocking_send";
 const char* WHITE_LIST = "interfaceWhiteList";
-const char* INTERFACE = "interface";
+const char* NETWORK_INTERFACE = "interface";
 const char* NETMASK_FILTER = "netmask_filter";
-const char* INTERFACES = "interfaces";
+const char* NETWORK_INTERFACES = "interfaces";
 const char* ALLOWLIST = "allowlist";
 const char* BLOCKLIST = "blocklist";
 const char* MAX_MESSAGE_SIZE = "maxMessageSize";

diff --git a/src/cpp/xmlparser/XMLParserCommon.h b/src/cpp/xmlparser/XMLParserCommon.h
@@ -61,9 +61,9 @@ extern const char* SEND_BUFFER_SIZE;
 extern const char* TTL;
 extern const char* NON_BLOCKING_SEND;
 extern const char* WHITE_LIST;
-extern const char* INTERFACE;
+extern const char* NETWORK_INTERFACE;
 extern const char* NETMASK_FILTER;
-extern const char* INTERFACES;
+extern const char* NETWORK_INTERFACES;
 extern const char* ALLOWLIST;
 extern const char* BLOCKLIST;
 extern const char* MAX_MESSAGE_SIZE;

diff --git a/src/cpp/xmlparser/XMLProfileManager.cpp b/src/cpp/xmlparser/XMLProfileManager.cpp
@@ -17,7 +17,9 @@
 #include <cstdlib>
 #ifdef _WIN32
 #include <windows.h>
-#endif // ifdef _WIN32
+#else
+#include <unistd.h>
+#endif // _WIN32
 
 #include <tinyxml2.h>
 
@@ -197,10 +199,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_FASTDDS_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'
@@ -210,21 +223,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_FASTDDS_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_FASTDDS_PROFILES);
+            loadXMLFile(current_directory, true);
+        }
     }
 #else
+    char absolute_path[PATH_MAX];
 
     if (const char* file_path = std::getenv(DEFAULT_FASTDDS_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_FASTDDS_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_FASTDDS_PROFILES);
+            loadXMLFile(absolute_path, true);
+        }
     }
 
 #endif // ifdef _WIN32
@@ -310,6 +351,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())
     {
@@ -325,10 +373,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_FASTDDS_PROFILES))
+        if (!is_default)
         {
             EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing '" << filename << "'");
         }

diff --git a/src/cpp/xmlparser/XMLProfileManager.h b/src/cpp/xmlparser/XMLProfileManager.h
@@ -73,6 +73,18 @@ class XMLProfileManager
     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/test/unittest/dds/status/CMakeLists.txt b/test/unittest/dds/status/CMakeLists.txt
@@ -136,6 +136,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/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);