Ini file parser should check if the file can be opened

Check if the file can be accessed for reading or writing.
Change the function signatures accordingly, so that the
caller can act accordingly to the success of failure.

Pick-to: 3.0
Change-Id: I734499aa5858631076bd9a64735f0c6b74a0d61c
Reviewed-by: Arttu Tarkiainen <arttu.tarkiainen@qt.io>

5 files changed, 143 insertions, 75 deletions

diff --git a/src/libs/qlicensecore/inifileparser.cpp b/src/libs/qlicensecore/inifileparser.cpp
index fe4c699..c9629c1 100644
--- a/src/libs/qlicensecore/inifileparser.cpp
+++ b/src/libs/qlicensecore/inifileparser.cpp
@@ -6,6 +6,7 @@
 #include "inifileparser.h"
 
 #include "utils.h"
+#include "logger.h"
 
 #include <iostream>
 #include <fstream>
@@ -14,31 +15,47 @@
 
 namespace QLicenseCore {
 
-std::string IniFileParser::readKeyValue(const std::string &section, const std::string &key) const
+bool IniFileParser::readKeyValue(const std::string &section, const std::string &key,
+                                 std::string &value) const
 {
-    auto res = readSection(section);
-    auto it = res.find(key);
-    if (it != res.end())
-        return it->second;
-    else
-        return std::string();
+    std::map<std::string, std::string> result;
+    if (readSection(section, result)) {
+        auto it = result.find(key);
+        if (it != result.end()) {
+            value = it->second;
+            return true;
+        }
+    }
+
+    return false;
 }
 
-std::map<std::string, std::string> IniFileParser::readSection(const std::string &section) const
+bool IniFileParser::readSection(const std::string &section, std::map<std::string, std::string> &result) const
 {
-    return readSections()[section];
+    std::map<std::string, std::map<std::string, std::string>> results;
+    if (readSections(results)) {
+        result = results[section];
+        return true;
+    }
+
+    return false;
 }
 
-std::map<std::string, std::map<std::string, std::string>> IniFileParser::readSections() const
+bool IniFileParser::readSections(std::map<std::string, std::map<std::string, std::string>> &results) const
 {
 #if _WIN32
     std::ifstream file(utils::widen(m_filename));
 #else
     std::ifstream file(m_filename);
 #endif
+
+    if (!file.is_open()) {
+        logError("Unable to open file for reading: %s", m_filename.c_str());
+        return false;
+    }
+
     std::string line;
     std::string currentSection;
-    std::map<std::string, std::map<std::string, std::string>> results;
     while (std::getline(file, line)) {
         utils::trimStr(line);
         if (skipLine(line))
@@ -49,19 +66,17 @@ std::map<std::string, std::map<std::string, std::string>> IniFileParser::readSec
             results[currentSection] = std::map<std::string, std::string>();
             continue;
         } else {
-            size_t separatorPos = line.find('=');
-            if (separatorPos != std::string::npos) {
-                std::string key = line.substr(0, separatorPos);
-                std::string value = line.substr(separatorPos + 1);
-                results[currentSection][key] = value;
+            size_t pos = line.find('=');
+            if (pos != std::string::npos) {
+                results[currentSection][line.substr(0, pos)] = line.substr(pos + 1);
             }
         }
     }
 
-    return results;
+    return true;
 }
 
-void IniFileParser::writeKeyValue(const std::string &section,
+bool IniFileParser::writeKeyValue(const std::string &section,
                                   const std::map<std::string, std::string> &keyValuePairs) const
 {
 #if _WIN32
@@ -72,6 +87,16 @@ void IniFileParser::writeKeyValue(const std::string &section,
     std::ofstream outputFile(m_filename + ".tmp", std::ios::out | std::ios::trunc);
 #endif
 
+    if (utils::fileExists(m_filename) && !inputFile.is_open()) {
+        logError("Unable to write section '%s'. Can't open: %s", section.c_str(), m_filename.c_str());
+        return false;
+    }
+
+    if (!outputFile.is_open()) {
+        logError("Unable to write section '%s'. Can't open: %s.tmp", section.c_str(), m_filename.c_str());
+        return false;
+    }
+
     std::string line;
     bool sectionFound = false;
     bool sectionWritten = false;
@@ -118,6 +143,7 @@ void IniFileParser::writeKeyValue(const std::string &section,
     outputFile.close();
     std::remove(m_filename.c_str());
     std::rename((m_filename + ".tmp").c_str(), m_filename.c_str());
+    return true;
 }
 
 bool IniFileParser::removeSection(const std::string &section) const
@@ -130,6 +156,15 @@ bool IniFileParser::removeSection(const std::string &section) const
     std::ofstream outputFile(m_filename + ".tmp", std::ios::out | std::ios::trunc);
 #endif
 
+    if (utils::fileExists(m_filename) && !inputFile.is_open()) {
+        logError("Unable to remove section '%s'. Can't open: %s", section.c_str(), m_filename.c_str());
+        return false;
+    }
+
+    if (!outputFile.is_open()) {
+        logError("Unable to remove section '%s'. Can't open: %s.tmp", section.c_str(), m_filename.c_str());
+        return false;
+    }
 
     std::string line;
     bool sectionErased = false;
diff --git a/src/libs/qlicensecore/inifileparser.h b/src/libs/qlicensecore/inifileparser.h
index f912d31..ca61120 100644
--- a/src/libs/qlicensecore/inifileparser.h
+++ b/src/libs/qlicensecore/inifileparser.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2023 The Qt Company Ltd.
+/* Copyright (C) 2024 The Qt Company Ltd.
  *
  * SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
  */
@@ -16,10 +16,10 @@ class IniFileParser
 public:
     IniFileParser(const std::string &filename) : m_filename(filename) { }
 
-    std::string readKeyValue(const std::string &section, const std::string &key) const;
-    std::map<std::string, std::string> readSection(const std::string &section) const;
-    std::map<std::string, std::map<std::string, std::string>> readSections() const;
-    void writeKeyValue(const std::string &section,
+    bool readKeyValue(const std::string &section, const std::string &key, std::string &value) const;
+    bool readSection(const std::string &section, std::map<std::string, std::string> &result) const;
+    bool readSections(std::map<std::string, std::map<std::string, std::string>> &results) const;
+    bool writeKeyValue(const std::string &section,
                        const std::map<std::string, std::string> &keyValuePairs) const;
     bool removeSection(const std::string &section) const;
 
diff --git a/src/libs/qlicensecore/licdsetup.cpp b/src/libs/qlicensecore/licdsetup.cpp
index 7c0d190..b2cf966 100644
--- a/src/libs/qlicensecore/licdsetup.cpp
+++ b/src/libs/qlicensecore/licdsetup.cpp
@@ -279,7 +279,11 @@ std::string LicdSetup::parseMatchingServiceInstallation(const std::string &confi
                                                         const std::string &requiredMinimumVersion)
 {
     IniFileParser parser(configPath);
-    std::map<std::string, std::map<std::string, std::string>> sections = parser.readSections();
+    std::map<std::string, std::map<std::string, std::string>> sections;
+    if (!parser.readSections(sections)) {
+        logError("Unable to find License Service installation from: %s", configPath.c_str());
+        return "";
+    }
     std::map<std::string, std::string> suitableMatches;
     for (const auto &section : sections) {
         const std::string &foundVersion = sections[section.first]["version"];
@@ -323,26 +327,32 @@ void LicdSetup::registerInstallation(const std::string &installSource)
     const std::string programPath = utils::getProgramPath();
     const std::string formattedProgramPath = utils::replaceCharacters(programPath, ":/\\", '_');
     IniFileParser parser(installationsFile);
-    parser.writeKeyValue(formattedProgramPath,
-                         { { "path", programPath },
-                           { "source", installSource },
-                           { "version", DAEMON_VERSION },
-                           { "timestamp", currentTime } });
-    logInfo("%s registered into: %s by: %s", programPath.c_str(), installationsFile.c_str(),
-            installSource.c_str());
+    if (parser.writeKeyValue(formattedProgramPath,
+                             { { "path", programPath },
+                               { "source", installSource },
+                               { "version", DAEMON_VERSION },
+                               { "timestamp", currentTime } })) {
+        logInfo("%s registered into: %s by: %s", programPath.c_str(), installationsFile.c_str(),
+                installSource.c_str());
+    } else {
+        logError("Failed to register '%s' into: %s by: %s", programPath.c_str(),
+                 installationsFile.c_str(), installSource.c_str());
+    }
 }
 
 
 void LicdSetup::unregisterInstallation()
 {
-    const std::string installationsFile =
+    const std::string instFile =
             LicdSetup::getQtAppDataLocation() + USER_SETTINGS_FOLDER_NAME + DIR_SEPARATOR
                     + DAEMON_INSTALLATIONS_FILE;
-    IniFileParser parser(installationsFile);
+    IniFileParser parser(instFile);
     const std::string programPath = utils::getProgramPath();
     const std::string formattedProgramPath = utils::replaceCharacters(programPath, ":/\\", '_');
-    parser.removeSection(formattedProgramPath);
-    logInfo("%s unregistered from: %s", programPath.c_str(), installationsFile.c_str());
+    if (parser.removeSection(formattedProgramPath))
+        logInfo("%s unregistered from: %s", programPath.c_str(), instFile.c_str());
+    else
+        logError("Failed to unregister '%s' from: %s", programPath.c_str(), instFile.c_str());
 }
 
 
@@ -352,7 +362,10 @@ void LicdSetup::purgeInstallationsConfig(const std::string &path)
         return;
 
     IniFileParser parser(path);
-    auto sections = parser.readSections();
+    std::map<std::string, std::map<std::string, std::string>> sections;
+    if (!parser.readSections(sections))
+        return;
+
     for (const auto &section : sections) {
         const std::string &foundPath = sections[section.first]["path"];
         if (!utils::fileExists(foundPath)) {
diff --git a/tests/auto/licdsetup/tst_licdsetup.cpp b/tests/auto/licdsetup/tst_licdsetup.cpp
index 8bd45c3..b8100c3 100644
--- a/tests/auto/licdsetup/tst_licdsetup.cpp
+++ b/tests/auto/licdsetup/tst_licdsetup.cpp
@@ -24,34 +24,38 @@ TEST_CASE("Read matching service installation from installations.ini", "[licdset
     std::remove(TEST_INI_FILE);
     IniFileParser iniHandler(TEST_INI_FILE);
 
+    REQUIRE(
     iniHandler.writeKeyValue("_home_foo_bar1_bin_qtlicd", {
             {"path", "/home/foo/bar1/bin/qtlicd"},
             {"version", "3.1.0"}
-        });
+        }));
 
+    REQUIRE(
     iniHandler.writeKeyValue("_home_foo_bar2_bin_qtlicd", {
             {"path", "/home/foo/bar2/bin/qtlicd"},
             {"version", "3.0.1"}
-        });
+        }));
 
+    REQUIRE(
     iniHandler.writeKeyValue("_home_foo_bar4_bin_qtlicd", {
             {"path", "/home/foo/bar4/bin/qtlicd"},
             {"version", "3.2.0"}
-        });
+        }));
 
+    REQUIRE(
     iniHandler.writeKeyValue("_home_foo_bar5_bin_qtlicd", {
             {"path", "/home/foo/bar5/bin/qtlicd"},
             {"version", "2.1.0"}
-        });
+        }));
 
     REQUIRE(LicdSetup::parseMatchingServiceInstallation(TEST_INI_FILE, "3.0.0")
             == "/home/foo/bar4/bin/qtlicd");
 
-
+    REQUIRE(
     iniHandler.writeKeyValue("_home_foo_bar3_bin_qtlicd", {
             {"path", "/home/foo/bar3/bin/qtlicd"},
             {"version", "4.1.0"}
-        });
+        }));
 
     REQUIRE(LicdSetup::parseMatchingServiceInstallation(TEST_INI_FILE, "2.2.0")
             == "/home/foo/bar3/bin/qtlicd");
