ArgParser: global options are matched anywhere in argv, even after --

[brbzull0]

Impact

ArgParser matches a command's options anywhere in the rest of the command line, and the -- escape added in #13570 does not reach that scan. For the top-level command that is the whole argv, so a global option is recognised even when it appears as a subcommand's value, and even after --, which #13570 made the way to pass a value that starts with -. The same scan lets a global option silently shadow a subcommand option that has the same short name.

What an operator sees, with traffic_ctl (it applies equally to traffic_layout and traffic_cache_tool):

• A value spelled like a global option (-f, -w, --run-root, --read-timeout-ms, ...) is taken as that option: usage error, exit 64, and -- does not help.
• A value of -h or -V prints help or the version and exits 0. The command never runs, but a script sees success.
• traffic_ctl config reload -w <seconds> sets the global --watch, which nothing in the tree reads, instead of the documented config reload --initial-wait, -w.

It takes a value that happens to match a global option name, so real-world impact is low. There is no workaround for such values. For the -w clash, --initial-wait works.

Version:  master @ 738cd651b3
          top-level scan unchanged since d9b06a12acf (2018); the "--" escape added in d0fb283406 (#13570)
          only covers collecting an option's own values. The -w clash dates from 5bab268cb4 (#12892).
Platform: Darwin 25.6.0, Apple clang 21.0.0
Config:   n/a

Proof

Command::parse() runs append_option_data() on the command's own argv before recursing into subcommands, and append_option_data() walks every remaining token with no check for --:

src/tscore/ArgParser.cc#L737-L740, called at #L861:

ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int index)
{
  std::map<std::string, unsigned> check_map;
  for (unsigned i = index; i < args.size(); i++) {

-- is only honoured inside handle_args(), once an option has been matched and is collecting its values (#L564, #L591, #L620). A -h anywhere prints help with exit code 0 (#L768-L778).

traffic_ctl registers -w twice, globally for --watch (src/traffic_ctl/traffic_ctl.cc#L95) and on config reload for --initial-wait (#L172). The watch key is never read.

Reproduction, with no traffic_server running; the RPC connection error shows parsing got through:

$ traffic_ctl config set proxy.config.diags.debug.tags -- -m      # -m is not a global option: passed through
Error found:
RPC Node Error: connect(attempts=1/5): Couldn't open connection with .../jsonrpc20.sock. Last error: No such file or directory(2)
[exit 2]

$ traffic_ctl config set proxy.config.diags.debug.tags -- -f      # -f is: taken as --format
Error: 1 argument(s) expected by format
[exit 64]

$ traffic_ctl config set proxy.config.diags.debug.tags -- -f json
Error: 2 argument(s) expected by set
[exit 64]

$ traffic_ctl config set proxy.config.diags.debug.tags -h         # help, exit 0, nothing set
Usage: traffic_ctl [OPTIONS] CMD [ARGS ...]
[exit 0]

$ traffic_ctl config reload -w                                    # documented as --initial-wait
Error: 1 argument(s) expected by watch
[exit 64]

JSON output form (the parse fails before any output is formatted, so it matches):

$ traffic_ctl --format json config set proxy.config.diags.debug.tags -- -f json
Error: 2 argument(s) expected by set
[exit 64]

Expected: after --, -f and -h are the value of proxy.config.diags.debug.tags, as -m already is.

-- itself works. It is the global scan that ignores it: -m is not a global option and passes, -f and -h are and do not.

Proposed change

-- is scoped to one option today (#13570):

• For a fixed or at-most-one option it escapes the next value, and options written after that value still parse: server debug enable --tags -- -a, config get -c -- -weird.yaml proxy.config.x.
• For a variable-arity option it ends that option's own recognition: config reload -D -- -m.

The parent levels just need to leave alone the token that -- escapes. Let append_option_data() skip -- and the token after it, and leave both in place for the option or command further down that owns them:

--- a/src/tscore/ArgParser.cc
+++ b/src/tscore/ArgParser.cc
@@ -738,6 +738,11 @@ ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int inde
 {
   std::map<std::string, unsigned> check_map;
   for (unsigned i = index; i < args.size(); i++) {
+    // "--" escapes the token after it, which belongs to an option or command further down: leave both for it.
+    if (args[i] == "--") {
+      ++i;
+      continue;
+    }
     // find matches of the arg
     if (args[i][0] == '-' && args[i][1] == '-' && args[i].find('=') != std::string::npos) {
       // deal with --args=

I tried it on the tree above. The escaped tokens are handed on:

• config set <rec> -- -f and config set <rec> -- -h now reach the RPC.

Every existing pattern keeps its behaviour, with exit codes and results identical to master:

• server debug enable --tags -- -a -f json
• config get --cold -- -weird.yaml <rec> -f json
• config reload -D -- -m -f json
• config set <rec> -f json -- -x

The tscore unit tests pass (115/115). So do the AuTests that use --: traffic_ctl_server_debug, traffic_ctl_cold_config, config_reload_directive_cli, traffic_ctl_config_output, traffic_ctl_config_reload.

I rejected the obvious alternative, stopping option recognition at -- altogether (break). It turns -- into an end-of-line marker at every level and breaks a global option written after an escaped value: server debug enable --tags -- -a -f json fails with Unknown command, option or args: '-f' 'json'.

What skip-one does not cover:

• After a variable-arity option's --, only the first value is protected from the global scan. In config reload -D -- -a -f json, -f json is still taken as the global --format, the same as today. That contradicts the traffic_ctl.en.rst note that options written after -- are swallowed by -D. --directive=-value stays the way to pass several dash values.
• The -w clash. The global --watch is undocumented and unused, so dropping it (or its short form) gives config reload -w back to --initial-wait.

Still to do:

• A unit test in src/tscore/unit_tests/test_ArgParser.cc: a subcommand value after -- that spells a global option is left to the subcommand, and a global option after the escaped value is still recognised.
• Optionally, reject at registration a subcommand option whose long or short name is already taken by an ancestor, so the next clash fails at startup instead of being silently shadowed.

Related:

• ArgParser: option value recognition should be syntactic, not registry-based #13597: the other direction, where a parent option takes a subcommand's option as its value. Same root cause: every level scans the rest of argv.
• Argparser variable arg option parsing fix/improvement. #13570: added the -- escape for option values.

[aryan9948]

Hi @brbzull0, I'd like to work on this issue. I can add the regression test for the -- handling in ArgParser and investigate any additional cases needed for the proposed fix. Would you be okay with assigning this issue to me?