Merge bitcoin#14708: Warn unrecognised sections in the config file

3fb09b9 Warn unrecognized sections in the config file (Akio Nakamura)

Pull request description:

  This PR intends to resolve bitcoin#14702.

  In the config file, sections are specified by square bracket pair "[]"$,
  or included in the option name itself which separated by a period"(.)".

  Typicaly, [testnet] is not a correct section name and specified options
  in that section are ignored but user cannot recognize what is happen.

  So, add some log-warning messages if unrecognized section names are
  present in the config file after checking section only args.

  note: Currentry, followings are out of scope of this PR.
  1) Empty section name or option name can describe.
  e.g. [] , .a=b, =c
  2) Multiple period characters can exist in the section name and option name.
  e.g. [c.d.e], [..], f.g.h.i=j, ..=k

Tree-SHA512: 2cea02a0525feb40320613989a75cd7b7b1bd12158d5e6f3174ca77e6a25bb84425dd8812f62483df9fc482045c7b5402d69bc714430518b1847d055a2dc304b

Author: laanwj

src/init.cpp

@@ -1124,7 +1124,15 @@ void InitParameterInteraction()
     // Warn if network-specific options (-addnode, -connect, etc) are
     // specified in default section of config file, but not overridden
     // on the command line or in this network's section of the config file.
-    gArgs.WarnForSectionOnlyArgs();
+    std::string network = gArgs.GetChainName();
+    for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
+        InitWarning(strprintf(_("Config setting for %s only applied on %s network when in [%s] section."), arg, network, network));
+    }
+
+    // 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));
+    }
 }
 
 static std::string ResolveErrMsg(const char * const optname, const std::string& strBind)

src/util/system.cpp

@@ -400,15 +400,17 @@ ArgsManager::ArgsManager() :
     // nothing to do
 }
 
-void ArgsManager::WarnForSectionOnlyArgs()
+const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
 {
+    std::set<std::string> unsuitables;
+
     LOCK(cs_args);
 
     // if there's no section selected, don't worry
-    if (m_network.empty()) return;
+    if (m_network.empty()) return std::set<std::string> {};
 
     // if it's okay to use the default section for this network, don't worry
-    if (m_network == CBaseChainParams::MAIN) return;
+    if (m_network == CBaseChainParams::MAIN) return std::set<std::string> {};
 
     for (const auto& arg : m_network_only_args) {
         std::pair<bool, std::string> found_result;
@@ -426,8 +428,29 @@ void ArgsManager::WarnForSectionOnlyArgs()
         if (!found_result.first) continue;
 
         // otherwise, issue a warning
-        LogPrintf("Warning: Config setting for %s only applied on %s network when in [%s] section.\n", arg, m_network, m_network);
+        unsuitables.insert(arg);
     }
+    return unsuitables;
+}
+
+
+const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
+{
+    // Section names to be recognized in the config file.
+    static const std::set<std::string> available_sections{
+        CBaseChainParams::REGTEST,
+        CBaseChainParams::TESTNET,
+        CBaseChainParams::MAIN,
+        CBaseChainParams::DEVNET
+    };
+    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;
 }
 
 void ArgsManager::SelectConfigNetwork(const std::string& network)
@@ -888,7 +911,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)
+static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::set<std::string>& sections)
 {
     std::string str, prefix;
     std::string::size_type pos;
@@ -903,7 +926,9 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
         str = TrimString(str, pattern);
         if (!str.empty()) {
             if (*str.begin() == '[' && *str.rbegin() == ']') {
-                prefix = str.substr(1, str.size() - 2) + '.';
+                const std::string section = str.substr(1, str.size() - 2);
+                sections.insert(section);
+                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);
                 return false;
@@ -915,6 +940,9 @@ 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));
+                }
             } else {
                 error = strprintf("parse error on line %i: %s", linenr, str);
                 if (str.size() >= 2 && str.substr(0, 2) == "no") {
@@ -932,7 +960,8 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, boo
 {
     LOCK(cs_args);
     std::vector<std::pair<std::string, std::string>> options;
-    if (!GetConfigOptions(stream, error, options)) {
+    m_config_sections.clear();
+    if (!GetConfigOptions(stream, error, options, m_config_sections)) {
         return false;
     }
     for (const std::pair<std::string, std::string>& option : options) {

src/util/system.h

@@ -178,6 +178,7 @@ class ArgsManager
     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);
 
     [[nodiscard]] bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
 
@@ -198,7 +199,12 @@ class ArgsManager
      * on the command line or in a network-specific section in the
      * config file.
      */
-    void WarnForSectionOnlyArgs();
+    const std::set<std::string> GetUnsuitableSectionOnlyArgs() const;
+
+    /**
+     * Log warnings for unrecognized section names in the config file.
+     */
+    const std::set<std::string> GetUnrecognizedSections() const;
 
     /**
      * Return a vector of strings of the given argument

test/functional/feature_config_args.py

@@ -43,6 +43,11 @@ def test_config_file_parser(self):
             conf.write('server=1\nrpcuser=someuser\nrpcpassword=some#pass')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 3, using # in rpcpassword can be ambiguous and should be avoided')
 
+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('testnot.datadir=1\n[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.')
+
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('')  # clear