@@ -67,27 +71,32 @@ TEST_CASE("Remove orphans from installations.ini", "[licdsetup]")
     {
         IniFileParser iniHandler(TEST_INI_FILE);
 
+        REQUIRE(
         iniHandler.writeKeyValue("path_1", {
                 {"path", "/home/foo/bar1/bin/qtlicd"},
                 {"version", "3.1.0"}
-            });
+            }));
 
+        REQUIRE(
         iniHandler.writeKeyValue("path_2", {
                 {"path", MOCK_QTLICD_PATH},
                 {"version", "3.0.1"}
-            });
+            }));
 
+        REQUIRE(
         iniHandler.writeKeyValue("path_3", {
                 {"path", "/home/foo/bar4/bin/qtlicd"},
                 {"version", "3.2.0"}
-            });
+            }));
     }
 
     LicdSetup::purgeInstallationsConfig(TEST_INI_FILE);
 
     IniFileParser iniHandler(TEST_INI_FILE);
-    REQUIRE(iniHandler.readSection("path_1").empty() == true);
-    REQUIRE(iniHandler.readSection("path_2").empty() == false);
-    REQUIRE(iniHandler.readSection("path_3").empty() == true);
+    std::map<std::string, std::map<std::string, std::string>> results;
+    REQUIRE(iniHandler.readSections(results));
+    REQUIRE(results["path_1"].empty() == true);
+    REQUIRE(results["path_2"].empty() == false);
+    REQUIRE(results["path_3"].empty() == true);
 }
 
