revision: parse integer arguments to --max-count, --skip, etc., more carefully
The "rev-list" and other commands in the "log" family, being the oldest part of the system, use their own custom argument parsers, and integer values of some options are parsed with atoi(), which allows a non-digit after the number (e.g., "1q") to be silently ignored. As a natural consequence, an argument that does not begin with a digit (e.g., "q") silently becomes zero, too. Switch to use strtol_i() and parse_timestamp() appropriately to catch bogus input. Note that one may naïvely expect that --max-count, --skip, etc., to only take non-negative values, but we must allow them to also take negative values, as an escape hatch to countermand a limit set by an earlier option on the command line; the underlying variables are initialized to (-1) and "--max-count=-1", for example, is a legitimate way to reinitialize the limit. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Dec 9, 2023 at 07:35 UTC
71a1e94821666909b7b2bd62a36244c601f8430e
3 files changed
+57
-13
revision.c
+30
-11
@@ -2214,6 +2214,27 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)
2214
add_grep(revs, pattern, GREP_PATTERN_BODY);
2215
}
2216
2217
+static int parse_count(const char *arg)
2218
+{
2219
+ int count;
2220
+
2221
+ if (strtol_i(arg, 10, &count) < 0)
2222
+ die("'%s': not an integer", arg);
2223
+ return count;
2224
+}
2225
+
2226
+static timestamp_t parse_age(const char *arg)
2227
+{
2228
+ timestamp_t num;
2229
+ char *p;
2230
+
2231
+ errno = 0;
2232
+ num = parse_timestamp(arg, &p, 10);
2233
+ if (errno || *p || p == arg)
2234
+ die("'%s': not a number of seconds since epoch", arg);
2235
+ return num;
2236
+}
2237
+
2238
static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
2239
int *unkc, const char **unkv,
2240
const struct setup_revision_opt* opt)
@@ -2240,29 +2261,27 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
2261
}
2262
2263
if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
2243
- revs->max_count = atoi(optarg);
2264
+ revs->max_count = parse_count(optarg);
2265
revs->no_walk = 0;
2266
return argcount;
2267
} else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
2247
- revs->skip_count = atoi(optarg);
2268
+ revs->skip_count = parse_count(optarg);
2269
return argcount;
2270
} else if ((*arg == '-') && isdigit(arg[1])) {
2271
/* accept -<digit>, like traditional "head" */
2251
- if (strtol_i(arg + 1, 10, &revs->max_count) < 0 ||
2252
- revs->max_count < 0)
2253
- die("'%s': not a non-negative integer", arg + 1);
2272
+ revs->max_count = parse_count(arg + 1);
2273
revs->no_walk = 0;
2274
} else if (!strcmp(arg, "-n")) {
2275
if (argc <= 1)
2276
return error("-n requires an argument");
2258
- revs->max_count = atoi(argv[1]);
2277
+ revs->max_count = parse_count(argv[1]);
2278
revs->no_walk = 0;
2279
return 2;
2280
} else if (skip_prefix(arg, "-n", &optarg)) {
2262
- revs->max_count = atoi(optarg);
2281
+ revs->max_count = parse_count(optarg);
2282
revs->no_walk = 0;
2283
} else if ((argcount = parse_long_opt("max-age", argv, &optarg))) {
2265
- revs->max_age = atoi(optarg);
2284
+ revs->max_age = parse_age(optarg);
2285
return argcount;
2286
} else if ((argcount = parse_long_opt("since", argv, &optarg))) {
2287
revs->max_age = approxidate(optarg);
@@ -2274,7 +2293,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
2293
revs->max_age = approxidate(optarg);
2294
return argcount;
2295
} else if ((argcount = parse_long_opt("min-age", argv, &optarg))) {
2277
- revs->min_age = atoi(optarg);
2296
+ revs->min_age = parse_age(optarg);
2297
return argcount;
2298
} else if ((argcount = parse_long_opt("before", argv, &optarg))) {
2299
revs->min_age = approxidate(optarg);
@@ -2362,11 +2381,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
2381
} else if (!strcmp(arg, "--no-merges")) {
2382
revs->max_parents = 1;
2383
} else if (skip_prefix(arg, "--min-parents=", &optarg)) {
2365
- revs->min_parents = atoi(optarg);
2384
+ revs->min_parents = parse_count(optarg);
2385
} else if (!strcmp(arg, "--no-min-parents")) {
2386
revs->min_parents = 0;
2387
} else if (skip_prefix(arg, "--max-parents=", &optarg)) {
2369
- revs->max_parents = atoi(optarg);
2388
+ revs->max_parents = parse_count(optarg);
2389
} else if (!strcmp(arg, "--no-max-parents")) {
2390
revs->max_parents = -1;
2391
} else if (!strcmp(arg, "--boundary")) {
t/t6005-rev-list-count.sh
+16
-2
@@ -18,20 +18,34 @@ test_expect_success 'no options' '
18
'
19
20
test_expect_success '--max-count' '
21
+ test_must_fail git rev-list --max-count=1q HEAD 2>error &&
22
+ grep "not an integer" error &&
23
+
24
test_stdout_line_count = 0 git rev-list HEAD --max-count=0 &&
25
test_stdout_line_count = 3 git rev-list HEAD --max-count=3 &&
26
test_stdout_line_count = 5 git rev-list HEAD --max-count=5 &&
24
- test_stdout_line_count = 5 git rev-list HEAD --max-count=10
27
+ test_stdout_line_count = 5 git rev-list HEAD --max-count=10 &&
28
+ test_stdout_line_count = 5 git rev-list HEAD --max-count=-1
29
'
30
31
test_expect_success '--max-count all forms' '
32
+ test_must_fail git rev-list -1q HEAD 2>error &&
33
+ grep "not an integer" error &&
34
+ test_must_fail git rev-list --1 HEAD &&
35
+ test_must_fail git rev-list -n 1q HEAD 2>error &&
36
+ grep "not an integer" error &&
37
+
38
test_stdout_line_count = 1 git rev-list HEAD --max-count=1 &&
39
test_stdout_line_count = 1 git rev-list HEAD -1 &&
40
test_stdout_line_count = 1 git rev-list HEAD -n1 &&
31
- test_stdout_line_count = 1 git rev-list HEAD -n 1
41
+ test_stdout_line_count = 1 git rev-list HEAD -n 1 &&
42
+ test_stdout_line_count = 5 git rev-list HEAD -n -1
43
'
44
45
test_expect_success '--skip' '
46
+ test_must_fail git rev-list --skip 1q HEAD 2>error &&
47
+ grep "not an integer" error &&
48
+
49
test_stdout_line_count = 5 git rev-list HEAD --skip=0 &&
50
test_stdout_line_count = 2 git rev-list HEAD --skip=3 &&
51
test_stdout_line_count = 0 git rev-list HEAD --skip=5 &&
t/t6009-rev-list-parent.sh
+11
@@ -62,6 +62,17 @@ test_expect_success 'setup roots, merges and octopuses' '
62
git checkout main
63
'
64
65
+test_expect_success 'parse --max-parents & --min-parents' '
66
+ test_must_fail git rev-list --max-parents=1q HEAD 2>error &&
67
+ grep "not an integer" error &&
68
+
69
+ test_must_fail git rev-list --min-parents=1q HEAD 2>error &&
70
+ grep "not an integer" error &&
71
+
72
+ git rev-list --max-parents=1 --min-parents=1 HEAD &&
73
+ git rev-list --max-parents=-1 --min-parents=-1 HEAD
74
+'
75
+
76
test_expect_success 'rev-list roots' '
77
78
check_revlist "--max-parents=0" one five