Re: [PATCH 6/6] fast-import: use early_scan_options() for --allow-unsafe-features
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 4, 2026, 03:38 UTC
- Message-ID
- <xmqqwlt18z3q.fsf@gitster.g>
- In-Reply-To
- <20260902161047.476753-7-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
Show 11 quoted lines
> The "feature" lines at the start of the stream are processed before the > command line options are parsed, so cmd_fast_import() scans its > arguments early to find out if `--allow-unsafe-features` was given. > > That scan doesn't know which options take their value as a separate > argument, and it stops at the first argument that doesn't start with a > dash. So it disagrees with parse_options(), which accepts values > separated from their option by a space, for a command line like > "--depth 5 --allow-unsafe-features": the scan stops at "5" and never > sees the option, so unsafe "feature" commands from the stream are > refused even though the option was given.
Well explained.
Show 18 quoted lines
> @@ -4218,23 +4235,20 @@ int cmd_fast_import(int argc,
> * line to override stream data). But we must do an early parse of any
> * command-line options that impact how we interpret the feature lines.
> *
> + * NEEDSWORK: This scan only matches the exact
> + * "--allow-unsafe-features" spelling, while parse_options() below
> + * also accepts unambiguous abbreviations, so the two disagree for
> + * a command line like "--allow-unsafe": parse_options() accepts
> + * the option, but this scan doesn't see it, so unsafe features
> + * from the stream are still refused. This errs on the safe side.
> */
> - for (int i = 1; i < argc; i++) {
> - const char *arg = argv[i];
> - if (*arg != '-' || !strcmp(arg, "--"))
> - break;
> - if (!strcmp(arg, "--allow-unsafe-features"))
> - state.allow_unsafe_features = 1;
> - }This is the ad-hoc one that does not know --depth takes a value after it.
Show 7 quoted lines
> + early = early_scan_options_from_options(fast_import_options, > + early_wanted); > + early_scan_options(argc - 1, argv + 1, early, > + EARLY_SCAN_STOP_AT_DASHDASH | > + EARLY_SCAN_STOP_AT_NON_OPTION, > + option_parse_early_allow_unsafe, &state); > + free(early);
Interesting. This one now "knows" enough to skip what comes after "--depth" that takes an option ;-) And it is perfectly fine if we skip over "--depth hello" to find "--allow-unsafe", as such a "oops we require number but hello is not a number" will be caught by the real parser anyway.
Nicely done.