diff --git a/tests/auto/qinifileparser/tst_inifileparser.cpp b/tests/auto/qinifileparser/tst_inifileparser.cpp
index 320cd0f..817099e 100644
--- a/tests/auto/qinifileparser/tst_inifileparser.cpp
+++ b/tests/auto/qinifileparser/tst_inifileparser.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 2023 The Qt Company Ltd.
+/* Copyright (C) 2024 The Qt Company Ltd.
  *
  * SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 */
@@ -8,6 +8,7 @@
 
 #include <string>
 #include <iostream>
+#include <map>
 
 #include <inifileparser.h>
 
@@ -21,37 +22,47 @@ TEST_CASE("Read and write to .ini file", "[inifileparser]")
     std::remove(TEST_INI_FILE);
     IniFileParser iniHandler(TEST_INI_FILE);
 
-    std::string temp = iniHandler.readKeyValue("Section1", "Key1");
-    REQUIRE(iniHandler.readKeyValue("Section1", "Key1") == "");
+    std::string temp;
+    REQUIRE(!iniHandler.readKeyValue("Section1", "Key1", temp));
 
-    iniHandler.writeKeyValue("Section1", {{"Key1", "Value1"}});
-    REQUIRE(iniHandler.readKeyValue("Section1", "Key1") == "Value1");
+    REQUIRE(iniHandler.writeKeyValue("Section1", {{"Key1", "Value1"}}));
+    REQUIRE(iniHandler.readKeyValue("Section1", "Key1", temp));
+    REQUIRE(temp == "Value1");
 
