Merge #15335: Fix lack of warning of unrecognized section names

1a7ba84e11 Fix lack of warning of unrecognized section names (Akio Nakamura)

Pull request description:

  In #14708, It was introduced that to warn when unrecognized section names are exist in the config file.
  But ```m_config_sections.clear()```  in ```ArgsManager::ReadConfigStream()``` is called every time when reading each configuration file, so it can warn about only last reading file if ```includeconf``` exists.

  This PR fix lack of warning by collecting all section names by moving ```m_config_sections.clear()```  to ```ArgsManager::ReadConfigFiles()``` .
  Also add a test code to confirm this situation.

Tree-SHA512: 26aa0cbe3e4ae2e58cbe73d4492ee5cf465fd4c3e5df2c8ca7e282b627df9e637267af1e3816386b1dc6db2398b31936925ce0e432219fec3a9b3398f01e3e65

5 files changed, 36 insertions, 24 deletions

diff --git a/src/init.cpp b/src/init.cpp
index 8876b54fde..e0da962030 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -839,7 +839,7 @@ void InitParameterInteraction()
 
     // Warn if unrecognized section name are present in the config file.
     for (const auto& section : gArgs.GetUnrecognizedSections()) {
-        InitWarning(strprintf(_("Section [%s] is not recognized."), section));
+        InitWarning(strprintf("%s:%i " + _("Section [%s] is not recognized."), section.m_file, section.m_line, section.m_name));
     }
 }
 
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 860f64bb11..e17ae7103b 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -180,9 +180,10 @@ struct TestArgsManager : public ArgsManager
         {
             LOCK(cs_args);
             m_config_args.clear();
+            m_config_sections.clear();
         }
         std::string error;
-        BOOST_REQUIRE(ReadConfigStream(streamConfig, error));
+        BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
     }
     void SetNetworkOnlyArg(const std::string arg)
     {
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 6e82de743a..5ae0604894 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -353,8 +353,7 @@ const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
     return unsuitables;
 }
 
-
-const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
+const std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const
 {
     // Section names to be recognized in the config file.
     static const std::set<std::string> available_sections{
@@ -362,14 +361,11 @@ const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
         CBaseChainParams::TESTNET,
         CBaseChainParams::MAIN
     };
-    std::set<std::string> diff;
 
     LOCK(cs_args);
-    std::set_difference(
-        m_config_sections.begin(), m_config_sections.end(),
-        available_sections.begin(), available_sections.end(),
-        std::inserter(diff, diff.end()));
-    return diff;
+    std::list<SectionInfo> unrecognized = m_config_sections;
+    unrecognized.remove_if([](const SectionInfo& appeared){ return available_sections.find(appeared.m_name) != available_sections.end(); });
+    return unrecognized;
 }
 
 void ArgsManager::SelectConfigNetwork(const std::string& network)
@@ -793,7 +789,7 @@ static std::string TrimString(const std::string& str, const std::string& pattern
     return str.substr(front, end - front + 1);
 }
 
-static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::set<std::string>& sections)
+static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::list<SectionInfo>& sections)
 {
     std::string str, prefix;
     std::string::size_type pos;
@@ -809,7 +805,7 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
         if (!str.empty()) {
             if (*str.begin() == '[' && *str.rbegin() == ']') {
                 const std::string section = str.substr(1, str.size() - 2);
-                sections.insert(section);
+                sections.emplace_back(SectionInfo{section, filepath, linenr});
                 prefix = section + '.';
             } else if (*str.begin() == '-') {
                 error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str);
@@ -822,8 +818,8 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
                     return false;
                 }
                 options.emplace_back(name, value);
-                if ((pos = name.rfind('.')) != std::string::npos) {
-                    sections.insert(name.substr(0, pos));
+                if ((pos = name.rfind('.')) != std::string::npos && prefix.length() <= pos) {
+                    sections.emplace_back(SectionInfo{name.substr(0, pos), filepath, linenr});
                 }
             } else {
                 error = strprintf("parse error on line %i: %s", linenr, str);
@@ -838,12 +834,11 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
     return true;
 }
 
-bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
+bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
 {
     LOCK(cs_args);
     std::vector<std::pair<std::string, std::string>> options;
-    m_config_sections.clear();
-    if (!GetConfigOptions(stream, error, options, m_config_sections)) {
+    if (!GetConfigOptions(stream, filepath, error, options, m_config_sections)) {
         return false;
     }
     for (const std::pair<std::string, std::string>& option : options) {
@@ -874,6 +869,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
     {
         LOCK(cs_args);
         m_config_args.clear();
+        m_config_sections.clear();
     }
 
     const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
@@ -881,7 +877,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
 
     // ok to not have a config file
     if (stream.good()) {
-        if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
+        if (!ReadConfigStream(stream, confPath, error, ignore_invalid_keys)) {
             return false;
         }
         // if there is an -includeconf in the override args, but it is empty, that means the user
@@ -912,7 +908,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
             for (const std::string& to_include : includeconf) {
                 fsbridge::ifstream include_config(GetConfigFile(to_include));
                 if (include_config.good()) {
-                    if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
+                    if (!ReadConfigStream(include_config, to_include, error, ignore_invalid_keys)) {
                         return false;
                     }
                     LogPrintf("Included configuration file %s\n", to_include.c_str());
diff --git a/src/util/system.h b/src/util/system.h
index 69ae11d1ed..6899e38c9e 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -127,6 +127,13 @@ enum class OptionsCategory {
     HIDDEN // Always the last option to avoid printing these in the help
 };
 
+struct SectionInfo
+{
+    std::string m_name;
+    std::string m_file;
+    int m_line;
+};
+
 class ArgsManager
 {
 protected:
@@ -147,9 +154,9 @@ protected:
     std::string m_network GUARDED_BY(cs_args);
     std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
     std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
-    std::set<std::string> m_config_sections GUARDED_BY(cs_args);
+    std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
 
-    NODISCARD bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
+    NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
 
 public:
     ArgsManager();
@@ -173,7 +180,7 @@ public:
     /**
      * Log warnings for unrecognized section names in the config file.
      */
-    const std::set<std::string> GetUnrecognizedSections() const;
+    const std::list<SectionInfo> GetUnrecognizedSections() const;
 
     /**
      * Return a vector of strings of the given argument
diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index 4b3f6603a2..460e664c48 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -41,13 +41,21 @@ class ConfArgsTest(BitcoinTestFramework):
             conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided')
 
+        inc_conf_file2_path = os.path.join(self.nodes[0].datadir, 'include2.conf')
+        with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
+            conf.write('includeconf={}\n'.format(inc_conf_file2_path))
+
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
-            conf.write('testnot.datadir=1\n[testnet]\n')
+            conf.write('testnot.datadir=1\n')
+        with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
+            conf.write('[testnet]\n')
         self.restart_node(0)
-        self.nodes[0].stop_node(expected_stderr='Warning: Section [testnet] is not recognized.' + os.linesep + 'Warning: Section [testnot] is not recognized.')
+        self.nodes[0].stop_node(expected_stderr='Warning: ' + inc_conf_file_path + ':1 Section [testnot] is not recognized.' + os.linesep + 'Warning: ' + inc_conf_file2_path + ':1 Section [testnet] is not recognized.')
 
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('')  # clear
+        with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
+            conf.write('')  # clear
 
     def run_test(self):
         self.stop_node(0)