[PATCH v2 2/3] config: require at least one digit when parsing numbers

From: Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date: Nov 9, 2022, 14:16 UTC
Message-ID: <54f2ebefa0d8308dc00f6dd035c1baae4a8e1fc7.1668003388.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.1389.v2.git.1668003388.gitgitgadget@gmail.com>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If the input to strtoimax() or strtoumax() does not contain any digits then they return zero and set `end` to point to the start of the input string. git_parse_[un]signed() do not check `end` and so fail to return an error and instead return a value of zero if the input string is a valid units factor without any digits (e.g "k").

Tests are added to check that 'git config --int' and OPT_MAGNITUDE() reject a units specifier without a leading digit.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 config.c                 | 8 ++++++++
 t/t0040-parse-options.sh | 7 +++++++
 t/t1300-config.sh        | 6 ++++++
 3 files changed, 21 insertions(+)

diff --git a/config.c b/config.c
index d5069d4f01d..b7fb68026d8 100644
--- a/config.c
+++ b/config.c
@@ -1167,6 +1167,10 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
 		val = strtoimax(value, &end, 0);
 		if (errno == ERANGE)
 			return 0;
+		if (end == value) {
+			errno = EINVAL;
+			return 0;
+		}
 		factor = get_unit_factor(end);
 		if (!factor) {
 			errno = EINVAL;
@@ -1202,6 +1206,10 @@ static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
 		val = strtoumax(value, &end, 0);
 		if (errno == ERANGE)
 			return 0;
+		if (end == value) {
+			errno = EINVAL;
+			return 0;
+		}
 		factor = get_unit_factor(end);
 		if (!factor) {
 			errno = EINVAL;
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 64d2327b744..7d7ecfd5716 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -714,4 +714,11 @@ test_expect_success 'negative magnitude' '
 	grep "non-negative integer" err &&
 	test_must_be_empty out
 '
+
+test_expect_success 'magnitude with units but no numbers' '
+	test_must_fail test-tool parse-options --magnitude m >out 2>err &&
+	grep "non-negative integer" err &&
+	test_must_be_empty out
+'
+
 test_done
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index c6661e61af5..2575279ab84 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2228,6 +2228,12 @@ test_expect_success '--type rejects unknown specifiers' '
 	test_i18ngrep "unrecognized --type argument" error
 '
 
+test_expect_success '--type=int requires at least one digit' '
+	test_must_fail git config --type int --default m some.key >out 2>error &&
+	grep "bad numeric config value" error &&
+	test_must_be_empty out
+'
+
 test_expect_success '--replace-all does not invent newlines' '
 	q_to_tab >.git/config <<-\EOF &&
 	[abc]key

-- 
gitgitgadget