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.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions