Fix lack of warning of unrecognized section names
1. Fix lack of warning by collecting all section names by moving m_config_sections.clear() to ArgsManager::ReadConfigFiles(). 2. Add info(file name, line number) to warning message. 3. Add a test code to confirm this situation. 3. Do clear() in ReadConfigString().
This commit is contained in:
parent
904308dca3
commit
1a7ba84e11
5 changed files with 38 additions and 26 deletions
|
@ -815,7 +815,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));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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)
|
||||
{
|
||||
|
|
|
@ -354,8 +354,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{
|
||||
|
@ -363,14 +362,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)
|
||||
|
@ -794,7 +790,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;
|
||||
|
@ -810,7 +806,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);
|
||||
|
@ -823,8 +819,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);
|
||||
|
@ -839,12 +835,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) {
|
||||
|
@ -875,6 +870,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);
|
||||
|
@ -882,7 +878,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
|
||||
|
@ -913,7 +909,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());
|
||||
|
|
|
@ -132,6 +132,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:
|
||||
|
@ -152,9 +159,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();
|
||||
|
@ -178,7 +185,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
|
||||
|
|
|
@ -41,12 +41,20 @@ 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')
|
||||
|
||||
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.')
|
||||
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')
|
||||
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: ' + 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):
|
||||
|
|
Loading…
Reference in a new issue