From 0f5bf21e9157d06d188d9d7ae4398935f27ecf46 Mon Sep 17 00:00:00 2001 From: Philip Top Date: Mon, 18 Dec 2023 05:21:32 -0800 Subject: [PATCH] add some reduction methods to the options on the fuzz tests (#930) This adds a round trip test for config file generation to the fuzzer. (the next step after this PR will be a fuzzer that verifies that the round trip actually matches the results. This change ended up requiring quite a few minor changes to fix the ambiguities between the config file generation and config file reader. 1). There was a number of potential conflicts between positional names and regular option names that could be triggered in config files, this required a number of additional checks on the positional naming to ensure no conflicts. 2). flag options with disable flag override can produce output results that are not valid by themselves, resolving this required flag input to be able to handle an array and output the original value set of results. 3). strings with non-printable characters could cause all sorts of chaos in the config files. This was resolved by generating a binary string conversion format and handling multiline comments and characters, and handling escaped characters. Note; I think a better solution is to move to fully supporting string formatting and escaping along with the binary strings from TOML now that TOML 1.0 is finalized. That will not be this PR though, maybe the next one. 4). Lot of ambiguities and edge cases in the string splitter, this was reworked 5). handling of comments was not done well, especially comment characters in the name of the option which is allowed. 6). non printable characters in the option naming. This would be weird in practice but it also cause some big holes in the config file generation, so the restricted character set for option naming was expanded. (don't allow spaces or control characters). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .codacy.yml | 18 +++ CPPLINT.cfg | 1 + book/chapters/config.md | 10 ++ fuzz/cli11_app_fuzz.cpp | 10 +- fuzz/cli11_file_fuzz.cpp | 7 + fuzz/fuzzApp.cpp | 28 +++- fuzz/fuzzApp.hpp | 11 ++ fuzz/fuzzCommand.cpp | 1 + fuzz/fuzz_dictionary1.txt | 16 ++ fuzz/fuzz_dictionary2.txt | 8 + include/CLI/Config.hpp | 5 +- include/CLI/ConfigFwd.hpp | 2 +- include/CLI/Error.hpp | 3 + include/CLI/Option.hpp | 12 +- include/CLI/StringTools.hpp | 40 ++++- include/CLI/impl/App_inl.hpp | 79 ++++++++-- include/CLI/impl/Config_inl.hpp | 147 ++++++++++++++++--- include/CLI/impl/Option_inl.hpp | 25 +++- include/CLI/impl/Split_inl.hpp | 10 +- include/CLI/impl/StringTools_inl.hpp | 210 ++++++++++++++++++++++++--- tests/AppTest.cpp | 22 --- tests/CMakeLists.txt | 2 +- tests/ConfigFileTest.cpp | 44 +++++- tests/CreationTest.cpp | 41 ++++++ tests/FuzzFailTest.cpp | 37 +++++ tests/HelpersTest.cpp | 120 +++++++++++++++ tests/fuzzFail/fuzz_app_file_fail1 | 0 tests/fuzzFail/fuzz_app_file_fail10 | 3 + tests/fuzzFail/fuzz_app_file_fail11 | 1 + tests/fuzzFail/fuzz_app_file_fail12 | Bin 0 -> 17 bytes tests/fuzzFail/fuzz_app_file_fail13 | 1 + tests/fuzzFail/fuzz_app_file_fail14 | 4 + tests/fuzzFail/fuzz_app_file_fail15 | 1 + tests/fuzzFail/fuzz_app_file_fail16 | Bin 0 -> 13 bytes tests/fuzzFail/fuzz_app_file_fail17 | 9 ++ tests/fuzzFail/fuzz_app_file_fail18 | 1 + tests/fuzzFail/fuzz_app_file_fail19 | 1 + tests/fuzzFail/fuzz_app_file_fail2 | 1 + tests/fuzzFail/fuzz_app_file_fail20 | 1 + tests/fuzzFail/fuzz_app_file_fail21 | Bin 0 -> 24 bytes tests/fuzzFail/fuzz_app_file_fail22 | 1 + tests/fuzzFail/fuzz_app_file_fail23 | Bin 0 -> 14 bytes tests/fuzzFail/fuzz_app_file_fail24 | Bin 0 -> 25 bytes tests/fuzzFail/fuzz_app_file_fail25 | Bin 0 -> 36 bytes tests/fuzzFail/fuzz_app_file_fail26 | 1 + tests/fuzzFail/fuzz_app_file_fail27 | 2 + tests/fuzzFail/fuzz_app_file_fail28 | 5 + tests/fuzzFail/fuzz_app_file_fail29 | 4 + tests/fuzzFail/fuzz_app_file_fail3 | 1 + tests/fuzzFail/fuzz_app_file_fail30 | 1 + tests/fuzzFail/fuzz_app_file_fail31 | 1 + tests/fuzzFail/fuzz_app_file_fail32 | 1 + tests/fuzzFail/fuzz_app_file_fail4 | 1 + tests/fuzzFail/fuzz_app_file_fail5 | 6 + tests/fuzzFail/fuzz_app_file_fail6 | Bin 0 -> 26 bytes tests/fuzzFail/fuzz_app_file_fail7 | 3 + tests/fuzzFail/fuzz_app_file_fail8 | Bin 0 -> 14 bytes tests/fuzzFail/fuzz_app_file_fail9 | 1 + 58 files changed, 846 insertions(+), 114 deletions(-) create mode 100644 .codacy.yml create mode 100644 tests/fuzzFail/fuzz_app_file_fail1 create mode 100644 tests/fuzzFail/fuzz_app_file_fail10 create mode 100644 tests/fuzzFail/fuzz_app_file_fail11 create mode 100644 tests/fuzzFail/fuzz_app_file_fail12 create mode 100644 tests/fuzzFail/fuzz_app_file_fail13 create mode 100644 tests/fuzzFail/fuzz_app_file_fail14 create mode 100644 tests/fuzzFail/fuzz_app_file_fail15 create mode 100644 tests/fuzzFail/fuzz_app_file_fail16 create mode 100644 tests/fuzzFail/fuzz_app_file_fail17 create mode 100644 tests/fuzzFail/fuzz_app_file_fail18 create mode 100644 tests/fuzzFail/fuzz_app_file_fail19 create mode 100644 tests/fuzzFail/fuzz_app_file_fail2 create mode 100644 tests/fuzzFail/fuzz_app_file_fail20 create mode 100644 tests/fuzzFail/fuzz_app_file_fail21 create mode 100644 tests/fuzzFail/fuzz_app_file_fail22 create mode 100644 tests/fuzzFail/fuzz_app_file_fail23 create mode 100644 tests/fuzzFail/fuzz_app_file_fail24 create mode 100644 tests/fuzzFail/fuzz_app_file_fail25 create mode 100644 tests/fuzzFail/fuzz_app_file_fail26 create mode 100644 tests/fuzzFail/fuzz_app_file_fail27 create mode 100644 tests/fuzzFail/fuzz_app_file_fail28 create mode 100644 tests/fuzzFail/fuzz_app_file_fail29 create mode 100644 tests/fuzzFail/fuzz_app_file_fail3 create mode 100644 tests/fuzzFail/fuzz_app_file_fail30 create mode 100644 tests/fuzzFail/fuzz_app_file_fail31 create mode 100644 tests/fuzzFail/fuzz_app_file_fail32 create mode 100644 tests/fuzzFail/fuzz_app_file_fail4 create mode 100644 tests/fuzzFail/fuzz_app_file_fail5 create mode 100644 tests/fuzzFail/fuzz_app_file_fail6 create mode 100644 tests/fuzzFail/fuzz_app_file_fail7 create mode 100644 tests/fuzzFail/fuzz_app_file_fail8 create mode 100644 tests/fuzzFail/fuzz_app_file_fail9 diff --git a/.codacy.yml b/.codacy.yml new file mode 100644 index 00000000..03a1e522 --- /dev/null +++ b/.codacy.yml @@ -0,0 +1,18 @@ +--- +engines: + rubocop: + enabled: true + duplication: + enabled: true + metrics: + enabled: true + coverage: + enabled: false +languages: + +exclude_paths: + - "fuzz/**/*" + - "fuzz/*" + - "scripts/**/*" + - "scripts/*" + - "**.md" diff --git a/CPPLINT.cfg b/CPPLINT.cfg index 40bec371..e1d27d9f 100644 --- a/CPPLINT.cfg +++ b/CPPLINT.cfg @@ -9,6 +9,7 @@ filter=-readability/nolint # Conflicts with clang-tidy filter=-readability/check # Catch uses CHECK(a == b) (Tests only) filter=-build/namespaces # Currently using it for one test (Tests only) filter=-runtime/references # Requires fundamental change of API, don't see need for this +filter=-runtime/string # Requires not using static const strings which makes thing really annoying filter=-whitespace/blank_line # Unnecessarily strict with blank lines that otherwise help with readability filter=-whitespace/indent # Requires strange 3-space indent of private/protected/public markers filter=-whitespace/parens,-whitespace/braces # Conflict with clang-format diff --git a/book/chapters/config.md b/book/chapters/config.md index d8b2d475..e6720502 100644 --- a/book/chapters/config.md +++ b/book/chapters/config.md @@ -206,6 +206,16 @@ str3 = """\ The key is that the closing of the multiline string must be at the end of a line and match the starting 3 quote sequence. +### Binary Strings + +Config files have a binary conversion capability, this is mainly to support +writing config files but can be used by user generated files as well. Strings +with the form `B"(XXXXX)"` will convert any characters inside the parenthesis +with the form \xHH to the equivalent binary value. The HH are hexadecimal +characters. Characters not in this form will be translated as given. If argument +values with unprintable characters are used to generate a config file this +binary form will be used in the output string. + ## Multiple configuration files If it is desired that multiple configuration be allowed. Use diff --git a/fuzz/cli11_app_fuzz.cpp b/fuzz/cli11_app_fuzz.cpp index 321d644d..bd3a5f9b 100644 --- a/fuzz/cli11_app_fuzz.cpp +++ b/fuzz/cli11_app_fuzz.cpp @@ -40,10 +40,16 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { try { app->parse(parseString); + } catch(const CLI::ParseError &e) { //(app)->exit(e); // this just indicates we caught an error known by CLI + return 0; // Non-zero return values are reserved for future use. } - - return 0; // Non-zero return values are reserved for future use. + // should be able to write the config to a file and read from it again + std::string configOut = app->config_to_str(); + app->clear(); + std::stringstream out(configOut); + app->parse_from_stream(out); + return 0; } diff --git a/fuzz/cli11_file_fuzz.cpp b/fuzz/cli11_file_fuzz.cpp index 5858822d..e69193b9 100644 --- a/fuzz/cli11_file_fuzz.cpp +++ b/fuzz/cli11_file_fuzz.cpp @@ -22,6 +22,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { auto app = fuzzdata.generateApp(); try { app->parse_from_stream(out); + // should be able to write the config to a file and read from it again + std::string configOut = app->config_to_str(); + + app->clear(); + std::stringstream out(configOut); + app->parse_from_stream(out); + } catch(const CLI::ParseError &e) { // (app)->exit(e); // this just indicates we caught an error known by CLI diff --git a/fuzz/fuzzApp.cpp b/fuzz/fuzzApp.cpp index 9769fef9..794d2b6a 100644 --- a/fuzz/fuzzApp.cpp +++ b/fuzz/fuzzApp.cpp @@ -68,9 +68,9 @@ std::shared_ptr FuzzApp::generateApp() { auto *vgroup = fApp->add_option_group("vectors"); vgroup->add_option("--vopt1", vv1); - vgroup->add_option("--vopt2", vvs); + vgroup->add_option("--vopt2", vvs)->inject_separator(); vgroup->add_option("--vopt3", vstr); - vgroup->add_option("--vopt4", vecvecd); + vgroup->add_option("--vopt4", vecvecd)->inject_separator(); fApp->add_option("--oopt1", od1); fApp->add_option("--oopt2", ods); @@ -121,6 +121,30 @@ std::shared_ptr FuzzApp::generateApp() { sub->add_option("--sdwrap", dwrap); sub->add_option("--siwrap", iwrap); + auto *resgroup = fApp->add_option_group("outputOrder"); + + resgroup->add_option("--vA", vstrA)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::TakeAll); + resgroup->add_option("--vB", vstrB)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::TakeLast); + resgroup->add_option("--vC", vstrC)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::TakeFirst); + resgroup->add_option("--vD", vstrD)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::Reverse); + resgroup->add_option("--vS", val32)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::Sum); + resgroup->add_option("--vM", mergeBuffer)->expected(0, 2)->multi_option_policy(CLI::MultiOptionPolicy::Join); + resgroup->add_option("--vE", vstrE)->expected(2, 4)->delimiter(','); + + auto *vldtr = fApp->add_option_group("validators"); + + validator_strings.resize(10); + vldtr->add_option("--vdtr1", validator_strings[0])->join()->check(CLI::PositiveNumber); + vldtr->add_option("--vdtr2", validator_strings[1])->join()->check(CLI::NonNegativeNumber); + vldtr->add_option("--vdtr3", validator_strings[2])->join()->check(CLI::NonexistentPath); + vldtr->add_option("--vdtr4", validator_strings[3])->join()->check(CLI::Range(7, 3456)); + vldtr->add_option("--vdtr5", validator_strings[4]) + ->join() + ->check(CLI::Range(std::string("aa"), std::string("zz"), "string range")); + vldtr->add_option("--vdtr6", validator_strings[5])->join()->check(CLI::TypeValidator()); + vldtr->add_option("--vdtr7", validator_strings[6])->join()->check(CLI::TypeValidator()); + vldtr->add_option("--vdtr8", validator_strings[7])->join()->check(CLI::ValidIPV4); + vldtr->add_option("--vdtr9", validator_strings[8])->join()->transform(CLI::Bound(2, 255)); return fApp; } diff --git a/fuzz/fuzzApp.hpp b/fuzz/fuzzApp.hpp index 45d02a17..2a97bc49 100644 --- a/fuzz/fuzzApp.hpp +++ b/fuzz/fuzzApp.hpp @@ -75,6 +75,7 @@ class FuzzApp { std::vector vv1{}; std::vector vstr{}; + std::vector> vecvecd{}; std::vector> vvs{}; std::optional od1{}; @@ -103,5 +104,15 @@ class FuzzApp { std::string buffer{}; int intbuffer{0}; std::atomic doubleAtomic{0.0}; + + // for testing restrictions and reduction methods + std::vector vstrA{}; + std::vector vstrB{}; + std::vector vstrC{}; + std::vector vstrD{}; + std::vector vstrE{}; + std::vector vstrF{}; + std::string mergeBuffer{}; + std::vector validator_strings{}; }; } // namespace CLI diff --git a/fuzz/fuzzCommand.cpp b/fuzz/fuzzCommand.cpp index 07ab0df2..f73386fc 100644 --- a/fuzz/fuzzCommand.cpp +++ b/fuzz/fuzzCommand.cpp @@ -20,5 +20,6 @@ int main(int argc, char **argv) { (app)->exit(e); // this just indicates we caught an error known by CLI } + return 0; } diff --git a/fuzz/fuzz_dictionary1.txt b/fuzz/fuzz_dictionary1.txt index 166642ec..327f658b 100644 --- a/fuzz/fuzz_dictionary1.txt +++ b/fuzz/fuzz_dictionary1.txt @@ -66,6 +66,14 @@ "--siwrap" "--svtup" "--satd" +"--vA" +"--vB" +"--vC" +"--vD" +"--vS" +"--vM" +"--vE" +"--vdtr" "nflag2" "stup1" "svtup" @@ -143,6 +151,7 @@ "stup4" "sdwrap" "siwrap" +"vdtr" "svtup" "satd" "%%" @@ -156,3 +165,10 @@ "!" "{" "}" +"vA" +"vB" +"vC" +"vD" +"vS" +"vM" +"vE" diff --git a/fuzz/fuzz_dictionary2.txt b/fuzz/fuzz_dictionary2.txt index 47f30afa..828be3a3 100644 --- a/fuzz/fuzz_dictionary2.txt +++ b/fuzz/fuzz_dictionary2.txt @@ -72,6 +72,7 @@ "svopt4" "config" "nflag2" +"vdtr" "--" "fuzzer" "t-" @@ -82,3 +83,10 @@ "dexists" "fexists" "fnexists" +"vA" +"vB" +"vC" +"vD" +"vS" +"vM" +"vE" diff --git a/include/CLI/Config.hpp b/include/CLI/Config.hpp index a91f0da6..d5c6bc34 100644 --- a/include/CLI/Config.hpp +++ b/include/CLI/Config.hpp @@ -24,7 +24,10 @@ namespace CLI { // [CLI11:config_hpp:verbatim] namespace detail { -std::string convert_arg_for_ini(const std::string &arg, char stringQuote = '"', char characterQuote = '\''); +std::string convert_arg_for_ini(const std::string &arg, + char stringQuote = '"', + char characterQuote = '\'', + bool disable_multi_line = false); /// Comma separated join, adds quotes if needed std::string ini_join(const std::vector &args, diff --git a/include/CLI/ConfigFwd.hpp b/include/CLI/ConfigFwd.hpp index a9ae2176..76fc9fa7 100644 --- a/include/CLI/ConfigFwd.hpp +++ b/include/CLI/ConfigFwd.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include // [CLI11:public_includes:end] @@ -29,7 +30,6 @@ struct ConfigItem { /// This is the name std::string name{}; - /// Listing of inputs std::vector inputs{}; diff --git a/include/CLI/Error.hpp b/include/CLI/Error.hpp index 93f8469e..db222e72 100644 --- a/include/CLI/Error.hpp +++ b/include/CLI/Error.hpp @@ -127,6 +127,9 @@ class BadNameString : public ConstructionError { return BadNameString("Long names strings require 2 dashes " + name); } static BadNameString BadLongName(std::string name) { return BadNameString("Bad long name: " + name); } + static BadNameString BadPositionalName(std::string name) { + return BadNameString("Invalid positional Name: " + name); + } static BadNameString DashesOnly(std::string name) { return BadNameString("Must have a name, not just dashes: " + name); } diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp index 0afbc978..89210bb5 100644 --- a/include/CLI/Option.hpp +++ b/include/CLI/Option.hpp @@ -550,12 +550,12 @@ class Option : public OptionBase