From: Karel Zak Date: Tue, 12 May 2026 10:49:52 +0000 (+0200) Subject: script: fix "--" separator when used as option argument X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88425e66536d1a9561bb1483d016a2804f3e3a5e;p=thirdparty%2Futil-linux.git script: fix "--" separator when used as option argument The commit 7268e79b added "+" to the getopt_long() options string and post-getopt "--" detection to support commands after the "--" separator. The "+" prefix stops getopt at the first non-option argument, which broke backward compatible "script file -c command" usage. The commit 70507ab9ea fixed this by pre-scanning argv for "--" before getopt and removing the "+" prefix to restore GNU getopt argument permutation. However, the pre-scan was too naive — it treated any "--" in argv as the separator, even when "--" was used as an argument to an option (e.g., -I -- or --log-in --). The value can come from a variable, e.g., script -I "$VAR" will break if $VAR is --. Note that getopt_long() behavior with "--" depends on the option type: - required_argument: "--" is consumed as the option value - optional_argument: "--" is NOT consumed, treated as end-of-options - no_argument: "--" is treated as end-of-options Without "+" getopt also permutes arguments, which means non-option arguments before "--" (like the outfile) become inaccessible after getopt sets optind past "--". The pre-scan approach is necessary to split the command part out of argv before getopt runs. Fix the pre-scan by adding ul_find_argv_separator() to optutils.h. The helper ul_is_option_with_arg() checks if a given argv entry is an option that expects its value in the next argv entry, handling both long options (via o->name) and short/bundled options (via o->val). The separator search scans backward from each "--" counting consecutive options-with-required-arg. They pair up (each consumes the next as its value), so odd count means "--" is consumed as an option argument, even count means it is the real separator. This correctly handles chained cases like --log-in --log-in -- where the second --log-in is a value for the first and "--" is the separator. Addresses: 70507ab9eaed10b8dd77b77d4ea25c11ee726bed Reported-by: Evgeny Kapun Signed-off-by: Karel Zak --- diff --git a/include/optutils.h b/include/optutils.h index 87d20d0d53..906e9b475c 100644 --- a/include/optutils.h +++ b/include/optutils.h @@ -39,6 +39,98 @@ static inline const char *ul_get_shortopt(char *buf, size_t bufsz, int c) return buf; } +/* + * Check if @arg is an option whose value is expected in the next argv entry. + * Returns 1 for options with required_argument that don't have an inline + * value (i.e., not --name=val or bundled -Bval), 0 otherwise. + */ +static inline int ul_is_option_with_arg(const char *arg, + const struct option *opts) +{ + const struct option *o; + + if (arg[0] != '-' || !arg[1]) + return 0; + + if (arg[1] == '-') { + const char *name = arg + 2; + + if (!*name || strchr(name, '=')) + return 0; + for (o = opts; o->name; o++) { + if (o->has_arg == required_argument + && strcmp(name, o->name) == 0) + return 1; + } + return 0; + } + + /* short option(s): walk bundled chars; if an earlier char takes + * an argument, the rest of the string is consumed as its value */ + const char *p; + + for (p = arg + 1; *p; p++) { + for (o = opts; o->name; o++) { + if (o->val == *p) + break; + } + if (!o->name) + return 0; + if (o->has_arg != no_argument) + return !*(p + 1) && o->has_arg == required_argument; + } + + return 0; +} + +/* + * Find the "--" separator in argv[], ignoring "--" when it appears as an + * argument to an option that requires a value (e.g., -I -- or --log-in --). + * + * Note on getopt_long() behavior with "--": + * - required_argument: "--" is consumed as the option value, NOT treated + * as end-of-options (e.g., --log-in -- sets the value to "--") + * - optional_argument: "--" is NOT consumed as the value, it is treated + * as end-of-options (optarg is NULL) + * - no_argument: "--" is treated as end-of-options + * + * This function scans backward from each "--" counting consecutive + * options-with-required-arg. They pair up (each consumes the next as its + * value), so an odd count means "--" is consumed as an option value, an + * even count (including zero) means "--" is the real separator. + * + * For example: + * --log-in -- file odd (1), "--" is value for --log-in + * --log-in --log-in -- cmd even (2), "--" is separator + * + * Returns the index of "--" in argv, or -1 if not found. + */ +static inline int ul_find_argv_separator(int argc, char *const argv[], + const struct option *opts) +{ + int i; + + for (i = 1; i < argc; i++) { + int count, k; + + if (strcmp(argv[i], "--") != 0) + continue; + + for (count = 0, k = i - 1; k >= 1; k--) { + if (!ul_is_option_with_arg(argv[k], opts)) + break; + count++; + } + + if (count % 2 == 1) + continue; + + return i; + } + + return -1; +} + #ifndef OPTUTILS_EXIT_CODE # define OPTUTILS_EXIT_CODE EXIT_FAILURE #endif diff --git a/term-utils/script.c b/term-utils/script.c index 1da9402dd6..878717c2f7 100644 --- a/term-utils/script.c +++ b/term-utils/script.c @@ -840,13 +840,11 @@ int main(int argc, char **argv) * and command parts. This allows getopt to permute arguments * before "--" (backward compatible with "script file -c cmd"). */ - for (ch = 1; ch < argc; ch++) { - if (strcmp(argv[ch], "--") == 0) { - cmd_argv = argv + ch + 1; - cmd_argc = argc - ch - 1; - argc = ch; - break; - } + ch = ul_find_argv_separator(argc, argv, longopts); + if (ch > 0) { + cmd_argv = argv + ch + 1; + cmd_argc = argc - ch - 1; + argc = ch; } while ((ch = getopt_long(argc, argv, "aB:c:eE:fI:O:o:qm:T:t::Vh", longopts, NULL)) != -1) { diff --git a/tests/expected/script/options-dashdash-optarg-bundle b/tests/expected/script/options-dashdash-optarg-bundle new file mode 100644 index 0000000000..9242ad0a7c --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-bundle @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-bundle" ] +dashdash-bundle + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-optarg-chain b/tests/expected/script/options-dashdash-optarg-chain new file mode 100644 index 0000000000..29a952649a --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-chain @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-chain" ] +dashdash-chain + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-optarg-chain-even b/tests/expected/script/options-dashdash-optarg-chain-even new file mode 100644 index 0000000000..a0cc67232a --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-chain-even @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-chain-even" ] +dashdash-chain-even + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-optarg-long b/tests/expected/script/options-dashdash-optarg-long new file mode 100644 index 0000000000..a5cbf0c6b0 --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-long @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-long" ] +dashdash-long + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-optarg-odd b/tests/expected/script/options-dashdash-optarg-odd new file mode 100644 index 0000000000..b32d719e69 --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-odd @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-consumed" ] +dashdash-consumed + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-optarg-short b/tests/expected/script/options-dashdash-optarg-short new file mode 100644 index 0000000000..066da5f4d4 --- /dev/null +++ b/tests/expected/script/options-dashdash-optarg-short @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash-short" ] +dashdash-short + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/expected/script/options-dashdash-separator b/tests/expected/script/options-dashdash-separator new file mode 100644 index 0000000000..df23fd43db --- /dev/null +++ b/tests/expected/script/options-dashdash-separator @@ -0,0 +1,4 @@ +Script started on 2015-05-24 17:43:18+00:00 [COMMAND="echo dashdash" ] +dashdash + +Script done on 2015-05-24 17:43:18+00:00 [COMMAND_EXIT_CODE="0"] diff --git a/tests/ts/script/options b/tests/ts/script/options index 3fdbf83a89..35592a58d3 100755 --- a/tests/ts/script/options +++ b/tests/ts/script/options @@ -57,6 +57,52 @@ $TS_HELPER_SCRIPT --return --append -c "exit 127" "$TS_OUTPUT" /dev/ echo $? >> "$TS_OUTPUT" ts_finalize_subtest +# basic "--" separator: outfile before --, command after -- +ts_init_subtest "dashdash-separator" +$TS_HELPER_SCRIPT "$TS_OUTPUT" -- echo dashdash /dev/null 2>&1 +ts_finalize_subtest + +# "--" is consumed as required_argument value by --log-in (creates file +# named "--"), second "--" is the real separator +ts_init_subtest "dashdash-optarg-long" +$TS_HELPER_SCRIPT --log-in -- --log-out "$TS_OUTPUT" -- echo dashdash-long /dev/null 2>&1 +rm -f -- -- +ts_finalize_subtest + +# same as above but with short option -I +ts_init_subtest "dashdash-optarg-short" +$TS_HELPER_SCRIPT -I -- -O "$TS_OUTPUT" -- echo dashdash-short /dev/null 2>&1 +rm -f -- -- +ts_finalize_subtest + +# first --log-in takes second --log-in as filename (creates file named +# "--log-in"), --log-out takes $TS_OUTPUT, "--" is the separator +ts_init_subtest "dashdash-optarg-chain" +$TS_HELPER_SCRIPT --log-in --log-in --log-out "$TS_OUTPUT" -- echo dashdash-chain /dev/null 2>&1 +rm -f -- --log-in +ts_finalize_subtest + +# --log-in takes first "--" as filename (creates file named "--"), +# second "--" is the real separator +ts_init_subtest "dashdash-optarg-odd" +$TS_HELPER_SCRIPT -O "$TS_OUTPUT" --log-in -- -- echo dashdash-consumed /dev/null 2>&1 +rm -f -- -- +ts_finalize_subtest + +# first --log-in takes second --log-in as filename (creates file named +# "--log-in"), "--" is the separator +ts_init_subtest "dashdash-optarg-chain-even" +$TS_HELPER_SCRIPT -O "$TS_OUTPUT" --log-in --log-in -- echo dashdash-chain-even /dev/null 2>&1 +rm -f -- --log-in +ts_finalize_subtest + +# bundled short options: -B takes "c" as inline value (creates file named +# "c"), "--" is the separator +ts_init_subtest "dashdash-optarg-bundle" +$TS_HELPER_SCRIPT -Bc -- echo dashdash-bundle /dev/null 2>&1 +mv c "$TS_OUTPUT" +ts_finalize_subtest + ts_init_subtest "size" $TS_HELPER_SCRIPT --output-limit 9 --command "echo 1:1234567890" "$TS_OUTPUT" /dev/null 2>&1 $TS_HELPER_SCRIPT -a --output-limit 9 --command "echo 2:1234567890" "$TS_OUTPUT" /dev/null 2>&1