More fuzzing2 (#1170)

add mechanic for fuzzing subcommands, and several fixes for found issues
from longer fuzzer runs

This includes some issues with option group positional name ambiguity, issue with join multioption policy and config files, and a few edge cases for configuration of multiline output interpretation.

Also added complex variables to the options, no issues found from this addition. 

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

include/CLI/Option.hpp

@@ -34,6 +34,7 @@ using callback_t = std::function<bool(const results_t &)>;
 
 class Option;
 class App;
+class ConfigBase;
 
 using Option_p = std::unique_ptr<Option>;
 /// Enumeration of the multiOption Policy selection
@@ -51,6 +52,7 @@ enum class MultiOptionPolicy : char {
 /// to share parts of the class; an OptionDefaults can copy to an Option.
 template <typename CRTP> class OptionBase {
     friend App;
+    friend ConfigBase;
 
   protected:
     /// The group membership
@@ -230,6 +232,7 @@ class OptionDefaults : public OptionBase<OptionDefaults> {
 
 class Option : public OptionBase<Option> {
     friend App;
+    friend ConfigBase;
 
   protected:
     /// @name Names

include/CLI/StringTools.hpp

@@ -263,7 +263,10 @@ CLI11_INLINE bool is_binary_escaped_string(const std::string &escaped_string);
 CLI11_INLINE std::string extract_binary_string(const std::string &escaped_string);
 
 /// process a quoted string, remove the quotes and if appropriate handle escaped characters
-CLI11_INLINE bool process_quoted_string(std::string &str, char string_char = '\"', char literal_char = '\'');
+CLI11_INLINE bool process_quoted_string(std::string &str,
+                                        char string_char = '\"',
+                                        char literal_char = '\'',
+                                        bool disable_secondary_array_processing = false);
 
 /// This function formats the given text as a paragraph with fixed width and applies correct line wrapping
 /// with a custom line prefix. The paragraph will get streamed to the given ostream.

include/CLI/impl/App_inl.hpp

@@ -165,94 +165,103 @@ CLI11_INLINE Option *App::add_option(std::string option_name,
                                      std::function<std::string()> func) {
     Option myopt{option_name, option_description, option_callback, this, allow_non_standard_options_};
 
-    if(std::find_if(std::begin(options_), std::end(options_), [&myopt](const Option_p &v) { return *v == myopt; }) ==
-       std::end(options_)) {
-        if(myopt.lnames_.empty() && myopt.snames_.empty()) {
-            // if the option is positional only there is additional potential for ambiguities in config files and needs
-            // to be checked
-            std::string test_name = "--" + myopt.get_single_name();
-            if(test_name.size() == 3) {
-                test_name.erase(0, 1);
-            }
+    // do a quick search in current subcommand for options
+    auto res =
+        std::find_if(std::begin(options_), std::end(options_), [&myopt](const Option_p &v) { return *v == myopt; });
+    if(res != options_.end()) {
+        const auto &matchname = (*res)->matching_name(myopt);
+        throw(OptionAlreadyAdded("added option matched existing option name: " + matchname));
+    }
+    /** get a top level parent*/
+    const App *top_level_parent = this;
+    while(top_level_parent->name_.empty() && top_level_parent->parent_ != nullptr) {
+        top_level_parent = top_level_parent->parent_;
+    }
 
-            auto *op = get_option_no_throw(test_name);
+    if(myopt.lnames_.empty() && myopt.snames_.empty()) {
+        // if the option is positional only there is additional potential for ambiguities in config files and needs
+        // to be checked
+        std::string test_name = "--" + myopt.get_single_name();
+        if(test_name.size() == 3) {
+            test_name.erase(0, 1);
+        }
+        // if we are in option group
+        const auto *op = top_level_parent->get_option_no_throw(test_name);
+        if(op != nullptr && op->get_configurable()) {
+            throw(OptionAlreadyAdded("added option positional name matches existing option: " + test_name));
+        }
+        // need to check if there is another positional with the same name that also doesn't have any long or
+        // short names
+        op = top_level_parent->get_option_no_throw(myopt.get_single_name());
+        if(op != nullptr && op->lnames_.empty() && op->snames_.empty()) {
+            throw(OptionAlreadyAdded("unable to disambiguate with existing option: " + test_name));
+        }
+    } else if(top_level_parent != this) {
+        for(auto &ln : myopt.lnames_) {
+            const auto *op = top_level_parent->get_option_no_throw(ln);
             if(op != nullptr && op->get_configurable()) {
-                throw(OptionAlreadyAdded("added option positional name matches existing option: " + test_name));
+                throw(OptionAlreadyAdded("added option matches existing positional option: " + ln));
             }
-            // need to check if there is another positional with the same name that also doesn't have any long or short
-            // names
-            op = get_option_no_throw(myopt.get_single_name());
-            if(op != nullptr && op->lnames_.empty() && op->snames_.empty()) {
-                throw(OptionAlreadyAdded("unable to disambiguate with existing option: " + test_name));
+            op = top_level_parent->get_option_no_throw("--" + ln);
+            if(op != nullptr && op->get_configurable()) {
+                throw(OptionAlreadyAdded("added option matches existing option: --" + ln));
             }
-        } else if(parent_ != nullptr) {
-            for(auto &ln : myopt.lnames_) {
-                auto *op = parent_->get_option_no_throw(ln);
-                if(op != nullptr && op->get_configurable()) {
-                    throw(OptionAlreadyAdded("added option matches existing positional option: " + ln));
-                }
+        }
+        for(auto &sn : myopt.snames_) {
+            const auto *op = top_level_parent->get_option_no_throw(sn);
+            if(op != nullptr && op->get_configurable()) {
+                throw(OptionAlreadyAdded("added option matches existing positional option: " + sn));
             }
-            for(auto &sn : myopt.snames_) {
-                auto *op = parent_->get_option_no_throw(sn);
-                if(op != nullptr && op->get_configurable()) {
-                    throw(OptionAlreadyAdded("added option matches existing positional option: " + sn));
+            op = top_level_parent->get_option_no_throw("-" + sn);
+            if(op != nullptr && op->get_configurable()) {
+                throw(OptionAlreadyAdded("added option matches existing option: -" + sn));
+            }
+        }
+    }
+    if(allow_non_standard_options_ && !myopt.snames_.empty()) {
+
+        for(auto &sname : myopt.snames_) {
+            if(sname.length() > 1) {
+                std::string test_name;
+                test_name.push_back('-');
+                test_name.push_back(sname.front());
+                const auto *op = top_level_parent->get_option_no_throw(test_name);
+                if(op != nullptr) {
+                    throw(OptionAlreadyAdded("added option interferes with existing short option: " + sname));
                 }
             }
         }
-        if(allow_non_standard_options_ && !myopt.snames_.empty()) {
-            for(auto &sname : myopt.snames_) {
-                if(sname.length() > 1) {
+        for(auto &opt : top_level_parent->get_options()) {
+            for(const auto &osn : opt->snames_) {
+                if(osn.size() > 1) {
                     std::string test_name;
-                    test_name.push_back('-');
-                    test_name.push_back(sname.front());
-                    auto *op = get_option_no_throw(test_name);
-                    if(op != nullptr) {
-                        throw(OptionAlreadyAdded("added option interferes with existing short option: " + sname));
-                    }
-                }
-            }
-            for(auto &opt : options_) {
-                for(const auto &osn : opt->snames_) {
-                    if(osn.size() > 1) {
-                        std::string test_name;
-                        test_name.push_back(osn.front());
-                        if(myopt.check_sname(test_name)) {
-                            throw(OptionAlreadyAdded("added option interferes with existing non standard option: " +
-                                                     osn));
-                        }
+                    test_name.push_back(osn.front());
+                    if(myopt.check_sname(test_name)) {
+                        throw(OptionAlreadyAdded("added option interferes with existing non standard option: " + osn));
                     }
                 }
             }
         }
-        options_.emplace_back();
-        Option_p &option = options_.back();
-        option.reset(new Option(option_name, option_description, option_callback, this, allow_non_standard_options_));
-
-        // Set the default string capture function
-        option->default_function(func);
-
-        // For compatibility with CLI11 1.7 and before, capture the default string here
-        if(defaulted)
-            option->capture_default_str();
-
-        // Transfer defaults to the new option
-        option_defaults_.copy_to(option.get());
-
-        // Don't bother to capture if we already did
-        if(!defaulted && option->get_always_capture_default())
-            option->capture_default_str();
-
-        return option.get();
     }
-    // we know something matches now find what it is so we can produce more error information
-    for(auto &opt : options_) {
-        const auto &matchname = opt->matching_name(myopt);
-        if(!matchname.empty()) {
-            throw(OptionAlreadyAdded("added option matched existing option name: " + matchname));
-        }
-    }
-    // this line should not be reached the above loop should trigger the throw
-    throw(OptionAlreadyAdded("added option matched existing option name"));  // LCOV_EXCL_LINE
+    options_.emplace_back();
+    Option_p &option = options_.back();
+    option.reset(new Option(option_name, option_description, option_callback, this, allow_non_standard_options_));
+
+    // Set the default string capture function
+    option->default_function(func);
+
+    // For compatibility with CLI11 1.7 and before, capture the default string here
+    if(defaulted)
+        option->capture_default_str();
+
+    // Transfer defaults to the new option
+    option_defaults_.copy_to(option.get());
+
+    // Don't bother to capture if we already did
+    if(!defaulted && option->get_always_capture_default())
+        option->capture_default_str();
+
+    return option.get();
 }
 
 CLI11_INLINE Option *App::set_help_flag(std::string flag_name, const std::string &help_description) {
@@ -841,8 +850,8 @@ CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Opt
             std::end(options));
     }
     for(auto &subc : subcommands_) {
-        // also check down into nameless subcommands
-        if(subc->get_name().empty() && !subc->get_group().empty() && subc->get_group().front() == '+') {
+        // also check down into nameless subcommands and specific groups
+        if(subc->get_name().empty() || (!subc->get_group().empty() && subc->get_group().front() == '+')) {
             auto subcopts = subc->get_options(filter);
             options.insert(options.end(), subcopts.begin(), subcopts.end());
         }
@@ -1539,7 +1548,8 @@ App::_add_flag_like_result(Option *op, const ConfigItem &item, const std::vector
         return true;
     }
     if(static_cast<int>(inputs.size()) > op->get_items_expected_max() &&
-       op->get_multi_option_policy() != MultiOptionPolicy::TakeAll) {
+       op->get_multi_option_policy() != MultiOptionPolicy::TakeAll &&
+       op->get_multi_option_policy() != MultiOptionPolicy::Join) {
         if(op->get_items_expected_max() > 1) {
             throw ArgumentMismatch::AtMost(item.fullname(), op->get_items_expected_max(), inputs.size());
         }
@@ -1608,11 +1618,6 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
         }
         if(op == nullptr) {
             op = get_option_no_throw(item.name);
-        } else if(!op->get_configurable()) {
-            auto *testop = get_option_no_throw(item.name);
-            if(testop != nullptr && testop->get_configurable()) {
-                op = testop;
-            }
         }
     } else if(!op->get_configurable()) {
         if(item.name.size() == 1) {
@@ -1621,14 +1626,17 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
                 op = testop;
             }
         }
-        if(!op->get_configurable()) {
-            auto *testop = get_option_no_throw(item.name);
-            if(testop != nullptr && testop->get_configurable()) {
-                op = testop;
-            }
+    }
+    if(op == nullptr || !op->get_configurable()) {
+        std::string iname = item.name;
+        auto options = get_options([iname](const CLI::Option *opt) {
+            return (opt->get_configurable() &&
+                    (opt->check_name(iname) || opt->check_lname(iname) || opt->check_sname(iname)));
+        });
+        if(!options.empty()) {
+            op = options[0];
         }
     }
-
     if(op == nullptr) {
         // If the option was not present
         if(get_allow_config_extras() == config_extras_mode::capture) {
@@ -1785,7 +1793,9 @@ CLI11_INLINE bool App::_parse_positional(std::vector<std::string> &args, bool ha
                 posOpt->add_result(std::string{});
             }
         }
+        results_t prev;
         if(posOpt->get_trigger_on_parse() && posOpt->current_option_state_ == Option::option_state::callback_run) {
+            prev = posOpt->results();
             posOpt->clear();
         }
         if(posOpt->get_expected_min() == 0) {
@@ -1801,6 +1811,10 @@ CLI11_INLINE bool App::_parse_positional(std::vector<std::string> &args, bool ha
         if(posOpt->get_trigger_on_parse()) {
             if(!posOpt->empty()) {
                 posOpt->run_callback();
+            } else {
+                if(!prev.empty()) {
+                    posOpt->add_result(prev);
+                }
             }
         }
 

include/CLI/impl/Config_inl.hpp

@@ -93,6 +93,9 @@ convert_arg_for_ini(const std::string &arg, char stringQuote, char literalQuote,
                 return_string.push_back('\n');
             }
             return_string.append(arg);
+            if(arg.back() == '\n' || arg.back() == '\r') {
+                return_string.push_back('\n');
+            }
             return_string.append(multiline_literal_quote, 3);
             return return_string;
         }
@@ -349,7 +352,7 @@ inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) cons
                 inMLineValue = true;
                 bool lineExtension{false};
                 bool firstLine = true;
-                if(!item.empty() && item.back() == '\\') {
+                if(!item.empty() && item.back() == '\\' && keyChar == '\"') {
                     item.pop_back();
                     lineExtension = true;
                 } else if(detail::hasMLString(item, keyChar)) {
@@ -435,7 +438,7 @@ inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) cons
         std::vector<std::string> parents;
         try {
             parents = detail::generate_parents(currentSection, name, parentSeparatorChar);
-            detail::process_quoted_string(name);
+            detail::process_quoted_string(name, '"', '\'', true);
             // clean up quotes on the items and check for escaped strings
             for(auto &it : items_buffer) {
                 detail::process_quoted_string(it, stringQuote, literalQuote);
@@ -556,6 +559,31 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description,
                 if(results.size() > 1 && opt->get_multi_option_policy() == CLI::MultiOptionPolicy::Reverse) {
                     std::reverse(results.begin(), results.end());
                 }
+                if(opt->get_multi_option_policy() == CLI::MultiOptionPolicy::Sum && opt->count() >= 1 &&
+                   results.size() == 1) {
+                    // if the multi option policy is sum then there is a possibility of incorrect fields being produced
+                    // best to just use the original data for config files
+                    auto pos = opt->_validate(results[0], 0);
+                    if(!pos.empty()) {
+                        results = opt->results();
+                    }
+                }
+                if(opt->get_multi_option_policy() == CLI::MultiOptionPolicy::Join && opt->count() > 1) {
+                    char delim = opt->get_delimiter();
+                    if(delim == '\0') {
+                        // this branch deals with a situation where the output would not be readable by a config file
+                        results = opt->results();
+                    } else {
+                        // this branch deals with the case of the strings containing the delimiter itself or empty
+                        // strings which would be interpreted incorrectly
+                        auto delim_count = std::count(results[0].begin(), results[0].end(), delim);
+                        if(results[0].back() == delim ||
+                           static_cast<decltype(delim_count)>(opt->count()) < delim_count - 1 ||
+                           results[0].find(std::string(2, delim)) != std::string::npos) {
+                            results = opt->results();
+                        }
+                    }
+                }
                 std::string value =
                     detail::ini_join(results, arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote);
 

include/CLI/impl/StringTools_inl.hpp

@@ -544,13 +544,15 @@ CLI11_INLINE void handle_secondary_array(std::string &str) {
     }
 }
 
-CLI11_INLINE bool process_quoted_string(std::string &str, char string_char, char literal_char) {
+CLI11_INLINE bool
+process_quoted_string(std::string &str, char string_char, char literal_char, bool disable_secondary_array_processing) {
     if(str.size() <= 1) {
         return false;
     }
     if(detail::is_binary_escaped_string(str)) {
         str = detail::extract_binary_string(str);
-        handle_secondary_array(str);
+        if(!disable_secondary_array_processing)
+            handle_secondary_array(str);
         return true;
     }
     if(str.front() == string_char && str.back() == string_char) {
@@ -558,12 +560,14 @@ CLI11_INLINE bool process_quoted_string(std::string &str, char string_char, char
         if(str.find_first_of('\\') != std::string::npos) {
             str = detail::remove_escaped_characters(str);
         }
-        handle_secondary_array(str);
+        if(!disable_secondary_array_processing)
+            handle_secondary_array(str);
         return true;
     }
     if((str.front() == literal_char || str.front() == '`') && str.back() == str.front()) {
         detail::remove_outer(str, str.front());
-        handle_secondary_array(str);
+        if(!disable_secondary_array_processing)
+            handle_secondary_array(str);
         return true;
     }
     return false;