-    iniHandler.writeKeyValue("Section2", {{"Key1", "Value1"}});
-    REQUIRE(iniHandler.readKeyValue("Section2", "Key1") == "Value1");
+    REQUIRE(iniHandler.writeKeyValue("Section2", {{"Key1", "Value1"}}));
+    REQUIRE(iniHandler.readKeyValue("Section2", "Key1", temp));
+    REQUIRE(temp == "Value1");
 
-    iniHandler.writeKeyValue("Section1", {{"Key2", "Value2"}});
-    REQUIRE(iniHandler.readKeyValue("Section1", "Key2") == "Value2");
+    REQUIRE(iniHandler.writeKeyValue("Section1", {{"Key2", "Value2"}}));
+    REQUIRE(iniHandler.readKeyValue("Section1", "Key2", temp));
+    REQUIRE(temp == "Value2");
 
-    iniHandler.writeKeyValue("Section1", {{"Key1", "Value3"}});
-    REQUIRE(iniHandler.readKeyValue("Section1", "Key1") == "Value3");
+    REQUIRE(iniHandler.writeKeyValue("Section1", {{"Key1", "Value3"}}));
+    REQUIRE(iniHandler.readKeyValue("Section1", "Key1", temp));
+    REQUIRE(temp == "Value3");
 
-    iniHandler.writeKeyValue("Section3", {{"Key1", "Value1"}, {"Key11", "Value11"}});
-    REQUIRE(iniHandler.readKeyValue("Section3", "Key1") == "Value1");
-    REQUIRE(iniHandler.readKeyValue("Section3", "Key11") == "Value11");
+    REQUIRE(iniHandler.writeKeyValue("Section3", {{"Key1", "Value1"}, {"Key11", "Value11"}}));
+    REQUIRE(iniHandler.readKeyValue("Section3", "Key1", temp));
+    REQUIRE(temp == "Value1");
+    REQUIRE(iniHandler.readKeyValue("Section3", "Key11", temp));
+    REQUIRE(temp == "Value11");
 
     REQUIRE(iniHandler.removeSection("Section1"));
-    REQUIRE(iniHandler.readKeyValue("Section1", "Key1") == "");
-    REQUIRE(iniHandler.readKeyValue("Section2", "Key1") == "Value1");
-    REQUIRE(iniHandler.readKeyValue("Section3", "Key1") == "Value1");
-
-    auto section = iniHandler.readSection("Section3");
-    REQUIRE(section["Key1"] == "Value1");
-    REQUIRE(section["Key11"] == "Value11");
-
-    auto allSections = iniHandler.readSections();
-    REQUIRE(allSections.size() == 2);
-    REQUIRE(allSections["Section2"]["Key1"] == "Value1");
-    REQUIRE(allSections["Section3"]["Key1"] == "Value1");
-    REQUIRE(allSections["Section3"]["Key11"] == "Value11");
+    REQUIRE(!iniHandler.readKeyValue("Section1", "Key1", temp));
+    REQUIRE(iniHandler.readKeyValue("Section2", "Key1", temp));
+    REQUIRE(temp == "Value1");
+    REQUIRE(iniHandler.readKeyValue("Section3", "Key1", temp));
+    REQUIRE(temp == "Value1");
+
+    std::map<std::string, std::string> result;
+    REQUIRE(iniHandler.readSection("Section3", result));
+    REQUIRE(result["Key1"] == "Value1");
+    REQUIRE(result["Key11"] == "Value11");
+
+    std::map<std::string, std::map<std::string, std::string>> results;
+    REQUIRE(iniHandler.readSections(results));
+    REQUIRE(results.size() == 2);
+    REQUIRE(results["Section2"]["Key1"] == "Value1");
+    REQUIRE(results["Section3"]["Key1"] == "Value1");
+    REQUIRE(results["Section3"]["Key11"] == "Value11");
 }