diff --git a/include/cxxopts.hpp b/include/cxxopts.hpp index 1a50874..7b4e2f1 100644 --- a/include/cxxopts.hpp +++ b/include/cxxopts.hpp @@ -672,6 +672,16 @@ inline bool IsFalseText(const std::string &text) return false; } +static inline bool valid_option_later_char(char c) +{ + return c!='=' && c!=',' && !std::isspace(c, std::locale::classic()) && !std::iscntrl(c, std::locale::classic()); +} + +static inline bool valid_option_first_char(char c) +{ + return c != '-' && valid_option_later_char(c); +} + inline OptionNames split_option_names(const std::string &text) { OptionNames split_names; @@ -692,25 +702,27 @@ inline OptionNames split_option_names(const std::string &text) } token_start_pos = next_non_space_pos; auto next_delimiter_pos = text.find(',', token_start_pos); - if (next_delimiter_pos == token_start_pos) { - throw_or_mimic(text); - } + if (next_delimiter_pos == npos) { next_delimiter_pos = length; } + else if (next_delimiter_pos == token_start_pos) { + throw_or_mimic(text); + } + else if(next_delimiter_pos == length-1) { + // delimter at the end. ex : "a,ab," + throw_or_mimic(text); + } auto token_length = next_delimiter_pos - token_start_pos; - // validate the token itself matches the regex /([:alnum:][-_[:alnum:]]*/ { - const char* option_name_valid_chars = - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789" - "_-.?"; - - if (!std::isalnum(text[token_start_pos], std::locale::classic()) || - text.find_first_not_of(option_name_valid_chars, token_start_pos) < next_delimiter_pos) { + if(!valid_option_first_char(text[token_start_pos])){ throw_or_mimic(text); } + for(size_t i=token_start_pos+1; i(text); + } + } } split_names.emplace_back(text.substr(token_start_pos, token_length)); token_start_pos = next_delimiter_pos + 1; @@ -726,11 +738,11 @@ inline ArguDesc ParseArgument(const char *arg, bool &matched) if (strncmp(pdata, "--", 2) == 0) { pdata += 2; - if (isalnum(*pdata, std::locale::classic())) + if (valid_option_first_char(*pdata)) { argu_desc.arg_name.push_back(*pdata); pdata += 1; - while (isalnum(*pdata, std::locale::classic()) || *pdata == '-' || *pdata == '_' || *pdata == '.') + while (valid_option_later_char(*pdata)) { argu_desc.arg_name.push_back(*pdata); pdata += 1; @@ -757,7 +769,7 @@ inline ArguDesc ParseArgument(const char *arg, bool &matched) else if (strncmp(pdata, "-", 1) == 0) { pdata += 1; - if(isalnum(*pdata, std::locale::classic())) { + if(valid_option_first_char(*pdata)) { // If we have '=' right after first alnum, its a match. if(*(pdata+1) == '=') { argu_desc.arg_name.push_back(*pdata); @@ -777,6 +789,13 @@ inline ArguDesc ParseArgument(const char *arg, bool &matched) #else // CXXOPTS_NO_REGEX namespace { + +#define CXXOPTS_RE_NAME_START "[^-=,[:space:][:cntrl:]]" +#define CXXOPTS_RE_NAME_CHAR "[^=,[:space:][:cntrl:]]" +#define CXXOPTS_RE_NAME CXXOPTS_RE_NAME_START CXXOPTS_RE_NAME_CHAR "*" +#define CXXOPTS_RE_LONG_NAME CXXOPTS_RE_NAME_START CXXOPTS_RE_NAME_CHAR "+" +#define CXXOPTS_RE_SHORT_NAME CXXOPTS_RE_NAME_START + CXXOPTS_LINKONCE const char* const integer_pattern = "(-)?(0x)?([0-9a-zA-Z]+)|((0x)?0)"; @@ -788,12 +807,12 @@ const char* const falsy_pattern = "(f|F)(alse)?|0"; CXXOPTS_LINKONCE const char* const option_pattern = - "--([[:alnum:]][-_[:alnum:]\\.]+)(=(.*))?|-([[:alnum:]])((=(.*))|(.*))"; -// <-------Long Option--------------------> <-----Short Option-------> + "--(" CXXOPTS_RE_LONG_NAME ")(=(.*))?|-(" CXXOPTS_RE_SHORT_NAME ")((=(.*))|(.*))"; +// <-------Long Option---------------> <-------------Short Option---------------> // Groups : -// <---------1------------------><--2--> <--4--------><-----5------> -// <-3> <--6--> <-8> -// <-7> +// <---------1--------------><--2--> <-----------4-------------><-----5------> +// <-3> <--6--> <-8> +// <-7> const int LONG_NAME_IDX=1; const int LONG_MATCH_IDX=2; const int LONG_MATCH_VALUE_IDX=3; @@ -804,10 +823,16 @@ const int SHORT_GROUPING_IDX=8; CXXOPTS_LINKONCE const char* const option_specifier_pattern = - "([[:alnum:]][-_[:alnum:]\\.]*)(,[ ]*[[:alnum:]][-_[:alnum:]]*)*"; + "(" CXXOPTS_RE_NAME ")(,[ ]*" CXXOPTS_RE_NAME ")*"; CXXOPTS_LINKONCE const char* const option_specifier_separator_pattern = ", *"; +#undef CXXOPTS_RE_NAME_START +#undef CXXOPTS_RE_NAME_CHAR +#undef CXXOPTS_RE_NAME +#undef CXXOPTS_RE_LONG_NAME +#undef CXXOPTS_RE_SHORT_NAME + } // namespace inline IntegerDesc SplitInteger(const std::string &text) diff --git a/test/options.cpp b/test/options.cpp index 7ef5aca..c028ed8 100644 --- a/test/options.cpp +++ b/test/options.cpp @@ -277,6 +277,122 @@ TEST_CASE("Providing options values via equal sign", "[options]") } } +TEST_CASE("Valid/Invalid Option names", "[options]") +{ + const char prog_name[] = "test_invalid_opt_names"; + std::vector> invalid_opts = { + {"Equals short name", "="}, + {"Dash short name", "-"}, + {"Long name starts with =", "a,=abcd"}, + {"Long name starts with dash", "a,-abcd"}, + {"Whitespace as name", " "}, + {"Name is a space", "a, "}, + {"Only comma delimiter given", ","}, + {"Leading comma", ",a"}, + {"Trailing comma after names", "a,ab,"}, + {"Double comma", "a,,ab"}, + {"Empty option name string", ""}, + {"Control character in name", "\x03"}, + {"Tab inside option name", "a,ab,\tac"}, + {"Equals inside option name", "a,ab,ac,ab=cd"}, + {"Two simple option names", "a,b"}, + }; + + std::vector> valid_opts = { + {"Basic", "a,ab"}, + {"Spaces", "a, ab, xyz"}, + {"Arbitrary characters", "#,%%%,/><>?"}, + }; + + for(const auto& t: invalid_opts) { + SECTION(t.first){ + cxxopts::Options options(prog_name, " - Option names as invalid characters"); + auto option_adder = options.add_options(); + CHECK_THROWS(option_adder(t.second, "description", cxxopts::value())); + } + } + for(const auto& t: valid_opts) { + SECTION(t.first){ + cxxopts::Options options(prog_name, " - Option names as invalid characters"); + auto option_adder = options.add_options(); + CHECK_NOTHROW(option_adder(t.second, "description", cxxopts::value())); + } + } +} + +TEST_CASE("Option names as arbitrary characters and values parsing", "[options]") +{ + const char prog_name[] = "test_name_chars"; + const char implicit_value[] = "implicit"; + struct testcases + { + std::string name; + std::vector opt_names; + Argv argv; + std::vector> expValues; + bool parseException; + bool setImplicit; + } tests[] = { + { + "Arbitrary chars", + {"#,^^","$$$"}, + Argv{prog_name, "-#", "abc", "--$$$=@#$$"}, + {{"#", "abc"}, {"$$$", "@#$$"}}, + false, + false, + }, + { + "Arbitrary chars and spaces", + {"*, ab-cd","ab_cd"}, + Argv{prog_name, "--ab-cd", "xyz","--ab_cd={}()"}, + {{"ab-cd", "xyz"}, {"ab_cd", "{}()"}}, + false, + false, + }, + { + "Arbitrary character as single dot", + {"."}, + Argv{prog_name, "-.=.."}, + {{".", ".."}}, + false, + false, + }, + { + "Multiple Arbitrary characters as short options", + {"@",":","?","[","}","*","~","!"}, + Argv{prog_name, "-@:?[}*~!"}, + {{"@", implicit_value}, {"[", implicit_value}, {"!", implicit_value}}, + false, + true, + } + }; + + for(const auto& tc : tests) { + SECTION(tc.name){ + cxxopts::Options options(prog_name, " - Option names as arbitrary characters"); + auto option_adder = options.add_options(); + for(const auto& opt_name: tc.opt_names) { + if(tc.setImplicit){ + option_adder(opt_name, "description", cxxopts::value()->implicit_value(implicit_value)); + } + else{ + option_adder(opt_name, "description", cxxopts::value()); + } + } + if(tc.parseException) { + CHECK_THROWS(options.parse(tc.argv.argc(), tc.argv.argv())); + continue; + } + auto result = options.parse(tc.argv.argc(), tc.argv.argv()); + for (const auto& p : tc.expValues) { + CHECK(result[p.first].as() == p.second); + } + + } + } +} + + TEST_CASE("No positional", "[positional]") { cxxopts::Options options("test_no_positional", @@ -951,7 +1067,7 @@ TEST_CASE("Allow bad short syntax", "[options]") { Argv av({ "--ab?", - "-?b?#@" + "-=?b?#@" }); auto** argv = av.argv(); @@ -965,7 +1081,7 @@ TEST_CASE("Allow bad short syntax", "[options]") { options.allow_unrecognised_options(); CHECK_NOTHROW(options.parse(argc, argv)); REQUIRE(argc == 2); - CHECK_THAT(argv[1], Catch::Equals("-?b?#@")); + CHECK_THAT(argv[1], Catch::Equals("-=?b?#@")); } }