Crash when using a slash character as the name of a subcommand

[suzukieng]

Just stumbled upon this, while migrating a HTTP/REST based API to a CLI utility (yes, these kinds of projects exist).

I attempted to name a subcommand "legacy/status" and got a crash (Visual Studio 10):

Unhandled exception at 0x759FA6E2 in xxx.exe: Microsoft C++ exception: CLI::IncorrectConstruction at memory location 0x010AF178.

Regardless of it being a good or bad idea to have a slash as part of a command name, a better error message or something in the (excellent!) documentation might help.

[phlptp]

I am going through some of the issues today, I am going to try to see what is up with this one and fix it if possible or at least make a better error message

[henryiii]

I've added a "triage" label for things that are likely bugs and should be checked before release, but not verified quite yet (say by running locally or adding a test). Feel free to add that to any issues you see that look like they need to be addressed but you don't have time to do it ATM. :) If verified, they can become "bug".

[phlptp]

I don't think allowing a slash in general is a good idea. It can be used in shells and reinterpreted. That being said there are applications where it and other characters could be useful and allowed in options and subcommands.

Not sure what a good way of allowing that in code without library modification yet.
But the code that controls the validity is

/// Verify the first character of an option
template <typename T> bool valid_first_char(T c) {
    return std::isalnum(c, std::locale()) || c == '_' || c == '?' || c == '@';
}

/// Verify following characters of an option
template <typename T> bool valid_later_char(T c) { return valid_first_char(c) || c == '.' || c == '-'; }

It would be possible to add additional characters in that part of the code to allow other characters in a slightly modified library

[henryiii]

Would there be harm in including slashes in valid_later_char?

[phlptp]

can't use '\' since that is an escape character on many shells. '/' as a first character can't be allowed since that is often used on windows systems as an argument. But as a later character nothing pops out as a reason why it can't be allowed.

If we want to I can add it pretty quickly

[henryiii]

If someone passes \n, then we get the escaped version. If someone passes \\, we see \. So on our side, it doesn't seem like we should necessarily disallow it? Agree about the first character, just thinking for later characters. Also for a space, such as ' ', one could write ./app 'my command', which would be a terrible CLI, but technically valid.

[henryiii]

(not really that worried about weird characters in the middle, just wondering why they need to be disallowed)

[phlptp]

Some of them would generate invalid config files, but I think some of the ones we already allow would do the same so that is probably not much of an issue. Maybe should be noted somewhere though.

[phlptp]

I suppose we could move the later_characters to a blacklist instead of whitelist. So the only ones restricted are =:!{} =and : for separators, and ! and {} to reserve for the specifying default values like we do with flags. Obviously using some would be difficult on the command line but maybe we don't care

[phlptp]

hypothetically could change the first character to any with the addition of / and -

[phlptp]

that of course could lead to some bizarre CLI but again we might not care

[phlptp]

I am thinking to make a PR that relaxes most of the restrictions on options and subcommand names. That would enable the use of / in option names