Skip to content

ArgParser: variable-argument options consume following options, and --option=value truncates values containing '=' #13569

Description

@brbzull0

There are two distinct defects in ArgParser's handling of options, both reproducible on master.

Bug 1: a variable-argument option consumes the options that follow it

Options declared with MORE_THAN_ZERO_ARG_N / MORE_THAN_ONE_ARG_N collect every
remaining token as a value, including tokens that name another option of the same
command. Those options are then never parsed.

$ traffic_ctl config reload -D ip_allow.id=foo -t mytok
Error: '-t' looks like a flag, not a directive. Place -D as the last option on the command line.

$ traffic_ctl config reload -D ip_allow.id=foo -m
Error: '-m' looks like a flag, not a directive. Place -D as the last option on the command line.

(The error text above comes from a guard added in #13110; without it the flag would
be silently swallowed.)

Reversing the order works, which is what makes this a bug rather than a design
constraint:

$ traffic_ctl config reload -m -D ip_allow.id=foo    # fine

Only options registered on the same command are affected. Global options are
already gone by then, because Command::parse() runs the top-level
append_option_data() pass before recursing into subcommands, so
traffic_ctl config reload -D ip_allow.id=foo -f rpc parses correctly.

Root cause

handle_args() in src/tscore/ArgParser.cc:

for (unsigned j = index + 1; j < args.size(); j++) {
  ret.append_arg(name, args[j]);
}
args.erase(args.begin() + index, args.end());

Nothing stops the collection at an option-like token, and the erase truncates to
args.end(), so append_option_data() continues iterating over a vector whose
remaining tokens have been removed.

For comparison, Python's argparse stops consuming nargs='*' / nargs='+'
values at the next token that matches a known option.

There is also no support for -- as an end-of-options marker, so there is no way
to pass a value that legitimately begins with -.

Bug 2: --option=value truncates values containing =

The name is taken up to the first = but the value from the last one:

std::string option_name = args[i].substr(0, args[i].find_first_of('='));
std::string value       = args[i].substr(args[i].find_last_of('=') + 1);

So any value with an embedded = is silently cut down to whatever follows the
final =:

$ traffic_ctl config reload --directive=ip_allow.id=foo
Error: Invalid directive format 'foo'. Expected: config_key.directive_key=value

--directive values are key=value pairs by definition, so the = form is
unusable for the option that needs it most. The fix is to use find_first_of('=')
for both.

Not a parser bug: variable-argument options versus positional arguments

For completeness, since it looks related and is a real usability problem, but needs
a different fix.

$ traffic_ctl config get -c proxy.config.diags.debug.enabled
Error: at least one argument expected by get

$ traffic_ctl config set -c proxy.config.diags.debug.enabled 1
Error: 2 argument(s) expected by set

Here --cold, -c is MORE_THAN_ZERO_ARG_N and the record name is genuinely
ambiguous — it could be the filename for -c or the positional for get. Python's
argparse resolves it the same way (option wins, then the positional is reported
missing), so stopping collection at option-like tokens does not help. The = form
already works today and is the correct spelling:

$ traffic_ctl config get --cold=records.yaml proxy.config.diags.debug.enabled
$ traffic_ctl config set --cold=records.yaml proxy.config.diags.debug.enabled 1

What is wrong here is the advertised usage: traffic_ctl config get --help shows
traffic_ctl config get [OPTIONS] RECORD [RECORD ...], an order that cannot work
for -c. Worth addressing separately, either by giving --cold an explicit
"zero or one argument" semantic or by fixing the usage strings and documentation.

Affected declarations

Variable-argument options, the ones that can lose following options, all in
src/traffic_ctl/traffic_ctl.cc:

  • --cold, -c on config get and config set
  • --data, -d and --directive, -D on config reload
  • --params, -p on rpc invoke

Variable-argument commands (config describe, config get, config match,
host up/down, metric get/describe/match, plugin msg,
storage offline, rpc file, ...) are not directly affected, because options are
stripped before a command's positional arguments are collected.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions