brbzull0 opened a new pull request, #13862:
URL: https://github.com/apache/trafficserver/pull/13862
Fixes #13597
Stacked on #13861: the first commit is that PR. Only the second commit,
`ArgParser: refuse a subcommand option as a value`, is this PR's change.
## Problem
ArgParser refused an option's value only when it named another option of the
same command, so an outer option took a subcommand's option as its value:
`traffic_ctl config reload -f -m` set `--format` to `-m` and silently dropped
`--monitor`.
## What this PR does
- The parse records where each token came from, which commands it
dispatched, and which dash-prefixed tokens options took as values
(`ArgParser::Command::ParseTrace`). Once the line has parsed, a value that
names an option of a command dispatched below the option's own command, written
after that command's name, is a usage error:
`Error: '-m' is an option of reload, not a value of --format (put --
before it to make it one)`.
For an option taking a variable number of values the hint is the
`--option=value` form instead, since a `--` there would take the rest of the
line too.
Tests: `An option's value is not taken from an option of a command named
earlier on the line` in `test_ArgParser.cc`; `config_reload_directive_cli`
Tests 15-17.
- The refusal is the same for every arity, and an option's environment
variable does not stand in for the refused value.
Test: `A declared variable does not stand in for a value written for a
subcommand`.
- Only commands the parse dispatched count, and only for tokens after their
name. `traffic_ctl -f -m config reload` still sets `--format` to `-m`. A
subcommand name read as a value, a name escaped by `--`, and a default command
the parser inserts do not count; a subcommand of that default command that the
line does name counts. This also answers the issue's second point: an option
added to one command does not change how a line naming another command is read.
Tests: `Only a command the line dispatches counts, wherever its name
appears`, `A command with a value and a subcommand`, `A subcommand sharing its
parent's name counts only where it is named`, `A default command counts only
when it is written out`.
- The check runs once, after a successful parse, so a line refused for
another reason, or one asking for help or the version, is answered as before.
Tests: `a line refused for another reason keeps its own error`, `a line
asking for help or the version still gets it`.
- Docs: `ArgParser.en.rst` and a note in `upgrading.en.rst`.
## Why not the syntactic rule
The issue proposes argparse's rule: refuse any dash-prefixed token as a
value. That breaks `traffic_ctl config set <rec> -e` (pinned by
`traffic_ctl_error_level`) and `traffic_ctl config get -c -weird.yaml <rec>`
(#13570's example), which ATS keeps on purpose, and argparse also forbids
parent options after the subcommand, which ATS allows.
## What this PR does not do
- The issue's second example, `traffic_ctl config reload -f -D
ip_allow.id=foo`, is already refused on master, and keeps its message: `Unknown
command, option or args: 'ip_allow.id=foo'`.
- A value written before the subcommand's name is read as before, since the
subcommand could never receive it there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
