* [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-09 15:07 ` Phillip Wood
2026-11-07 8:42 ` [PATCH 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
` (16 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A rebase.missingCommitsCheck without a value, as in
[rebase]
missingCommitsCheck
or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
segfault as it writes the todo list: get_missing_commit_check_level()
passes the NULL value to strcasecmp().
The setting takes "ignore", "warn" or "error", not a bool. Read it
with repo_config_get_string_tmp(), which rejects a missing value with
config_error_nonbool(), as ba176db511 (config: handle NULL value when
parsing non-bools, 2023-12-07) did for other settings.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
rebase-interactive.c | 2 +-
t/t3404-rebase-interactive.sh | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 809f76a87b..4eb628ee8f 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -30,7 +30,7 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
{
const char *value;
- if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
+ if (repo_config_get_string_tmp(the_repository, "rebase.missingcommitscheck", &value) ||
!strcasecmp("ignore", value))
return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ff11abb2f2..813ad2a83d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1562,6 +1562,13 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'rebase -i rejects rebase.missingCommitsCheck without a value' '
+ rebase_setup_and_clean missing-commit &&
+ test_when_finished "test_might_fail git rebase --abort" &&
+ test_must_fail git -c rebase.missingCommitsCheck rebase -i --root 2>err &&
+ test_grep "missing value for .rebase.missingcommitscheck." err
+'
+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
test_config rebase.missingCommitsCheck ignore &&
rebase_setup_and_clean missing-commit &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-07 8:42 ` [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
@ 2026-11-09 15:07 ` Phillip Wood
2026-11-09 19:45 ` Junio C Hamano
0 siblings, 1 reply; 69+ messages in thread
From: Phillip Wood @ 2026-11-09 15:07 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> A rebase.missingCommitsCheck without a value, as in
>
> [rebase]
> missingCommitsCheck
>
> or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
> segfault as it writes the todo list: get_missing_commit_check_level()
> passes the NULL value to strcasecmp().
>
> The setting takes "ignore", "warn" or "error", not a bool. Read it
> with repo_config_get_string_tmp(), which rejects a missing value with
> config_error_nonbool(), as ba176db511 (config: handle NULL value when
> parsing non-bools, 2023-12-07) did for other settings.
If the user sets an invalid value for this config option then we print a
warning and default to "ignore". It would seem reasonable to me to do
the same for a missing value rather than erroring out.
Thanks for fixing these segfaults
Phillip
>
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
> ---
> rebase-interactive.c | 2 +-
> t/t3404-rebase-interactive.sh | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index 809f76a87b..4eb628ee8f 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -30,7 +30,7 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
> {
> const char *value;
>
> - if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
> + if (repo_config_get_string_tmp(the_repository, "rebase.missingcommitscheck", &value) ||
> !strcasecmp("ignore", value))
> return MISSING_COMMIT_CHECK_IGNORE;
> if (!strcasecmp("warn", value))
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff11abb2f2..813ad2a83d 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1562,6 +1562,13 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> +test_expect_success 'rebase -i rejects rebase.missingCommitsCheck without a value' '
> + rebase_setup_and_clean missing-commit &&
> + test_when_finished "test_might_fail git rebase --abort" &&
> + test_must_fail git -c rebase.missingCommitsCheck rebase -i --root 2>err &&
> + test_grep "missing value for .rebase.missingcommitscheck." err
> +'
> +
> test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
> test_config rebase.missingCommitsCheck ignore &&
> rebase_setup_and_clean missing-commit &&
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-09 15:07 ` Phillip Wood
@ 2026-11-09 19:45 ` Junio C Hamano
2026-11-09 20:03 ` Thomas Bachem
0 siblings, 1 reply; 69+ messages in thread
From: Junio C Hamano @ 2026-11-09 19:45 UTC (permalink / raw)
To: Phillip Wood
Cc: Thomas Bachem via GitGitGadget, git, Phillip Wood, Jeff King,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Colin Hinton,
Matt Hunter, Johannes Schindelin, Usman Akinyemi, Miklos Vajna,
Thomas Bachem
Phillip Wood <phillip.wood123@gmail.com> writes:
>> A rebase.missingCommitsCheck without a value, as in
>>
>> [rebase]
>> missingCommitsCheck
>>
>> or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
>> segfault as it writes the todo list: get_missing_commit_check_level()
>> passes the NULL value to strcasecmp().
>>
>> The setting takes "ignore", "warn" or "error", not a bool. Read it
>> with repo_config_get_string_tmp(), which rejects a missing value with
>> config_error_nonbool(), as ba176db511 (config: handle NULL value when
>> parsing non-bools, 2023-12-07) did for other settings.
>
> If the user sets an invalid value for this config option then we print a
> warning and default to "ignore". It would seem reasonable to me to do
> the same for a missing value rather than erroring out.
Interesting point. I think ignoring a textual invalid value is
mostly for future-proofing (a configuration item written for newer
version of Git added while trying it out is seen by the version of
Git the user regularly uses). It may be arguable if warn-and-ignore
is the most sensible option, but I fully agree with you that
behaving exactly the same way on valueless true, which is not a
valid setting for the current version of Git, would be a sensible
thing to do.
It seems with your and Derrick's review, this is the only thing that
needs to be updated in v2?
Thanks.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-09 19:45 ` Junio C Hamano
@ 2026-11-09 20:03 ` Thomas Bachem
0 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem @ 2026-11-09 20:03 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood, Thomas Bachem via GitGitGadget, git, Phillip Wood,
Jeff King, Taylor Blau, Derrick Stolee, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna
Junio C Hamano <gitster@pobox.com> writes:
> It seems with your and Derrick's review, this is the only thing that
> needs to be updated in v2?
Yes, v2 will change only that. A missing value will then warn and fall
back to "ignore" just like an invalid one. Thanks, Phillip and Junio.
Thomas
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 02/15] odb: handle NULL values for core.alternateRefs* config
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
` (15 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A core.alternateRefsCommand or core.alternateRefsPrefixes without a
value makes "git fetch", "git receive-pack" and
"git rev-list --alternate-refs" segfault in a repository that borrows
objects from an alternate: fill_alternate_refs_command() passes the
NULL value to strvec_push() or strvec_split().
Both settings take a string. Read them with
repo_config_get_string_tmp(), which rejects a missing value with
config_error_nonbool(), as the previous commit does for
rebase.missingCommitsCheck.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
odb.c | 4 ++--
t/t5618-alternate-refs.sh | 9 +++++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/odb.c b/odb.c
index 0200e26f21..ed59cfc989 100644
--- a/odb.c
+++ b/odb.c
@@ -379,7 +379,7 @@ static void fill_alternate_refs_command(struct repository *repo,
{
const char *value;
- if (!repo_config_get_value(repo, "core.alternateRefsCommand", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsCommand", &value)) {
cmd->use_shell = 1;
strvec_push(&cmd->args, value);
@@ -391,7 +391,7 @@ static void fill_alternate_refs_command(struct repository *repo,
strvec_push(&cmd->args, "for-each-ref");
strvec_push(&cmd->args, "--format=%(objectname)");
- if (!repo_config_get_value(repo, "core.alternateRefsPrefixes", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsPrefixes", &value)) {
strvec_push(&cmd->args, "--");
strvec_split(&cmd->args, value);
}
diff --git a/t/t5618-alternate-refs.sh b/t/t5618-alternate-refs.sh
index 2fb6d549d3..13abd81e98 100755
--- a/t/t5618-alternate-refs.sh
+++ b/t/t5618-alternate-refs.sh
@@ -58,4 +58,13 @@ test_expect_success 'log --source shows .alternate marker' '
test_cmp expect actual
'
+test_expect_success 'alternateRefsCommand and alternateRefsPrefixes need a value' '
+ test_must_fail git -c core.alternateRefsCommand \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsCommand." err &&
+ test_must_fail git -c core.alternateRefsPrefixes \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsPrefixes." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 03/15] add-patch: handle NULL value for color slots
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
` (14 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
With color on, a color slot without a value, such as
color.interactive.prompt or color.diff.frag, makes "git add -p",
"git add -i" and the other interactive patch modes segfault:
init_color() passes the NULL value to color_parse().
Read the slot with repo_config_get_string_tmp(), which rejects a
missing value with config_error_nonbool(), as "git diff" already does
for color.diff.frag and the other diff colors.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
add-patch.c | 2 +-
t/t3701-add-interactive.sh | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/add-patch.c b/add-patch.c
index f27edcbe8d..ad581eb4ae 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -295,7 +295,7 @@ static void init_color(struct repository *r,
if (!want_color(use_color))
dst[0] = '\0';
- else if (repo_config_get_value(r, key, &value) ||
+ else if (repo_config_get_string_tmp(r, key, &value) ||
color_parse(value, dst))
strlcpy(dst, default_color, COLOR_MAXLEN);
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6e120a4001..33050ccc53 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -815,6 +815,18 @@ test_expect_success 'colors can be overridden' '
test_cmp expect actual
'
+test_expect_success 'a color without a value is rejected' '
+ git reset --hard &&
+ test_when_finished "git rm -f color-test" &&
+ test_write_lines context old >color-test &&
+ git add color-test &&
+ test_write_lines context new >color-test &&
+
+ test_must_fail git -c color.interactive=always \
+ -c color.interactive.prompt add -p </dev/null 2>err &&
+ test_grep "missing value for .color.interactive.prompt." err
+'
+
test_expect_success 'brackets appear without color' '
git reset --hard &&
test_when_finished "git rm -f bracket-test" &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (2 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-09 13:39 ` Derrick Stolee
2026-11-07 8:42 ` [PATCH 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
` (13 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A fetch.bundleCreationToken without a value makes "git fetch" and
"git clone --bundle-uri" segfault when the bundle list uses the
creationToken heuristic: fetch_bundles_by_token() passes the NULL
value to sscanf().
The function already treats a token that does not parse as 0, as if
no bundles had been fetched before, so it downloads them and then
stores the new token. Treat a missing value the same way.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 3 ++-
t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/bundle-uri.c b/bundle-uri.c
index 2bb2eb99e4..7de17634cf 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -572,7 +572,8 @@ static int fetch_bundles_by_token(struct repository *r,
if (!repo_config_get_value(r,
"fetch.bundlecreationtoken",
&creationTokenStr)) {
- if (sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
+ if (!creationTokenStr ||
+ sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
maxCreationToken = 0;
if (bundles.items[0]->creationToken <= maxCreationToken) {
free(bundles.items);
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index f81f1f8db7..e86ed7d30c 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -346,6 +346,26 @@ test_expect_success 'clone bundle list (file, any mode, all failures)' '
test_grep ! "refs/bundles/heads/" refs
'
+test_expect_success 'fetch treats fetch.bundleCreationToken without a value as 0' '
+ test_when_finished "rm -rf token-without-value" &&
+ cat >bundle-list <<-EOF &&
+ [bundle]
+ version = 1
+ mode = all
+ heuristic = creationToken
+
+ [bundle "bundle-1"]
+ uri = file://$(pwd)/clone-from/bundle-1.bundle
+ creationToken = 1
+ EOF
+
+ git clone --no-local clone-from token-without-value &&
+ git -C token-without-value \
+ -c fetch.bundleURI="file://$(pwd)/bundle-list" \
+ -c fetch.bundleCreationToken fetch origin &&
+ test_cmp_config -C token-without-value 1 fetch.bundlecreationtoken
+'
+
test_expect_success 'negotiation: bundle with part of wanted commits' '
test_when_finished "rm -f trace*.txt" &&
GIT_TRACE_PACKET="$(pwd)/trace-packet.txt" \
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken
2026-11-07 8:42 ` [PATCH 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
@ 2026-11-09 13:39 ` Derrick Stolee
0 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-09 13:39 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Junio C Hamano, Thomas Bachem
On 11/7/2026 3:42 AM, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> A fetch.bundleCreationToken without a value makes "git fetch" and
> "git clone --bundle-uri" segfault when the bundle list uses the
> creationToken heuristic: fetch_bundles_by_token() passes the NULL
> value to sscanf().
>
> The function already treats a token that does not parse as 0, as if
> no bundles had been fetched before, so it downloads them and then
> stores the new token. Treat a missing value the same way.
I agree that this is the correct fallback.
> - if (sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
> + if (!creationTokenStr ||
> + sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
> maxCreationToken = 0;
Good.
> +test_expect_success 'fetch treats fetch.bundleCreationToken without a value as 0' '
> + test_when_finished "rm -rf token-without-value" &&
> + cat >bundle-list <<-EOF &&
> + [bundle]
> + version = 1
> + mode = all
> + heuristic = creationToken
> +
> + [bundle "bundle-1"]
> + uri = file://$(pwd)/clone-from/bundle-1.bundle
> + creationToken = 1
> + EOF
I looked at your patches out-of-order, and you have some later ones that
focus on this bundle list having bad values. However, the real test is
in the fetch:
> + git clone --no-local clone-from token-without-value &&
> + git -C token-without-value \
> + -c fetch.bundleURI="file://$(pwd)/bundle-list" \
> + -c fetch.bundleCreationToken fetch origin &&
Here, the local creation token is NULL and that causes the problem.
> + test_cmp_config -C token-without-value 1 fetch.bundlecreationtoken
> +'
> +
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 05/15] grep: handle NULL value for grep.patternType
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (3 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
` (12 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A grep.patternType without a value makes "git grep", "git log" and
"git show" segfault: grep_config(), which all three use, passes the
NULL value to parse_pattern_type_arg(), which hands it to strcmp().
Reject a missing value with config_error_nonbool(), as grep_config()
already does for the color.grep.* slots.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
grep.c | 2 ++
t/t7810-grep.sh | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/grep.c b/grep.c
index 1d75d31421..ab082f963e 100644
--- a/grep.c
+++ b/grep.c
@@ -71,6 +71,8 @@ int grep_config(const char *var, const char *value,
}
if (!strcmp(var, "grep.patterntype")) {
+ if (!value)
+ return config_error_nonbool(var);
opt->pattern_type_option = parse_pattern_type_arg(var, value);
return 0;
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index ac7ea9cf8d..d323927e54 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1551,6 +1551,11 @@ test_expect_success PCRE 'grep invalidpattern properly dies with grep.patternTyp
test_must_fail git -c grep.patterntype=perl grep "a["
'
+test_expect_success 'grep.patternType without a value is rejected' '
+ test_must_fail git -c grep.patternType grep "a" 2>err &&
+ test_grep "missing value for .grep.patterntype." err
+'
+
test_expect_success 'grep -G -E -F pattern' '
echo "ab:a+b*c" >expected &&
git grep -G -E -F "a+b*c" ab >actual &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 06/15] remote: handle NULL value for remote.<name>.tagOpt
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (4 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-10 11:14 ` Phillip Wood
2026-11-07 8:42 ` [PATCH 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
` (11 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remote.<name>.tagOpt without a value makes "git fetch",
"git remote" and "git status" segfault: handle_config() passes the
NULL value to strcmp().
Reject a missing value with config_error_nonbool(), as handle_config()
already does for remote.<name>.url and remote.<name>.pushurl.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 2 ++
t/t5505-remote.sh | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/remote.c b/remote.c
index 71170f36a9..024cfc547d 100644
--- a/remote.c
+++ b/remote.c
@@ -556,6 +556,8 @@ static int handle_config(const char *key, const char *value,
else
error(_("more than one uploadpack given, using the first"));
} else if (!strcmp(subkey, "tagopt")) {
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "--no-tags"))
remote->fetch_tags = -1;
else if (!strcmp(value, "--tags"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 5fbfcb0848..bec2e51afa 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -90,6 +90,11 @@ test_expect_success 'remote information for the origin' '
)
'
+test_expect_success 'tagOpt needs a value' '
+ test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
+ test_grep "missing value for .remote.origin.tagopt." err
+'
+
test_expect_success 'add another remote' '
(
cd test &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 06/15] remote: handle NULL value for remote.<name>.tagOpt
2026-11-07 8:42 ` [PATCH 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
@ 2026-11-10 11:14 ` Phillip Wood
0 siblings, 0 replies; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:14 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
> diff --git a/remote.c b/remote.c
> index 71170f36a9..024cfc547d 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -556,6 +556,8 @@ static int handle_config(const char *key, const char *value,
> else
> error(_("more than one uploadpack given, using the first"));
> } else if (!strcmp(subkey, "tagopt")) {
> + if (!value)
> + return config_error_nonbool(key);
> if (!strcmp(value, "--no-tags"))
> remote->fetch_tags = -1;
> else if (!strcmp(value, "--tags"))
remote->fetch_tags = 2;
} else if (!strcmp(subkey, "proxy")) {
We silently ignore invalid values which seems unfortunate, we could just
print a warning for a missing value rather than dying, but at least with
this change we are reporting an error.
Thanks
Phillip
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 5fbfcb0848..bec2e51afa 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -90,6 +90,11 @@ test_expect_success 'remote information for the origin' '
> )
> '
>
> +test_expect_success 'tagOpt needs a value' '
> + test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
> + test_grep "missing value for .remote.origin.tagopt." err
> +'
> +
> test_expect_success 'add another remote' '
> (
> cd test &&
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 07/15] blame: handle NULL values for color.blame.* config
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (5 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-10 11:14 ` Phillip Wood
2026-11-07 8:42 ` [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
` (10 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A color.blame.repeatedLines or color.blame.highlightRecent without a
value makes "git blame" segfault: git_blame_config() passes the NULL
value to strlen() or to parse_color_fields().
Reject a missing value with config_error_nonbool(), as
git_blame_config() already does for blame.date and blame.coloring.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/blame.c | 4 ++++
t/t8012-blame-colors.sh | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/builtin/blame.c b/builtin/blame.c
index 48d5251c6d..653449c0c9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -782,12 +782,16 @@ static int git_blame_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "color.blame.repeatedlines")) {
+ if (!value)
+ return config_error_nonbool(var);
if (color_parse_mem(value, strlen(value), repeated_meta_color))
warning(_("invalid value for '%s': '%s'"),
"color.blame.repeatedLines", value);
return 0;
}
if (!strcmp(var, "color.blame.highlightrecent")) {
+ if (!value)
+ return config_error_nonbool(var);
parse_color_fields(value);
return 0;
}
diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
index d2653b3c31..5eeb7338a3 100755
--- a/t/t8012-blame-colors.sh
+++ b/t/t8012-blame-colors.sh
@@ -69,4 +69,11 @@ test_expect_success 'blame color by age: new code is different' '
test_grep qfunc colored
'
+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
+ test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.repeatedlines." err &&
+ test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.highlightrecent." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 07/15] blame: handle NULL values for color.blame.* config
2026-11-07 8:42 ` [PATCH 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
@ 2026-11-10 11:14 ` Phillip Wood
0 siblings, 0 replies; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:14 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 48d5251c6d..653449c0c9 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -782,12 +782,16 @@ static int git_blame_config(const char *var, const char *value,
> return 0;
> }
> if (!strcmp(var, "color.blame.repeatedlines")) {
> + if (!value)
> + return config_error_nonbool(var);
> if (color_parse_mem(value, strlen(value), repeated_meta_color))
> warning(_("invalid value for '%s': '%s'"),
> "color.blame.repeatedLines", value);
> return 0;
We ignore an invalid value, so does it make sense to error out on a
missing value? The existing warning does get a bit lost though as it is
printed before the blame output.
> }
> if (!strcmp(var, "color.blame.highlightrecent")) {
> + if (!value)
> + return config_error_nonbool(var);
> parse_color_fields(value);
> return 0;
Same here
Thanks
Phillip
}
> diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
> index d2653b3c31..5eeb7338a3 100755
> --- a/t/t8012-blame-colors.sh
> +++ b/t/t8012-blame-colors.sh
> @@ -69,4 +69,11 @@ test_expect_success 'blame color by age: new code is different' '
> test_grep qfunc colored
> '
>
> +test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
> + test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
> + test_grep "missing value for .color.blame.repeatedlines." err &&
> + test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
> + test_grep "missing value for .color.blame.highlightrecent." err
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (6 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-10 11:12 ` Phillip Wood
2026-11-07 8:42 ` [PATCH 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
` (9 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A branch.<name>.mergeOptions without a value makes "git merge"
segfault on that branch: git_merge_config() passes the NULL value to
xstrdup().
Reject a missing value with config_error_nonbool(), as
git_config_string() already does for the other strings that
git_merge_config() reads.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/merge.c | 2 ++
t/t7600-merge.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 5b4eb23a83..fbabda97f8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
skip_prefix(k, "branch.", &str) &&
skip_prefix(str, branch, &str) &&
!strcmp(str, ".mergeoptions")) {
+ if (!v)
+ return config_error_nonbool(k);
free(branch_mergeoptions);
branch_mergeoptions = xstrdup(v);
return 0;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 64fe21717d..d4963f1fee 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -552,6 +552,12 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
test_cmp squash.1-5 .git/SQUASH_MSG
'
+test_expect_success 'merge rejects branch.main.mergeOptions without a value' '
+ git reset --hard c1 &&
+ test_must_fail git -c branch.main.mergeOptions merge c2 2>err &&
+ test_grep "missing value for .branch.main.mergeoptions." err
+'
+
test_debug 'git log --graph --decorate --oneline --all'
test_expect_success 'override config option -n with --summary' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-07 8:42 ` [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
@ 2026-11-10 11:12 ` Phillip Wood
2026-11-10 16:53 ` Junio C Hamano
0 siblings, 1 reply; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:12 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 5b4eb23a83..fbabda97f8 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
> skip_prefix(k, "branch.", &str) &&
> skip_prefix(str, branch, &str) &&
> !strcmp(str, ".mergeoptions")) {
> + if (!v)
> + return config_error_nonbool(k);
> free(branch_mergeoptions);
> branch_mergeoptions = xstrdup(v);
> return 0;
We treat an empty value differently to a missing value, I'm not sure if
that makes sense or not.
Thanks
Phillip
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 64fe21717d..d4963f1fee 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -552,6 +552,12 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
> test_cmp squash.1-5 .git/SQUASH_MSG
> '
>
> +test_expect_success 'merge rejects branch.main.mergeOptions without a value' '
> + git reset --hard c1 &&
> + test_must_fail git -c branch.main.mergeOptions merge c2 2>err &&
> + test_grep "missing value for .branch.main.mergeoptions." err
> +'
> +
> test_debug 'git log --graph --decorate --oneline --all'
>
> test_expect_success 'override config option -n with --summary' '
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-10 11:12 ` Phillip Wood
@ 2026-11-10 16:53 ` Junio C Hamano
2026-11-11 14:47 ` Phillip Wood
0 siblings, 1 reply; 69+ messages in thread
From: Junio C Hamano @ 2026-11-10 16:53 UTC (permalink / raw)
To: Phillip Wood
Cc: Thomas Bachem via GitGitGadget, git, Phillip Wood, Jeff King,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Colin Hinton,
Matt Hunter, Johannes Schindelin, Usman Akinyemi, Miklos Vajna,
Thomas Bachem
Phillip Wood <phillip.wood123@gmail.com> writes:
> Hi Thomas
>
> On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index 5b4eb23a83..fbabda97f8 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
>> skip_prefix(k, "branch.", &str) &&
>> skip_prefix(str, branch, &str) &&
>> !strcmp(str, ".mergeoptions")) {
>> + if (!v)
>> + return config_error_nonbool(k);
>> free(branch_mergeoptions);
>> branch_mergeoptions = xstrdup(v);
>> return 0;
>
> We treat an empty value differently to a missing value, I'm not sure if
> that makes sense or not.
Sorry but I do not understand this comment.
[branch "main"] mergeOptions =
is an explicit "there is no extra options when we make a merge".
[branch "main"] mergeOptions
is an attempt to set a configuration variable to boolean "true".
I am not sure if it helps users to pretend as if the latter were
[branch "main"] mergeOptions =
if that is what you are hinting. It would confuse them when they
see a true boolean variable
[branch "main"] rebase
and wonder why
[branch "main"] rebase =
does not behave the same way, no?
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-10 16:53 ` Junio C Hamano
@ 2026-11-11 14:47 ` Phillip Wood
2026-11-11 15:26 ` Ben Knoble
2026-11-11 17:50 ` Junio C Hamano
0 siblings, 2 replies; 69+ messages in thread
From: Phillip Wood @ 2026-11-11 14:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Bachem via GitGitGadget, git, Phillip Wood, Jeff King,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Colin Hinton,
Matt Hunter, Johannes Schindelin, Usman Akinyemi, Miklos Vajna,
Thomas Bachem
On 10/11/2026 16:53, Junio C Hamano wrote:
>
> Sorry but I do not understand this comment.
>
> [branch "main"] mergeOptions =
>
> is an explicit "there is no extra options when we make a merge".
>
> [branch "main"] mergeOptions
>
> is an attempt to set a configuration variable to boolean "true".
Oh, I'd forgotten we treat a missing value as "true" even for config
settings that expect a non-boolen value.
> I am not sure if it helps users to pretend as if the latter were
>
> [branch "main"] mergeOptions =
>
> if that is what you are hinting. It would confuse them when they
> see a true boolean variable
>
> [branch "main"] rebase
>
> and wonder why
>
> [branch "main"] rebase =
>
> does not behave the same way, no?
To me confusing thing is that we don't treat those two the same way -
I'd find it much easier to reason about if empty and missing values were
always treated the same and booleans required an explicit value, but it
is too late to change that now.
Thanks
Phillip
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-11 14:47 ` Phillip Wood
@ 2026-11-11 15:26 ` Ben Knoble
2026-11-11 17:50 ` Junio C Hamano
1 sibling, 0 replies; 69+ messages in thread
From: Ben Knoble @ 2026-11-11 15:26 UTC (permalink / raw)
To: Phillip Wood
Cc: Junio C Hamano, Thomas Bachem via GitGitGadget, git, Phillip Wood,
Jeff King, Taylor Blau, Derrick Stolee, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Thomas Bachem
> Le 11 nov. 2026 à 09:47, Phillip Wood <phillip.wood123@gmail.com> a écrit :
>
> On 10/11/2026 16:53, Junio C Hamano wrote:
>> Sorry but I do not understand this comment.
>> [branch "main"] mergeOptions =
>> is an explicit "there is no extra options when we make a merge".
>> [branch "main"] mergeOptions
>> is an attempt to set a configuration variable to boolean "true".
>
> Oh, I'd forgotten we treat a missing value as "true" even for config settings that expect a non-boolen value.
>
>> I am not sure if it helps users to pretend as if the latter were
>> [branch "main"] mergeOptions =
>> if that is what you are hinting. It would confuse them when they
>> see a true boolean variable
>> [branch "main"] rebase
>> and wonder why
>> [branch "main"] rebase =
>> does not behave the same way, no?
>
> To me confusing thing is that we don't treat those two the same way - I'd find it much easier to reason about if empty and missing values were always treated the same and booleans required an explicit value,
Ditto.
> but it is too late to change that now.
Just thinking aloud, a “[config] strict = true” directive could affect
parsing the rest of the directives after it (including included files)?
I’d also want a way to affect .git/config without editing them all, but…
Anyway, not something to derail this topic with, but something to muse
on for a future feature?
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-11 14:47 ` Phillip Wood
2026-11-11 15:26 ` Ben Knoble
@ 2026-11-11 17:50 ` Junio C Hamano
1 sibling, 0 replies; 69+ messages in thread
From: Junio C Hamano @ 2026-11-11 17:50 UTC (permalink / raw)
To: Phillip Wood
Cc: Thomas Bachem via GitGitGadget, git, Phillip Wood, Jeff King,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Colin Hinton,
Matt Hunter, Johannes Schindelin, Usman Akinyemi, Miklos Vajna,
Thomas Bachem
Phillip Wood <phillip.wood123@gmail.com> writes:
> To me confusing thing is that we don't treat those two the same way -
> I'd find it much easier to reason about if empty and missing values were
> always treated the same and booleans required an explicit value, but it
> is too late to change that now.
The "valueless true" was there from the beginning, as can be seen in
17712991a5 (Add ".git/config" file parser, 2005-10-10):
From: Linus Torvalds <torvalds@osdl.org>
Date: Mon, 10 Oct 2005 16:31:08 -0700
Subject: [PATCH] Add ".git/config" file parser
This is a first cut at a very simple parser for a git config file.
The format of the file is a simple ini-file like thing, ...
diff --git a/config.c b/config.c
new file mode 100644
index 0000000000..f3c4fa42ac
--- /dev/null
+++ b/config.c
@@ -0,0 +1,222 @@
...
+int git_config_bool(const char *name, const char *value)
+{
+ if (!value)
+ return 1;
+ if (!*value)
+ return 0;
+ if (!strcasecmp(value, "true"))
+ return 1;
+ if (!strcasecmp(value, "false"))
+ return 0;
+ return git_config_int(name, value) != 0;
+}
So I agree that it is way too late to change now.
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 09/15] remote: handle NULL value for remotes.<group>
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (7 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
` (8 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remotes.<group> without a value makes "git fetch <group>" and
"git push <group>" segfault: get_remote_group() dereferences the NULL
value as it splits the list of remotes.
Reject a missing value with config_error_nonbool(), and keep the
whole key for its message by skipping the "remotes." prefix into a
separate variable.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 5 ++++-
t/t5506-remote-groups.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 024cfc547d..4bd23ddac9 100644
--- a/remote.c
+++ b/remote.c
@@ -2191,8 +2191,11 @@ int get_remote_group(const char *key, const char *value,
void *priv)
{
struct remote_group_data *g = priv;
+ const char *group;
- if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
+ if (skip_prefix(key, "remotes.", &group) && !strcmp(group, g->name)) {
+ if (!value)
+ return config_error_nonbool(key);
/* split list by white space */
while (*value) {
size_t wordlen = strcspn(value, " \t\n");
diff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh
index 16e9a1bc2f..f11eb8511e 100755
--- a/t/t5506-remote-groups.sh
+++ b/t/t5506-remote-groups.sh
@@ -54,6 +54,11 @@ test_expect_success 'nonexistent group produces error' '
! repo_fetched two
'
+test_expect_success 'remotes.<group> without a value is rejected' '
+ test_must_fail git -c remotes.valueless fetch valueless 2>err &&
+ test_grep "missing value for .remotes.valueless." err
+'
+
test_expect_success 'updating group updates all members (remote update)' '
mark group-all &&
update_repos &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 10/15] setup: handle NULL value for safe.bareRepository
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (8 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
` (7 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A safe.bareRepository without a value makes git segfault in every
bare repository it discovers, as in "git -C repo.git log":
allowed_bare_repo_cb() passes the NULL value to strcmp().
Reject a missing value with config_error_nonbool(). The callback
already fails for any value other than "explicit" and "all".
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
setup.c | 2 ++
t/t0035-safe-bare-repository.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/setup.c b/setup.c
index 29474fc292..567a44bcb7 100644
--- a/setup.c
+++ b/setup.c
@@ -1472,6 +1472,8 @@ static int allowed_bare_repo_cb(const char *key, const char *value,
if (strcasecmp(key, "safe.bareRepository"))
return 0;
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "explicit")) {
*allowed_bare_repo = ALLOWED_BARE_REPO_EXPLICIT;
return 0;
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index 1d3d19f5b4..a0e0c48d7d 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -80,6 +80,12 @@ test_expect_success 'safe.bareRepository on the command line' '
-c safe.bareRepository=all
'
+test_expect_success 'safe.bareRepository without a value is rejected' '
+ test_must_fail git -C outer-repo/bare-repo -c safe.bareRepository \
+ rev-parse --git-dir 2>err &&
+ test_grep "missing value for .safe.barerepository." err
+'
+
test_expect_success 'safe.bareRepository in included file' '
cat >gitconfig-include <<-\EOF &&
[safe]
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (9 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
` (6 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bitmapPseudoMerge.<name>.pattern without a value makes
"git pack-objects" segfault while it writes a bitmap, e.g. during
"git repack -adb": pseudo_merge_config() dereferences the NULL value
to see whether the pattern starts with "^".
Reject a missing value with config_error_nonbool(), as the callback
already does through git_config_expiry_date() for the threshold keys.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
pseudo-merge.c | 4 ++++
t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 22b8600d68..19fae9233a 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -150,6 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,
if (!strcmp(key, "pattern")) {
struct strbuf re = STRBUF_INIT;
+ if (!value) {
+ ret = config_error_nonbool(var);
+ goto done;
+ }
if (group->pattern) {
regfree(group->pattern);
free(group->pattern);
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 5b2a17f90a..e7e6b2f8a8 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -233,6 +233,11 @@ test_expect_success 'out of order thresholds are rejected' '
test_cmp expect err
'
+test_expect_success 'pseudo-merge pattern without a value is rejected' '
+ test_must_fail git -c bitmapPseudoMerge.test.pattern repack -adb 2>err &&
+ test_grep "missing value for .bitmappseudomerge.test.pattern." err
+'
+
test_expect_success 'pseudo-merge pattern with capture groups' '
git init pseudo-merge-captures &&
(
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 12/15] scalar: handle NULL value for scalar.repo
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (10 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-09 13:32 ` Derrick Stolee
2026-11-07 8:42 ` [PATCH 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
` (5 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A scalar.repo without a value makes "scalar reconfigure --all"
segfault: get_scalar_repos() passes the NULL value to
string_list_append(), which copies it with xstrdup().
Reject a missing value with config_error_nonbool(). This cannot break
a working setup, as "scalar register" always writes scalar.repo with
a path.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
scalar.c | 5 ++++-
t/t9210-scalar.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/scalar.c b/scalar.c
index a80d8ee3ff..4fe4b2a647 100644
--- a/scalar.c
+++ b/scalar.c
@@ -652,8 +652,11 @@ static int get_scalar_repos(const char *key, const char *value,
{
struct string_list *list = data;
- if (!strcmp(key, "scalar.repo"))
+ if (!strcmp(key, "scalar.repo")) {
+ if (!value)
+ return config_error_nonbool(key);
string_list_append(list, value);
+ }
return 0;
}
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index c8463d0ac4..137765d3b8 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -288,6 +288,11 @@ test_expect_success '`reconfigure -a` removes stale config entries' '
test_cmp expect scalar.repos
'
+test_expect_success '`reconfigure -a` rejects scalar.repo without a value' '
+ test_must_fail scalar -c scalar.repo reconfigure -a 2>err &&
+ test_grep "missing value for .scalar.repo." err
+'
+
test_expect_success 'scalar delete without enlistment shows a usage' '
test_expect_code 129 scalar delete
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 12/15] scalar: handle NULL value for scalar.repo
2026-11-07 8:42 ` [PATCH 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
@ 2026-11-09 13:32 ` Derrick Stolee
0 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-09 13:32 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Junio C Hamano, Thomas Bachem
On 11/7/2026 3:42 AM, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> A scalar.repo without a value makes "scalar reconfigure --all"
> segfault: get_scalar_repos() passes the NULL value to
> string_list_append(), which copies it with xstrdup().
>
> Reject a missing value with config_error_nonbool(). This cannot break
> a working setup, as "scalar register" always writes scalar.repo with
> a path.
> - if (!strcmp(key, "scalar.repo"))
> + if (!strcmp(key, "scalar.repo")) {
> + if (!value)
> + return config_error_nonbool(key);
> string_list_append(list, value);
> + }
I had to look up the definition of 'config_error_nonbool' to
be sure that this was the right method to call, but it is:
/*
* Call this to report error for your variable that should not
* get a boolean value (i.e. "[my] var" means "true").
*/
#undef config_error_nonbool
int config_error_nonbool(const char *var)
{
return error(_("missing value for '%s'"), var);
}
> +test_expect_success '`reconfigure -a` rejects scalar.repo without a value' '
> + test_must_fail scalar -c scalar.repo reconfigure -a 2>err &&
> + test_grep "missing value for .scalar.repo." err
> +'
Thank you for testing.
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 13/15] config: handle NULL value for remote URLs in hasconfig includes
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (11 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-07 8:42 ` [PATCH 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
` (4 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
When the configuration has an includeIf "hasconfig:remote.*.url:..."
section, a remote.<name>.url without a value makes nearly every git
command segfault, "git log" too: add_remote_url() passes the NULL
value to string_list_append(), which copies it with xstrdup().
Leave such a URL out of the list that hasconfig:remote.*.url patterns
are matched against. It cannot match a pattern, and wherever git
reads its remotes, handle_config() in remote.c already rejects it
with the usual error. Commands that never look at remotes keep
working.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
config.c | 3 ++-
t/t1300-config.sh | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index e0bb29b53d..f6ccfd52dd 100644
--- a/config.c
+++ b/config.c
@@ -327,7 +327,8 @@ static int add_remote_url(const char *var, const char *value,
if (!parse_config_key(var, "remote", &remote_name, &remote_name_len,
&key) &&
remote_name &&
- !strcmp(key, "url"))
+ !strcmp(key, "url") &&
+ value)
string_list_append(remote_urls, value);
return 0;
}
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e3f8064889..8fb920b1ba 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2970,6 +2970,28 @@ test_expect_success 'includeIf.hasconfig:remote.*.url forbids remote url in such
test_grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasconfig:remote.*.url" err
'
+test_expect_success 'includeIf.hasconfig:remote.*.url skips a url without a value' '
+ git init hasremoteurlTest &&
+ test_when_finished "rm -rf hasremoteurlTest" &&
+
+ cat >include-this <<-\EOF &&
+ [user]
+ this = this-is-included
+ EOF
+ cat >>hasremoteurlTest/.git/config <<-EOF &&
+ [remote "valueless"]
+ url
+ [remote "foo"]
+ url = foourl
+ [includeIf "hasconfig:remote.*.url:foourl"]
+ path = "$(pwd)/include-this"
+ EOF
+
+ echo this-is-included >expect-this &&
+ git -C hasremoteurlTest config --get user.this >actual-this &&
+ test_cmp expect-this actual-this
+'
+
test_expect_success 'negated mode causes failure' '
test_must_fail git config --no-get 2>err &&
test_grep "unknown option \`no-get${SQ}" err
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (12 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-09 13:36 ` Derrick Stolee
2026-11-07 8:42 ` [PATCH 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
` (3 subsequent siblings)
17 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bundle list that gives bundle.mode, bundle.heuristic,
bundle.<id>.uri or bundle.<id>.creationToken without a value makes
"git clone --bundle-uri" and "git fetch" segfault:
bundle_list_update() passes the NULL value to strcmp(), relative_url()
or sscanf(). Such a list can also come from a server, through a
bundle URI that it advertises.
Reject a missing value with config_error_nonbool(). That makes the
list invalid, as an unknown mode or a second uri already does, and
the clone or fetch goes on without the bundles.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 9 +++++++++
t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/bundle-uri.c b/bundle-uri.c
index 7de17634cf..8c8d17bd2c 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -165,6 +165,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "mode")) {
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "all"))
list->mode = BUNDLE_MODE_ALL;
else if (!strcmp(value, "any"))
@@ -176,6 +178,9 @@ static int bundle_list_update(const char *key, const char *value,
if (!strcmp(subkey, "heuristic")) {
int i;
+
+ if (!value)
+ return config_error_nonbool(key);
for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
if (heuristics[i].heuristic &&
heuristics[i].name &&
@@ -210,6 +215,8 @@ static int bundle_list_update(const char *key, const char *value,
strbuf_release(&id);
if (!strcmp(subkey, "uri")) {
+ if (!value)
+ return config_error_nonbool(key);
if (bundle->uri)
return -1;
bundle->uri = relative_url(list->baseURI, value, NULL);
@@ -217,6 +224,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "creationtoken")) {
+ if (!value)
+ return config_error_nonbool(key);
if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
warning(_("could not parse bundle list key %s with value '%s'"),
"creationToken", value);
diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
index 70ae94c4d0..a91c1b428d 100755
--- a/t/t5750-bundle-uri-parse.sh
+++ b/t/t5750-bundle-uri-parse.sh
@@ -312,4 +312,20 @@ test_expect_success 'parse config format: bundle with url instead of uri' '
test_grep "bundle '\''typo'\'' has no uri" err
'
+test_expect_success 'parse config format edge cases: keys without a value' '
+ for key in mode heuristic
+ do
+ printf "[bundle]\n\tversion = 1\n\t%s\n" "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.$key." err || return 1
+ done &&
+ for key in uri creationtoken
+ do
+ printf "[bundle]\n\tversion = 1\n\tmode = all\n[bundle \"one\"]\n\t%s\n" \
+ "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.one.$key." err || return 1
+ done
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-07 8:42 ` [PATCH 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
@ 2026-11-09 13:36 ` Derrick Stolee
0 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-09 13:36 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Junio C Hamano, Thomas Bachem
On 11/7/2026 3:42 AM, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> A bundle list that gives bundle.mode, bundle.heuristic,
> bundle.<id>.uri or bundle.<id>.creationToken without a value makes
> "git clone --bundle-uri" and "git fetch" segfault:
> bundle_list_update() passes the NULL value to strcmp(), relative_url()
> or sscanf(). Such a list can also come from a server, through a
> bundle URI that it advertises.
>
> Reject a missing value with config_error_nonbool(). That makes the
> list invalid, as an unknown mode or a second uri already does, and
> the clone or fetch goes on without the bundles.
I'm not sure that this is the most-helpful error message for the
bundle URI feature, as this isn't an error in a local config file
but in a server-provided file. In that sense, it would be _better_
to provide a hint that the server gave a malformed bundle list.
However, this patch is a clear improvement over the current state,
so this isn't necessary right now. Doing the minimum "fix the
segfaults" fits with the rest of your patches. We can do another
pass later to make this error message more helpful, if necessary.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 15/15] config: handle NULL values when listing colors and expiry dates
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (13 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
@ 2026-11-07 8:42 ` Thomas Bachem via GitGitGadget
2026-11-09 13:42 ` [PATCH 00/15] Fix segfaults on settings given without a value Derrick Stolee
` (2 subsequent siblings)
17 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-07 8:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
Since 2d4ab5a885 (config: format colors quietly, 2026-02-23) and
9cb4a5e1ba (config: format expiry dates quietly, 2026-02-23),
"git config list --type=color" and "--type=expiry-date" pass every
value to color_parse_quietly() or parse_expiry_date(), which do not
handle NULL. So both segfault when any key in the configuration has
no value.
Skip such a key quietly, as both already do with a value that does
not parse, and as "--type=int" does with a key without a value.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/config.c | 4 ++--
t/t1300-config.sh | 10 ++++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 2554322317..f786c0b123 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -366,7 +366,7 @@ static int format_config_expiry_date(struct strbuf *buf,
{
timestamp_t t;
if (quietly) {
- if (parse_expiry_date(value_, &t))
+ if (!value_ || parse_expiry_date(value_, &t))
return -1;
} else if (git_config_expiry_date(&t, key_, value_) < 0) {
return -1;
@@ -384,7 +384,7 @@ static int format_config_color(struct strbuf *buf,
char v[COLOR_MAXLEN];
if (gently) {
- if (color_parse_quietly(value_, v) < 0)
+ if (!value_ || color_parse_quietly(value_, v) < 0)
return -1;
} else if (git_config_color(v, key_, value_) < 0) {
return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 8fb920b1ba..3322b0bf4f 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2648,6 +2648,16 @@ test_expect_success 'list --type=color shows only canonicalizable color values'
test_must_be_empty err
'
+test_expect_success 'list --type=color and --type=expiry-date skip keys without a value' '
+ git -c section.valueless config ${mode_prefix}list --type=color >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err &&
+
+ git -c section.valueless config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err
+'
+
test_expect_success '--type rejects unknown specifiers' '
test_must_fail git config --type=nonsense section.foo 2>error &&
test_grep "unrecognized --type argument" error
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 00/15] Fix segfaults on settings given without a value
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (14 preceding siblings ...)
2026-11-07 8:42 ` [PATCH 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
@ 2026-11-09 13:42 ` Derrick Stolee
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
17 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-09 13:42 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Junio C Hamano, Thomas Bachem
On 11/7/2026 3:42 AM, Thomas Bachem via GitGitGadget wrote:
> A setting given without a value, as in "git -c foo.bar" or a bare "bar"
> under "[foo]", is passed as NULL to the code that reads it. In these places
> git does not check for that and segfaults:
I read each patch, but commented (positively) on the ones where I was
involved in creating the bug in the first place. Thank you for including
a test in each case to prevent this from happening again.
The only suggestion I had for improving your patches is to include a
more helpful error message when the bad data comes from a server-
provided bundle list, but that isn't necessary for the bugfix. Also,
such a custom response would look out of place with the rest of your
patches. We can make that improvement later, if so desired.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread* [PATCH v2 00/15] Fix segfaults on settings given without a value
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (15 preceding siblings ...)
2026-11-09 13:42 ` [PATCH 00/15] Fix segfaults on settings given without a value Derrick Stolee
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
` (15 more replies)
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
17 siblings, 16 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
A setting given without a value, as in "git -c foo.bar" or a bare "bar"
under "[foo]", is passed as NULL to the code that reads it. In these places
git does not check for that and segfaults:
* rebase.missingCommitsCheck in "git rebase -i"
* core.alternateRefsCommand and core.alternateRefsPrefixes in "git fetch",
"git receive-pack" and "git rev-list --alternate-refs", in a repository
with an alternate
* the color slots of "git add -p", "git add -i" and the other interactive
patch modes
* fetch.bundleCreationToken in "git fetch" and "git clone --bundle-uri",
with a creationToken bundle list
* grep.patternType in "git grep", "git log" and "git show"
* remote.*.tagOpt in "git fetch", "git remote" and "git status"
* color.blame.repeatedLines and color.blame.highlightRecent in "git blame"
* branch.*.mergeOptions in "git merge" on that branch
* remotes.* in "git fetch" and "git push" of that remote group
* safe.bareRepository in any bare repository that git discovers
* bitmapPseudoMerge.*.pattern when writing a bitmap, as in "git repack
-adb"
* scalar.repo in "scalar reconfigure --all"
* remote.*.url in nearly every command, when an includeIf
"hasconfig:remote.*.url:..." section exists
* mode, heuristic, uri and creationToken in a bundle list, in "git clone
--bundle-uri" and "git fetch"
* any key at all in "git config list --type=color" and "--type=expiry-date"
Changes since v1:
* Patch 1 no longer rejects a rebase.missingCommitsCheck without a value.
It warns and falls back to "ignore", as for a value that git does not
recognize (Phillip, Junio).
* Rebased onto master, where iw/connected-deepen-fetch-no-alternate-refs
now adds tests at the end of t5618, as patch 2 does.
These turned up in a check of every config callback and every caller of
repo_config_get_value(), and in runs of two dozen common commands with each
key that "git help --config" lists set without a value. There may be more in
code that neither method reached.
Most patches reject the missing value with config_error_nonbool(), as
ba176db511 (config: handle NULL value when parsing non-bools, 2023-12-07)
did for other settings. Four accept it without an error: a missing
rebase.missingCommitsCheck warns and falls back to "ignore" like a value
that git does not recognize (patch 1), a missing fetch.bundleCreationToken
counts as 0 (patch 4), a remote URL without a value matches no includeIf
condition (patch 13), and "git config list" leaves the key out (patch 15).
The crash fixed in patch 14 can also come from a server, through a bundle
list it advertises to clients that use bundle URIs. It is still a plain NULL
dereference. Only patch 15 fixes a regression, which came in with 2.54.0.
remote.*.followRemoteHEAD without a value crashes the same commands. This
series leaves it alone, as ch/remote-defer-follow-remote-head-validation in
'seen' already makes it safe, checking the value only where fetch uses it.
The series is now based on 3c60ef1e38 (The 17th batch, 2026-11-08) and
merges cleanly into 'next' and 'seen' too.
Thomas Bachem (15):
rebase -i: handle NULL value for rebase.missingCommitsCheck
odb: handle NULL values for core.alternateRefs* config
add-patch: handle NULL value for color slots
bundle-uri: handle NULL value for fetch.bundleCreationToken
grep: handle NULL value for grep.patternType
remote: handle NULL value for remote.<name>.tagOpt
blame: handle NULL values for color.blame.* config
merge: handle NULL value for branch.<name>.mergeOptions
remote: handle NULL value for remotes.<group>
setup: handle NULL value for safe.bareRepository
pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
scalar: handle NULL value for scalar.repo
config: handle NULL value for remote URLs in hasconfig includes
bundle-uri: handle NULL values in bundle lists
config: handle NULL values when listing colors and expiry dates
add-patch.c | 2 +-
builtin/blame.c | 4 ++++
builtin/config.c | 4 ++--
builtin/merge.c | 2 ++
bundle-uri.c | 12 +++++++++++-
config.c | 3 ++-
grep.c | 2 ++
odb.c | 4 ++--
pseudo-merge.c | 4 ++++
rebase-interactive.c | 10 ++++++++--
remote.c | 7 ++++++-
scalar.c | 5 ++++-
setup.c | 2 ++
t/t0035-safe-bare-repository.sh | 6 ++++++
t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
t/t3404-rebase-interactive.sh | 12 ++++++++++++
t/t3701-add-interactive.sh | 12 ++++++++++++
t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
t/t5505-remote.sh | 5 +++++
t/t5506-remote-groups.sh | 5 +++++
t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
t/t5618-alternate-refs.sh | 9 +++++++++
t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
t/t7600-merge.sh | 6 ++++++
t/t7810-grep.sh | 5 +++++
t/t8012-blame-colors.sh | 7 +++++++
t/t9210-scalar.sh | 5 +++++
27 files changed, 195 insertions(+), 11 deletions(-)
base-commit: 3c60ef1e38ee80337025a1b908327f7879386329
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2297%2Fthomasbachem%2Fconfig-null-value-segfaults-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2297/thomasbachem/config-null-value-segfaults-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2297
Range-diff vs v1:
1: a9a3f4b705 ! 1: 47224a9893 rebase -i: handle NULL value for rebase.missingCommitsCheck
@@ Commit message
segfault as it writes the todo list: get_missing_commit_check_level()
passes the NULL value to strcasecmp().
- The setting takes "ignore", "warn" or "error", not a bool. Read it
- with repo_config_get_string_tmp(), which rejects a missing value with
- config_error_nonbool(), as ba176db511 (config: handle NULL value when
- parsing non-bools, 2023-12-07) did for other settings.
+ The setting takes "ignore", "warn" or "error". An unrecognized value,
+ perhaps one meant for a newer version of Git, gets a warning and is
+ treated as "ignore". Treat a missing value the same way.
+ Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
+ Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
## rebase-interactive.c ##
@@ rebase-interactive.c: static enum missing_commit_check_level get_missing_commit_
const char *value;
- if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
-+ if (repo_config_get_string_tmp(the_repository, "rebase.missingcommitscheck", &value) ||
- !strcasecmp("ignore", value))
+- !strcasecmp("ignore", value))
++ if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value))
++ return MISSING_COMMIT_CHECK_IGNORE;
++ if (!value) {
++ warning(_("missing value for option "
++ "rebase.missingCommitsCheck. Ignoring."));
++ return MISSING_COMMIT_CHECK_IGNORE;
++ }
++ if (!strcasecmp("ignore", value))
return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
+ return MISSING_COMMIT_CHECK_WARN;
## t/t3404-rebase-interactive.sh ##
@@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
-+test_expect_success 'rebase -i rejects rebase.missingCommitsCheck without a value' '
++test_expect_success 'rebase -i warns about and ignores rebase.missingCommitsCheck without a value' '
+ rebase_setup_and_clean missing-commit &&
-+ test_when_finished "test_might_fail git rebase --abort" &&
-+ test_must_fail git -c rebase.missingCommitsCheck rebase -i --root 2>err &&
-+ test_grep "missing value for .rebase.missingcommitscheck." err
++ (
++ set_fake_editor &&
++ FAKE_LINES="1 2 3 4" \
++ git -c rebase.missingCommitsCheck rebase -i --root 2>actual
++ ) &&
++ test D = $(git cat-file commit HEAD | sed -ne \$p) &&
++ test_grep "missing value for option rebase.missingCommitsCheck" actual &&
++ test_grep ! "commits may have been dropped" actual
+'
+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
2: 74ab5bfc62 ! 2: 9170c4162f odb: handle NULL values for core.alternateRefs* config
@@ odb.c: static void fill_alternate_refs_command(struct repository *repo,
}
## t/t5618-alternate-refs.sh ##
-@@ t/t5618-alternate-refs.sh: test_expect_success 'log --source shows .alternate marker' '
+@@ t/t5618-alternate-refs.sh: test_expect_success 'refs of a shallow alternate are ignored' '
test_cmp expect actual
'
3: 1b5f243294 = 3: d3fd6290e0 add-patch: handle NULL value for color slots
4: 4eb7085084 = 4: a822cbb844 bundle-uri: handle NULL value for fetch.bundleCreationToken
5: 88df66ab7e = 5: 6c2a671a92 grep: handle NULL value for grep.patternType
6: e95249fb51 = 6: d4b49d1363 remote: handle NULL value for remote.<name>.tagOpt
7: 260b90a4a2 = 7: 25843b43bf blame: handle NULL values for color.blame.* config
8: e10f1fc0ba = 8: d8681a8560 merge: handle NULL value for branch.<name>.mergeOptions
9: 0faf4e884f = 9: 51f8eba073 remote: handle NULL value for remotes.<group>
10: 0991a1b80c = 10: a5b6436943 setup: handle NULL value for safe.bareRepository
11: 3f4cbf4494 = 11: 1533ef9cc6 pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
12: b4bcaf19ec = 12: 6219e9fd7f scalar: handle NULL value for scalar.repo
13: 5c12b13d90 = 13: 080106d5ea config: handle NULL value for remote URLs in hasconfig includes
14: ce4c190ea2 = 14: fb9e63541b bundle-uri: handle NULL values in bundle lists
15: 7ac4e3a3d9 = 15: 12f841e5ff config: handle NULL values when listing colors and expiry dates
--
gitgitgadget
^ permalink raw reply [flat|nested] 69+ messages in thread* [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 11:15 ` Phillip Wood
2026-11-10 7:36 ` [PATCH v2 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
` (14 subsequent siblings)
15 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A rebase.missingCommitsCheck without a value, as in
[rebase]
missingCommitsCheck
or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
segfault as it writes the todo list: get_missing_commit_check_level()
passes the NULL value to strcasecmp().
The setting takes "ignore", "warn" or "error". An unrecognized value,
perhaps one meant for a newer version of Git, gets a warning and is
treated as "ignore". Treat a missing value the same way.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
rebase-interactive.c | 10 ++++++++--
t/t3404-rebase-interactive.sh | 12 ++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 809f76a87b..9eb70b09b3 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -30,8 +30,14 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
{
const char *value;
- if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
- !strcasecmp("ignore", value))
+ if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value))
+ return MISSING_COMMIT_CHECK_IGNORE;
+ if (!value) {
+ warning(_("missing value for option "
+ "rebase.missingCommitsCheck. Ignoring."));
+ return MISSING_COMMIT_CHECK_IGNORE;
+ }
+ if (!strcasecmp("ignore", value))
return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
return MISSING_COMMIT_CHECK_WARN;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index b3ee96f560..5b9fc67c19 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1562,6 +1562,18 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'rebase -i warns about and ignores rebase.missingCommitsCheck without a value' '
+ rebase_setup_and_clean missing-commit &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="1 2 3 4" \
+ git -c rebase.missingCommitsCheck rebase -i --root 2>actual
+ ) &&
+ test D = $(git cat-file commit HEAD | sed -ne \$p) &&
+ test_grep "missing value for option rebase.missingCommitsCheck" actual &&
+ test_grep ! "commits may have been dropped" actual
+'
+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
test_config rebase.missingCommitsCheck ignore &&
rebase_setup_and_clean missing-commit &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-10 7:36 ` [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
@ 2026-11-10 11:15 ` Phillip Wood
0 siblings, 0 replies; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:15 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> A rebase.missingCommitsCheck without a value, as in
>
> [rebase]
> missingCommitsCheck
>
> or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
> segfault as it writes the todo list: get_missing_commit_check_level()
> passes the NULL value to strcasecmp().
>
> The setting takes "ignore", "warn" or "error". An unrecognized value,
> perhaps one meant for a newer version of Git, gets a warning and is
> treated as "ignore". Treat a missing value the same way.
Sounds good
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index 809f76a87b..9eb70b09b3 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -30,8 +30,14 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
> {
> const char *value;
>
> - if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
> - !strcasecmp("ignore", value))
> + if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value))
> + return MISSING_COMMIT_CHECK_IGNORE;
> + if (!value) {
> + warning(_("missing value for option "
> + "rebase.missingCommitsCheck. Ignoring."));
> + return MISSING_COMMIT_CHECK_IGNORE;
> + }
If "value" is NULL we print a warning and continue as if the user had
said "ignore" - good
> + if (!strcasecmp("ignore", value))
> return MISSING_COMMIT_CHECK_IGNORE;
> if (!strcasecmp("warn", value))
> return MISSING_COMMIT_CHECK_WARN;
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index b3ee96f560..5b9fc67c19 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1562,6 +1562,18 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> +test_expect_success 'rebase -i warns about and ignores rebase.missingCommitsCheck without a value' '
> + rebase_setup_and_clean missing-commit &&
> + (
> + set_fake_editor &&
> + FAKE_LINES="1 2 3 4" \
> + git -c rebase.missingCommitsCheck rebase -i --root 2>actual
Because this uses "--root", it is hard to tell if it is dropping any
commits or not. Using something like
FAKE_LINES=1 \
git -c rebase.missingCommitCheck rebase -i HEAD~3
makes it easier to see we're dropping a commit by looking at FAKE_LINES
and the upstream commit passed to rebase.
> + ) &&
> + test D = $(git cat-file commit HEAD | sed -ne \$p) &&
This is quite common in our older tests, but makes it very hard to
diagnose test failures because it does not print anything if there is an
error. These days we can use
test_commit_message HEAD -m D
> + test_grep "missing value for option rebase.missingCommitsCheck" actual &&
> + test_grep ! "commits may have been dropped" actual
We check for the error message caused by a missing config value and also
check we don't complain about the missing commits - good.
Thanks
Phillip
> +'
> +
> test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
> test_config rebase.missingCommitsCheck ignore &&
> rebase_setup_and_clean missing-commit &&
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH v2 02/15] odb: handle NULL values for core.alternateRefs* config
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
` (13 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A core.alternateRefsCommand or core.alternateRefsPrefixes without a
value makes "git fetch", "git receive-pack" and
"git rev-list --alternate-refs" segfault in a repository that borrows
objects from an alternate: fill_alternate_refs_command() passes the
NULL value to strvec_push() or strvec_split().
Both settings take a string. Read them with
repo_config_get_string_tmp(), which rejects a missing value with
config_error_nonbool(), as the previous commit does for
rebase.missingCommitsCheck.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
odb.c | 4 ++--
t/t5618-alternate-refs.sh | 9 +++++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/odb.c b/odb.c
index 865c404910..ea086be8b3 100644
--- a/odb.c
+++ b/odb.c
@@ -161,7 +161,7 @@ static void fill_alternate_refs_command(struct repository *repo,
{
const char *value;
- if (!repo_config_get_value(repo, "core.alternateRefsCommand", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsCommand", &value)) {
cmd->use_shell = 1;
strvec_push(&cmd->args, value);
@@ -173,7 +173,7 @@ static void fill_alternate_refs_command(struct repository *repo,
strvec_push(&cmd->args, "for-each-ref");
strvec_push(&cmd->args, "--format=%(objectname)");
- if (!repo_config_get_value(repo, "core.alternateRefsPrefixes", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsPrefixes", &value)) {
strvec_push(&cmd->args, "--");
strvec_split(&cmd->args, value);
}
diff --git a/t/t5618-alternate-refs.sh b/t/t5618-alternate-refs.sh
index 68139a1b3e..422da6dde7 100755
--- a/t/t5618-alternate-refs.sh
+++ b/t/t5618-alternate-refs.sh
@@ -83,4 +83,13 @@ test_expect_success 'refs of a shallow alternate are ignored' '
test_cmp expect actual
'
+test_expect_success 'alternateRefsCommand and alternateRefsPrefixes need a value' '
+ test_must_fail git -c core.alternateRefsCommand \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsCommand." err &&
+ test_must_fail git -c core.alternateRefsPrefixes \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsPrefixes." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 03/15] add-patch: handle NULL value for color slots
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
` (12 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
With color on, a color slot without a value, such as
color.interactive.prompt or color.diff.frag, makes "git add -p",
"git add -i" and the other interactive patch modes segfault:
init_color() passes the NULL value to color_parse().
Read the slot with repo_config_get_string_tmp(), which rejects a
missing value with config_error_nonbool(), as "git diff" already does
for color.diff.frag and the other diff colors.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
add-patch.c | 2 +-
t/t3701-add-interactive.sh | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/add-patch.c b/add-patch.c
index f27edcbe8d..ad581eb4ae 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -295,7 +295,7 @@ static void init_color(struct repository *r,
if (!want_color(use_color))
dst[0] = '\0';
- else if (repo_config_get_value(r, key, &value) ||
+ else if (repo_config_get_string_tmp(r, key, &value) ||
color_parse(value, dst))
strlcpy(dst, default_color, COLOR_MAXLEN);
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6e120a4001..33050ccc53 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -815,6 +815,18 @@ test_expect_success 'colors can be overridden' '
test_cmp expect actual
'
+test_expect_success 'a color without a value is rejected' '
+ git reset --hard &&
+ test_when_finished "git rm -f color-test" &&
+ test_write_lines context old >color-test &&
+ git add color-test &&
+ test_write_lines context new >color-test &&
+
+ test_must_fail git -c color.interactive=always \
+ -c color.interactive.prompt add -p </dev/null 2>err &&
+ test_grep "missing value for .color.interactive.prompt." err
+'
+
test_expect_success 'brackets appear without color' '
git reset --hard &&
test_when_finished "git rm -f bracket-test" &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (2 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
` (11 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A fetch.bundleCreationToken without a value makes "git fetch" and
"git clone --bundle-uri" segfault when the bundle list uses the
creationToken heuristic: fetch_bundles_by_token() passes the NULL
value to sscanf().
The function already treats a token that does not parse as 0, as if
no bundles had been fetched before, so it downloads them and then
stores the new token. Treat a missing value the same way.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 3 ++-
t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/bundle-uri.c b/bundle-uri.c
index 9797eaed9a..67a717e857 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -572,7 +572,8 @@ static int fetch_bundles_by_token(struct repository *r,
if (!repo_config_get_value(r,
"fetch.bundlecreationtoken",
&creationTokenStr)) {
- if (sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
+ if (!creationTokenStr ||
+ sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
maxCreationToken = 0;
if (bundles.items[0]->creationToken <= maxCreationToken) {
free(bundles.items);
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index f81f1f8db7..e86ed7d30c 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -346,6 +346,26 @@ test_expect_success 'clone bundle list (file, any mode, all failures)' '
test_grep ! "refs/bundles/heads/" refs
'
+test_expect_success 'fetch treats fetch.bundleCreationToken without a value as 0' '
+ test_when_finished "rm -rf token-without-value" &&
+ cat >bundle-list <<-EOF &&
+ [bundle]
+ version = 1
+ mode = all
+ heuristic = creationToken
+
+ [bundle "bundle-1"]
+ uri = file://$(pwd)/clone-from/bundle-1.bundle
+ creationToken = 1
+ EOF
+
+ git clone --no-local clone-from token-without-value &&
+ git -C token-without-value \
+ -c fetch.bundleURI="file://$(pwd)/bundle-list" \
+ -c fetch.bundleCreationToken fetch origin &&
+ test_cmp_config -C token-without-value 1 fetch.bundlecreationtoken
+'
+
test_expect_success 'negotiation: bundle with part of wanted commits' '
test_when_finished "rm -f trace*.txt" &&
GIT_TRACE_PACKET="$(pwd)/trace-packet.txt" \
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 05/15] grep: handle NULL value for grep.patternType
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (3 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
` (10 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A grep.patternType without a value makes "git grep", "git log" and
"git show" segfault: grep_config(), which all three use, passes the
NULL value to parse_pattern_type_arg(), which hands it to strcmp().
Reject a missing value with config_error_nonbool(), as grep_config()
already does for the color.grep.* slots.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
grep.c | 2 ++
t/t7810-grep.sh | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/grep.c b/grep.c
index 1d75d31421..ab082f963e 100644
--- a/grep.c
+++ b/grep.c
@@ -71,6 +71,8 @@ int grep_config(const char *var, const char *value,
}
if (!strcmp(var, "grep.patterntype")) {
+ if (!value)
+ return config_error_nonbool(var);
opt->pattern_type_option = parse_pattern_type_arg(var, value);
return 0;
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index ac7ea9cf8d..d323927e54 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1551,6 +1551,11 @@ test_expect_success PCRE 'grep invalidpattern properly dies with grep.patternTyp
test_must_fail git -c grep.patterntype=perl grep "a["
'
+test_expect_success 'grep.patternType without a value is rejected' '
+ test_must_fail git -c grep.patternType grep "a" 2>err &&
+ test_grep "missing value for .grep.patterntype." err
+'
+
test_expect_success 'grep -G -E -F pattern' '
echo "ab:a+b*c" >expected &&
git grep -G -E -F "a+b*c" ab >actual &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 06/15] remote: handle NULL value for remote.<name>.tagOpt
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (4 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
` (9 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remote.<name>.tagOpt without a value makes "git fetch",
"git remote" and "git status" segfault: handle_config() passes the
NULL value to strcmp().
Reject a missing value with config_error_nonbool(), as handle_config()
already does for remote.<name>.url and remote.<name>.pushurl.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 2 ++
t/t5505-remote.sh | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/remote.c b/remote.c
index 49ee2ea318..0db29d6dda 100644
--- a/remote.c
+++ b/remote.c
@@ -556,6 +556,8 @@ static int handle_config(const char *key, const char *value,
else
error(_("more than one uploadpack given, using the first"));
} else if (!strcmp(subkey, "tagopt")) {
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "--no-tags"))
remote->fetch_tags = -1;
else if (!strcmp(value, "--tags"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 5fbfcb0848..bec2e51afa 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -90,6 +90,11 @@ test_expect_success 'remote information for the origin' '
)
'
+test_expect_success 'tagOpt needs a value' '
+ test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
+ test_grep "missing value for .remote.origin.tagopt." err
+'
+
test_expect_success 'add another remote' '
(
cd test &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 07/15] blame: handle NULL values for color.blame.* config
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (5 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
` (8 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A color.blame.repeatedLines or color.blame.highlightRecent without a
value makes "git blame" segfault: git_blame_config() passes the NULL
value to strlen() or to parse_color_fields().
Reject a missing value with config_error_nonbool(), as
git_blame_config() already does for blame.date and blame.coloring.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/blame.c | 4 ++++
t/t8012-blame-colors.sh | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/builtin/blame.c b/builtin/blame.c
index 1e753a0b60..15cacd1002 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -783,12 +783,16 @@ static int git_blame_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "color.blame.repeatedlines")) {
+ if (!value)
+ return config_error_nonbool(var);
if (color_parse_mem(value, strlen(value), repeated_meta_color))
warning(_("invalid value for '%s': '%s'"),
"color.blame.repeatedLines", value);
return 0;
}
if (!strcmp(var, "color.blame.highlightrecent")) {
+ if (!value)
+ return config_error_nonbool(var);
parse_color_fields(value);
return 0;
}
diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
index d2653b3c31..5eeb7338a3 100755
--- a/t/t8012-blame-colors.sh
+++ b/t/t8012-blame-colors.sh
@@ -69,4 +69,11 @@ test_expect_success 'blame color by age: new code is different' '
test_grep qfunc colored
'
+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
+ test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.repeatedlines." err &&
+ test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.highlightrecent." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (6 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
` (7 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A branch.<name>.mergeOptions without a value makes "git merge"
segfault on that branch: git_merge_config() passes the NULL value to
xstrdup().
Reject a missing value with config_error_nonbool(), as
git_config_string() already does for the other strings that
git_merge_config() reads.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/merge.c | 2 ++
t/t7600-merge.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index d7bf209114..c6f9356ccb 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
skip_prefix(k, "branch.", &str) &&
skip_prefix(str, branch, &str) &&
!strcmp(str, ".mergeoptions")) {
+ if (!v)
+ return config_error_nonbool(k);
free(branch_mergeoptions);
branch_mergeoptions = xstrdup(v);
return 0;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 8f6109fb91..2da12d6662 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -552,6 +552,12 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
test_cmp squash.1-5 .git/SQUASH_MSG
'
+test_expect_success 'merge rejects branch.main.mergeOptions without a value' '
+ git reset --hard c1 &&
+ test_must_fail git -c branch.main.mergeOptions merge c2 2>err &&
+ test_grep "missing value for .branch.main.mergeoptions." err
+'
+
test_debug 'git log --graph --decorate --oneline --all'
test_expect_success 'override config option -n with --summary' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 09/15] remote: handle NULL value for remotes.<group>
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (7 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
` (6 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remotes.<group> without a value makes "git fetch <group>" and
"git push <group>" segfault: get_remote_group() dereferences the NULL
value as it splits the list of remotes.
Reject a missing value with config_error_nonbool(), and keep the
whole key for its message by skipping the "remotes." prefix into a
separate variable.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 5 ++++-
t/t5506-remote-groups.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 0db29d6dda..fd7a31cdcf 100644
--- a/remote.c
+++ b/remote.c
@@ -2198,8 +2198,11 @@ int get_remote_group(const char *key, const char *value,
void *priv)
{
struct remote_group_data *g = priv;
+ const char *group;
- if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
+ if (skip_prefix(key, "remotes.", &group) && !strcmp(group, g->name)) {
+ if (!value)
+ return config_error_nonbool(key);
/* split list by white space */
while (*value) {
size_t wordlen = strcspn(value, " \t\n");
diff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh
index 16e9a1bc2f..f11eb8511e 100755
--- a/t/t5506-remote-groups.sh
+++ b/t/t5506-remote-groups.sh
@@ -54,6 +54,11 @@ test_expect_success 'nonexistent group produces error' '
! repo_fetched two
'
+test_expect_success 'remotes.<group> without a value is rejected' '
+ test_must_fail git -c remotes.valueless fetch valueless 2>err &&
+ test_grep "missing value for .remotes.valueless." err
+'
+
test_expect_success 'updating group updates all members (remote update)' '
mark group-all &&
update_repos &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 10/15] setup: handle NULL value for safe.bareRepository
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (8 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
` (5 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A safe.bareRepository without a value makes git segfault in every
bare repository it discovers, as in "git -C repo.git log":
allowed_bare_repo_cb() passes the NULL value to strcmp().
Reject a missing value with config_error_nonbool(). The callback
already fails for any value other than "explicit" and "all".
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
setup.c | 2 ++
t/t0035-safe-bare-repository.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/setup.c b/setup.c
index 186815c780..e9a1bdbb50 100644
--- a/setup.c
+++ b/setup.c
@@ -1531,6 +1531,8 @@ static int allowed_bare_repo_cb(const char *key, const char *value,
if (strcasecmp(key, "safe.bareRepository"))
return 0;
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "explicit")) {
*allowed_bare_repo = ALLOWED_BARE_REPO_EXPLICIT;
return 0;
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index 1d3d19f5b4..a0e0c48d7d 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -80,6 +80,12 @@ test_expect_success 'safe.bareRepository on the command line' '
-c safe.bareRepository=all
'
+test_expect_success 'safe.bareRepository without a value is rejected' '
+ test_must_fail git -C outer-repo/bare-repo -c safe.bareRepository \
+ rev-parse --git-dir 2>err &&
+ test_grep "missing value for .safe.barerepository." err
+'
+
test_expect_success 'safe.bareRepository in included file' '
cat >gitconfig-include <<-\EOF &&
[safe]
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (9 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
` (4 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bitmapPseudoMerge.<name>.pattern without a value makes
"git pack-objects" segfault while it writes a bitmap, e.g. during
"git repack -adb": pseudo_merge_config() dereferences the NULL value
to see whether the pattern starts with "^".
Reject a missing value with config_error_nonbool(), as the callback
already does through git_config_expiry_date() for the threshold keys.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
pseudo-merge.c | 4 ++++
t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 22b8600d68..19fae9233a 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -150,6 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,
if (!strcmp(key, "pattern")) {
struct strbuf re = STRBUF_INIT;
+ if (!value) {
+ ret = config_error_nonbool(var);
+ goto done;
+ }
if (group->pattern) {
regfree(group->pattern);
free(group->pattern);
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 5b2a17f90a..e7e6b2f8a8 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -233,6 +233,11 @@ test_expect_success 'out of order thresholds are rejected' '
test_cmp expect err
'
+test_expect_success 'pseudo-merge pattern without a value is rejected' '
+ test_must_fail git -c bitmapPseudoMerge.test.pattern repack -adb 2>err &&
+ test_grep "missing value for .bitmappseudomerge.test.pattern." err
+'
+
test_expect_success 'pseudo-merge pattern with capture groups' '
git init pseudo-merge-captures &&
(
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 12/15] scalar: handle NULL value for scalar.repo
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (10 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
` (3 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A scalar.repo without a value makes "scalar reconfigure --all"
segfault: get_scalar_repos() passes the NULL value to
string_list_append(), which copies it with xstrdup().
Reject a missing value with config_error_nonbool(). This cannot break
a working setup, as "scalar register" always writes scalar.repo with
a path.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
scalar.c | 5 ++++-
t/t9210-scalar.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/scalar.c b/scalar.c
index a80d8ee3ff..4fe4b2a647 100644
--- a/scalar.c
+++ b/scalar.c
@@ -652,8 +652,11 @@ static int get_scalar_repos(const char *key, const char *value,
{
struct string_list *list = data;
- if (!strcmp(key, "scalar.repo"))
+ if (!strcmp(key, "scalar.repo")) {
+ if (!value)
+ return config_error_nonbool(key);
string_list_append(list, value);
+ }
return 0;
}
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index c8463d0ac4..137765d3b8 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -288,6 +288,11 @@ test_expect_success '`reconfigure -a` removes stale config entries' '
test_cmp expect scalar.repos
'
+test_expect_success '`reconfigure -a` rejects scalar.repo without a value' '
+ test_must_fail scalar -c scalar.repo reconfigure -a 2>err &&
+ test_grep "missing value for .scalar.repo." err
+'
+
test_expect_success 'scalar delete without enlistment shows a usage' '
test_expect_code 129 scalar delete
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 13/15] config: handle NULL value for remote URLs in hasconfig includes
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (11 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 7:36 ` [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
` (2 subsequent siblings)
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
When the configuration has an includeIf "hasconfig:remote.*.url:..."
section, a remote.<name>.url without a value makes nearly every git
command segfault, "git log" too: add_remote_url() passes the NULL
value to string_list_append(), which copies it with xstrdup().
Leave such a URL out of the list that hasconfig:remote.*.url patterns
are matched against. It cannot match a pattern, and wherever git
reads its remotes, handle_config() in remote.c already rejects it
with the usual error. Commands that never look at remotes keep
working.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
config.c | 3 ++-
t/t1300-config.sh | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 5976a418d5..5b823b3dcd 100644
--- a/config.c
+++ b/config.c
@@ -342,7 +342,8 @@ static int add_remote_url(const char *var, const char *value,
if (!parse_config_key(var, "remote", &remote_name, &remote_name_len,
&key) &&
remote_name &&
- !strcmp(key, "url"))
+ !strcmp(key, "url") &&
+ value)
string_list_append(remote_urls, value);
return 0;
}
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e3f8064889..8fb920b1ba 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2970,6 +2970,28 @@ test_expect_success 'includeIf.hasconfig:remote.*.url forbids remote url in such
test_grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasconfig:remote.*.url" err
'
+test_expect_success 'includeIf.hasconfig:remote.*.url skips a url without a value' '
+ git init hasremoteurlTest &&
+ test_when_finished "rm -rf hasremoteurlTest" &&
+
+ cat >include-this <<-\EOF &&
+ [user]
+ this = this-is-included
+ EOF
+ cat >>hasremoteurlTest/.git/config <<-EOF &&
+ [remote "valueless"]
+ url
+ [remote "foo"]
+ url = foourl
+ [includeIf "hasconfig:remote.*.url:foourl"]
+ path = "$(pwd)/include-this"
+ EOF
+
+ echo this-is-included >expect-this &&
+ git -C hasremoteurlTest config --get user.this >actual-this &&
+ test_cmp expect-this actual-this
+'
+
test_expect_success 'negated mode causes failure' '
test_must_fail git config --no-get 2>err &&
test_grep "unknown option \`no-get${SQ}" err
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (12 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 11:11 ` Phillip Wood
2026-11-10 7:36 ` [PATCH v2 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
2026-11-10 11:17 ` [PATCH v2 00/15] Fix segfaults on settings given without a value Phillip Wood
15 siblings, 1 reply; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bundle list that gives bundle.mode, bundle.heuristic,
bundle.<id>.uri or bundle.<id>.creationToken without a value makes
"git clone --bundle-uri" and "git fetch" segfault:
bundle_list_update() passes the NULL value to strcmp(), relative_url()
or sscanf(). Such a list can also come from a server, through a
bundle URI that it advertises.
Reject a missing value with config_error_nonbool(). That makes the
list invalid, as an unknown mode or a second uri already does, and
the clone or fetch goes on without the bundles.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 9 +++++++++
t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/bundle-uri.c b/bundle-uri.c
index 67a717e857..41c817eefb 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -164,6 +164,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "mode")) {
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "all"))
list->mode = BUNDLE_MODE_ALL;
else if (!strcmp(value, "any"))
@@ -175,6 +177,9 @@ static int bundle_list_update(const char *key, const char *value,
if (!strcmp(subkey, "heuristic")) {
int i;
+
+ if (!value)
+ return config_error_nonbool(key);
for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
if (heuristics[i].heuristic &&
heuristics[i].name &&
@@ -209,6 +214,8 @@ static int bundle_list_update(const char *key, const char *value,
strbuf_release(&id);
if (!strcmp(subkey, "uri")) {
+ if (!value)
+ return config_error_nonbool(key);
if (bundle->uri)
return -1;
bundle->uri = relative_url(list->baseURI, value, NULL);
@@ -216,6 +223,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "creationtoken")) {
+ if (!value)
+ return config_error_nonbool(key);
if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
warning(_("could not parse bundle list key %s with value '%s'"),
"creationToken", value);
diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
index 70ae94c4d0..a91c1b428d 100755
--- a/t/t5750-bundle-uri-parse.sh
+++ b/t/t5750-bundle-uri-parse.sh
@@ -312,4 +312,20 @@ test_expect_success 'parse config format: bundle with url instead of uri' '
test_grep "bundle '\''typo'\'' has no uri" err
'
+test_expect_success 'parse config format edge cases: keys without a value' '
+ for key in mode heuristic
+ do
+ printf "[bundle]\n\tversion = 1\n\t%s\n" "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.$key." err || return 1
+ done &&
+ for key in uri creationtoken
+ do
+ printf "[bundle]\n\tversion = 1\n\tmode = all\n[bundle \"one\"]\n\t%s\n" \
+ "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.one.$key." err || return 1
+ done
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-10 7:36 ` [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
@ 2026-11-10 11:11 ` Phillip Wood
2026-11-10 15:03 ` Derrick Stolee
0 siblings, 1 reply; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:11 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
I'll defer to Stolee on what's the best behavior here, but it seems that
in a couple of places we're erroring out on a NULL value when we would
ignore an invalid value.
On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
>
> diff --git a/bundle-uri.c b/bundle-uri.c
> index 67a717e857..41c817eefb 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c> @@ -175,6 +177,9 @@ static int bundle_list_update(const char *key,
const char *value,
>
> if (!strcmp(subkey, "heuristic")) {
> int i;
> +
> + if (!value)
> + return config_error_nonbool(key);
Adding a bit more context below shows we ignore unknown heuristics, so
do we really want to return an error here?
> for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
> if (heuristics[i].heuristic &&
> heuristics[i].name &&
!strcmp(value, heuristics[i].name)) {
list->heuristic =
heuristics[i].heuristic;
return 0;
}
}
/* Ignore unknown heuristics. */
return 0;
> @@ -216,6 +223,8 @@ static int bundle_list_update(const char *key, const char *value,
> }
>
> if (!strcmp(subkey, "creationtoken")) {
> + if (!value)
> + return config_error_nonbool(key);
> if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
> warning(_("could not parse bundle list key %s with value '%s'"),
> "creationToken", value);
return 0;
}
Here we ignore parse errors, so should we be returning an error on a
NULL value?
Thanks
Phillip
> diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
> index 70ae94c4d0..a91c1b428d 100755
> --- a/t/t5750-bundle-uri-parse.sh
> +++ b/t/t5750-bundle-uri-parse.sh
> @@ -312,4 +312,20 @@ test_expect_success 'parse config format: bundle with url instead of uri' '
> test_grep "bundle '\''typo'\'' has no uri" err
> '
>
> +test_expect_success 'parse config format edge cases: keys without a value' '
> + for key in mode heuristic
> + do
> + printf "[bundle]\n\tversion = 1\n\t%s\n" "$key" >input &&
> + test_must_fail test-tool bundle-uri parse-config input 2>err &&
> + test_grep "missing value for .bundle.$key." err || return 1
> + done &&
> + for key in uri creationtoken
> + do
> + printf "[bundle]\n\tversion = 1\n\tmode = all\n[bundle \"one\"]\n\t%s\n" \
> + "$key" >input &&
> + test_must_fail test-tool bundle-uri parse-config input 2>err &&
> + test_grep "missing value for .bundle.one.$key." err || return 1
> + done
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-10 11:11 ` Phillip Wood
@ 2026-11-10 15:03 ` Derrick Stolee
0 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-10 15:03 UTC (permalink / raw)
To: phillip.wood, Thomas Bachem via GitGitGadget, git
Cc: Jeff King, Taylor Blau, Patrick Steinhardt, Colin Hinton,
Matt Hunter, Johannes Schindelin, Usman Akinyemi, Miklos Vajna,
Junio C Hamano, Thomas Bachem
On 11/10/2026 6:11 AM, Phillip Wood wrote:
> I'll defer to Stolee on what's the best behavior here, but it seems that in a couple of places we're erroring out on a NULL value when we would ignore an invalid value.
>
> On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
>> From: Thomas Bachem <mail@thomasbachem.com>
>> + if (!value)
>> + return config_error_nonbool(key);
>
> Adding a bit more context below shows we ignore unknown heuristics, so do we really want to return an error here?
>> if (!strcmp(subkey, "creationtoken")) {
>> + if (!value)
>> + return config_error_nonbool(key);
>> if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
>> warning(_("could not parse bundle list key %s with value '%s'"),
>> "creationToken", value);
> return 0;
> }
>
> Here we ignore parse errors, so should we be returning an error on a NULL value?
I think that the bundle URI feature could be hardened to halt (safely,
with good error messages) instead of continuing in a best-effort way.
The cost to being so flexible is that a user can download partial
bundles that don't even fully install and then fetches too much data
from the Git remote.
But I also think that this series isn't the right place to do that.
You're right that this method becomes inconsistent with how it handles
certain parsing errors versus others, but we are upgrading from SEGFAULT
to error.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH v2 15/15] config: handle NULL values when listing colors and expiry dates
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (13 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
@ 2026-11-10 7:36 ` Thomas Bachem via GitGitGadget
2026-11-10 11:17 ` [PATCH v2 00/15] Fix segfaults on settings given without a value Phillip Wood
15 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-10 7:36 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
Since 2d4ab5a885 (config: format colors quietly, 2026-02-23) and
9cb4a5e1ba (config: format expiry dates quietly, 2026-02-23),
"git config list --type=color" and "--type=expiry-date" pass every
value to color_parse_quietly() or parse_expiry_date(), which do not
handle NULL. So both segfault when any key in the configuration has
no value.
Skip such a key quietly, as both already do with a value that does
not parse, and as "--type=int" does with a key without a value.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/config.c | 4 ++--
t/t1300-config.sh | 10 ++++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 2554322317..f786c0b123 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -366,7 +366,7 @@ static int format_config_expiry_date(struct strbuf *buf,
{
timestamp_t t;
if (quietly) {
- if (parse_expiry_date(value_, &t))
+ if (!value_ || parse_expiry_date(value_, &t))
return -1;
} else if (git_config_expiry_date(&t, key_, value_) < 0) {
return -1;
@@ -384,7 +384,7 @@ static int format_config_color(struct strbuf *buf,
char v[COLOR_MAXLEN];
if (gently) {
- if (color_parse_quietly(value_, v) < 0)
+ if (!value_ || color_parse_quietly(value_, v) < 0)
return -1;
} else if (git_config_color(v, key_, value_) < 0) {
return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 8fb920b1ba..3322b0bf4f 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2648,6 +2648,16 @@ test_expect_success 'list --type=color shows only canonicalizable color values'
test_must_be_empty err
'
+test_expect_success 'list --type=color and --type=expiry-date skip keys without a value' '
+ git -c section.valueless config ${mode_prefix}list --type=color >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err &&
+
+ git -c section.valueless config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err
+'
+
test_expect_success '--type rejects unknown specifiers' '
test_must_fail git config --type=nonsense section.foo 2>error &&
test_grep "unrecognized --type argument" error
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH v2 00/15] Fix segfaults on settings given without a value
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
` (14 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
@ 2026-11-10 11:17 ` Phillip Wood
2026-11-10 20:38 ` Thomas Bachem
15 siblings, 1 reply; 69+ messages in thread
From: Phillip Wood @ 2026-11-10 11:17 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
I've left a few comments on the test for the first patch and on the
later patches that I didn't look at yesterday. There are several places
where we now treat a NULL value as an error, but we ignore empty or
invalid values. Having said that this series is clearly an improvement
because we no longer segfault.
Thanks
Phillip
On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
> A setting given without a value, as in "git -c foo.bar" or a bare "bar"
> under "[foo]", is passed as NULL to the code that reads it. In these places
> git does not check for that and segfaults:
>
> * rebase.missingCommitsCheck in "git rebase -i"
> * core.alternateRefsCommand and core.alternateRefsPrefixes in "git fetch",
> "git receive-pack" and "git rev-list --alternate-refs", in a repository
> with an alternate
> * the color slots of "git add -p", "git add -i" and the other interactive
> patch modes
> * fetch.bundleCreationToken in "git fetch" and "git clone --bundle-uri",
> with a creationToken bundle list
> * grep.patternType in "git grep", "git log" and "git show"
> * remote.*.tagOpt in "git fetch", "git remote" and "git status"
> * color.blame.repeatedLines and color.blame.highlightRecent in "git blame"
> * branch.*.mergeOptions in "git merge" on that branch
> * remotes.* in "git fetch" and "git push" of that remote group
> * safe.bareRepository in any bare repository that git discovers
> * bitmapPseudoMerge.*.pattern when writing a bitmap, as in "git repack
> -adb"
> * scalar.repo in "scalar reconfigure --all"
> * remote.*.url in nearly every command, when an includeIf
> "hasconfig:remote.*.url:..." section exists
> * mode, heuristic, uri and creationToken in a bundle list, in "git clone
> --bundle-uri" and "git fetch"
> * any key at all in "git config list --type=color" and "--type=expiry-date"
>
> Changes since v1:
>
> * Patch 1 no longer rejects a rebase.missingCommitsCheck without a value.
> It warns and falls back to "ignore", as for a value that git does not
> recognize (Phillip, Junio).
>
> * Rebased onto master, where iw/connected-deepen-fetch-no-alternate-refs
> now adds tests at the end of t5618, as patch 2 does.
>
> These turned up in a check of every config callback and every caller of
> repo_config_get_value(), and in runs of two dozen common commands with each
> key that "git help --config" lists set without a value. There may be more in
> code that neither method reached.
>
> Most patches reject the missing value with config_error_nonbool(), as
> ba176db511 (config: handle NULL value when parsing non-bools, 2023-12-07)
> did for other settings. Four accept it without an error: a missing
> rebase.missingCommitsCheck warns and falls back to "ignore" like a value
> that git does not recognize (patch 1), a missing fetch.bundleCreationToken
> counts as 0 (patch 4), a remote URL without a value matches no includeIf
> condition (patch 13), and "git config list" leaves the key out (patch 15).
> The crash fixed in patch 14 can also come from a server, through a bundle
> list it advertises to clients that use bundle URIs. It is still a plain NULL
> dereference. Only patch 15 fixes a regression, which came in with 2.54.0.
>
> remote.*.followRemoteHEAD without a value crashes the same commands. This
> series leaves it alone, as ch/remote-defer-follow-remote-head-validation in
> 'seen' already makes it safe, checking the value only where fetch uses it.
>
> The series is now based on 3c60ef1e38 (The 17th batch, 2026-11-08) and
> merges cleanly into 'next' and 'seen' too.
>
> Thomas Bachem (15):
> rebase -i: handle NULL value for rebase.missingCommitsCheck
> odb: handle NULL values for core.alternateRefs* config
> add-patch: handle NULL value for color slots
> bundle-uri: handle NULL value for fetch.bundleCreationToken
> grep: handle NULL value for grep.patternType
> remote: handle NULL value for remote.<name>.tagOpt
> blame: handle NULL values for color.blame.* config
> merge: handle NULL value for branch.<name>.mergeOptions
> remote: handle NULL value for remotes.<group>
> setup: handle NULL value for safe.bareRepository
> pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
> scalar: handle NULL value for scalar.repo
> config: handle NULL value for remote URLs in hasconfig includes
> bundle-uri: handle NULL values in bundle lists
> config: handle NULL values when listing colors and expiry dates
>
> add-patch.c | 2 +-
> builtin/blame.c | 4 ++++
> builtin/config.c | 4 ++--
> builtin/merge.c | 2 ++
> bundle-uri.c | 12 +++++++++++-
> config.c | 3 ++-
> grep.c | 2 ++
> odb.c | 4 ++--
> pseudo-merge.c | 4 ++++
> rebase-interactive.c | 10 ++++++++--
> remote.c | 7 ++++++-
> scalar.c | 5 ++++-
> setup.c | 2 ++
> t/t0035-safe-bare-repository.sh | 6 ++++++
> t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
> t/t3404-rebase-interactive.sh | 12 ++++++++++++
> t/t3701-add-interactive.sh | 12 ++++++++++++
> t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
> t/t5505-remote.sh | 5 +++++
> t/t5506-remote-groups.sh | 5 +++++
> t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
> t/t5618-alternate-refs.sh | 9 +++++++++
> t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
> t/t7600-merge.sh | 6 ++++++
> t/t7810-grep.sh | 5 +++++
> t/t8012-blame-colors.sh | 7 +++++++
> t/t9210-scalar.sh | 5 +++++
> 27 files changed, 195 insertions(+), 11 deletions(-)
>
>
> base-commit: 3c60ef1e38ee80337025a1b908327f7879386329
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2297%2Fthomasbachem%2Fconfig-null-value-segfaults-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2297/thomasbachem/config-null-value-segfaults-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/2297
>
> Range-diff vs v1:
>
> 1: a9a3f4b705 ! 1: 47224a9893 rebase -i: handle NULL value for rebase.missingCommitsCheck
> @@ Commit message
> segfault as it writes the todo list: get_missing_commit_check_level()
> passes the NULL value to strcasecmp().
>
> - The setting takes "ignore", "warn" or "error", not a bool. Read it
> - with repo_config_get_string_tmp(), which rejects a missing value with
> - config_error_nonbool(), as ba176db511 (config: handle NULL value when
> - parsing non-bools, 2023-12-07) did for other settings.
> + The setting takes "ignore", "warn" or "error". An unrecognized value,
> + perhaps one meant for a newer version of Git, gets a warning and is
> + treated as "ignore". Treat a missing value the same way.
>
> + Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Assisted-by: Claude Fable 5.1
> + Assisted-by: Claude Opus 5.5
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>
> ## rebase-interactive.c ##
> @@ rebase-interactive.c: static enum missing_commit_check_level get_missing_commit_
> const char *value;
>
> - if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
> -+ if (repo_config_get_string_tmp(the_repository, "rebase.missingcommitscheck", &value) ||
> - !strcasecmp("ignore", value))
> +- !strcasecmp("ignore", value))
> ++ if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value))
> ++ return MISSING_COMMIT_CHECK_IGNORE;
> ++ if (!value) {
> ++ warning(_("missing value for option "
> ++ "rebase.missingCommitsCheck. Ignoring."));
> ++ return MISSING_COMMIT_CHECK_IGNORE;
> ++ }
> ++ if (!strcasecmp("ignore", value))
> return MISSING_COMMIT_CHECK_IGNORE;
> if (!strcasecmp("warn", value))
> + return MISSING_COMMIT_CHECK_WARN;
>
> ## t/t3404-rebase-interactive.sh ##
> @@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> -+test_expect_success 'rebase -i rejects rebase.missingCommitsCheck without a value' '
> ++test_expect_success 'rebase -i warns about and ignores rebase.missingCommitsCheck without a value' '
> + rebase_setup_and_clean missing-commit &&
> -+ test_when_finished "test_might_fail git rebase --abort" &&
> -+ test_must_fail git -c rebase.missingCommitsCheck rebase -i --root 2>err &&
> -+ test_grep "missing value for .rebase.missingcommitscheck." err
> ++ (
> ++ set_fake_editor &&
> ++ FAKE_LINES="1 2 3 4" \
> ++ git -c rebase.missingCommitsCheck rebase -i --root 2>actual
> ++ ) &&
> ++ test D = $(git cat-file commit HEAD | sed -ne \$p) &&
> ++ test_grep "missing value for option rebase.missingCommitsCheck" actual &&
> ++ test_grep ! "commits may have been dropped" actual
> +'
> +
> test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
> 2: 74ab5bfc62 ! 2: 9170c4162f odb: handle NULL values for core.alternateRefs* config
> @@ odb.c: static void fill_alternate_refs_command(struct repository *repo,
> }
>
> ## t/t5618-alternate-refs.sh ##
> -@@ t/t5618-alternate-refs.sh: test_expect_success 'log --source shows .alternate marker' '
> +@@ t/t5618-alternate-refs.sh: test_expect_success 'refs of a shallow alternate are ignored' '
> test_cmp expect actual
> '
>
> 3: 1b5f243294 = 3: d3fd6290e0 add-patch: handle NULL value for color slots
> 4: 4eb7085084 = 4: a822cbb844 bundle-uri: handle NULL value for fetch.bundleCreationToken
> 5: 88df66ab7e = 5: 6c2a671a92 grep: handle NULL value for grep.patternType
> 6: e95249fb51 = 6: d4b49d1363 remote: handle NULL value for remote.<name>.tagOpt
> 7: 260b90a4a2 = 7: 25843b43bf blame: handle NULL values for color.blame.* config
> 8: e10f1fc0ba = 8: d8681a8560 merge: handle NULL value for branch.<name>.mergeOptions
> 9: 0faf4e884f = 9: 51f8eba073 remote: handle NULL value for remotes.<group>
> 10: 0991a1b80c = 10: a5b6436943 setup: handle NULL value for safe.bareRepository
> 11: 3f4cbf4494 = 11: 1533ef9cc6 pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
> 12: b4bcaf19ec = 12: 6219e9fd7f scalar: handle NULL value for scalar.repo
> 13: 5c12b13d90 = 13: 080106d5ea config: handle NULL value for remote URLs in hasconfig includes
> 14: ce4c190ea2 = 14: fb9e63541b bundle-uri: handle NULL values in bundle lists
> 15: 7ac4e3a3d9 = 15: 12f841e5ff config: handle NULL values when listing colors and expiry dates
>
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH v2 00/15] Fix segfaults on settings given without a value
2026-11-10 11:17 ` [PATCH v2 00/15] Fix segfaults on settings given without a value Phillip Wood
@ 2026-11-10 20:38 ` Thomas Bachem
0 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem @ 2026-11-10 20:38 UTC (permalink / raw)
To: phillip.wood
Cc: gitgitgadget, git, peff, me, stolee, ps, colinlewishinton, m,
Johannes.Schindelin, usmanakinyemi202, vmiklos, gitster
Phillip Wood <phillip.wood123@gmail.com> writes:
> There are several places where we now treat a NULL value as an error,
> but we ignore empty or invalid values.
Thanks for reading the rest of the series too. Four of those places
reject a missing value while an invalid one is ignored, and they are
the only ones in the series. In v3, two of them will warn about a
missing value and then ignore it, as patch 1 now does:
- remote.<name>.tagOpt (patch 6)
- color.blame.repeatedLines (patch 7)
The other two, heuristic and creationToken in a bundle list
(patch 14), keep the error. Stolee would rather make the bundle URI
code stricter about malformed lists than more lenient.
Two more keep the error, as an invalid value there dies already:
color.blame.highlightRecent in parse_color_fields(), and
branch.<name>.mergeOptions on bad quoting or an unknown option.
Junio has already explained why a missing mergeOptions shouldn't
act like an empty one.
v3 also takes both of your suggestions for the test in patch 1.
Thomas
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH v3 00/15] Fix segfaults on settings given without a value
2026-11-07 8:42 [PATCH 00/15] Fix segfaults on settings given without a value Thomas Bachem via GitGitGadget
` (16 preceding siblings ...)
2026-11-10 7:36 ` [PATCH v2 " Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
` (16 more replies)
17 siblings, 17 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
A setting given without a value, as in "git -c foo.bar" or a bare "bar"
under "[foo]", is passed as NULL to the code that reads it. In these places
git does not check for that and segfaults:
* rebase.missingCommitsCheck in "git rebase -i"
* core.alternateRefsCommand and core.alternateRefsPrefixes in "git fetch",
"git receive-pack" and "git rev-list --alternate-refs", in a repository
with an alternate
* the color slots of "git add -p", "git add -i" and the other interactive
patch modes
* fetch.bundleCreationToken in "git fetch" and "git clone --bundle-uri",
with a creationToken bundle list
* grep.patternType in "git grep", "git log" and "git show"
* remote.*.tagOpt in "git fetch", "git remote" and "git status"
* color.blame.repeatedLines and color.blame.highlightRecent in "git blame"
* branch.*.mergeOptions in "git merge" on that branch
* remotes.* in "git fetch" and "git push" of that remote group
* safe.bareRepository in any bare repository that git discovers
* bitmapPseudoMerge.*.pattern when writing a bitmap, as in "git repack
-adb"
* scalar.repo in "scalar reconfigure --all"
* remote.*.url in nearly every command, when an includeIf
"hasconfig:remote.*.url:..." section exists
* mode, heuristic, uri and creationToken in a bundle list, in "git clone
--bundle-uri" and "git fetch"
* any key at all in "git config list --type=color" and "--type=expiry-date"
Changes since v2:
* Where an invalid value is ignored, a missing one now is too, with a
warning: remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch
7). v2 rejected them (Phillip).
* The test in patch 1 rebases onto HEAD~3 and checks the result with
test_commit_message (Phillip).
These turned up in a check of every config callback and every caller of
repo_config_get_value(), and in runs of two dozen common commands with each
key that "git help --config" lists set without a value. There may be more in
code that neither method reached.
Most patches reject the missing value with config_error_nonbool(), as
ba176db511 (config: handle NULL value when parsing non-bools, 2023-12-07)
did for other settings. Where git ignores an invalid value, it now ignores a
missing one too, with a warning: rebase.missingCommitsCheck (patch 1),
remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch 7). The
exception is a bundle list, where patch 14 rejects a missing heuristic or
creationToken even though an invalid one is accepted, as Derrick would
rather make the bundle URI code stricter about malformed lists than more
lenient. A missing fetch.bundleCreationToken counts as 0 (patch 4), a remote
URL without a value matches no includeIf condition (patch 13), and "git
config list" leaves the key out (patch 15). The crash fixed in patch 14 can
also come from a server, through a bundle list it advertises to clients that
use bundle URIs. It is still a plain NULL dereference. Only patch 15 fixes a
regression, which came in with 2.54.0.
remote.*.followRemoteHEAD without a value crashes the same commands. This
series leaves it alone, as ch/remote-defer-follow-remote-head-validation in
'seen' already makes it safe, checking the value only where fetch uses it.
The series is still based on 3c60ef1e38 (The 17th batch, 2026-11-08) and
merges cleanly into 'master', 'next' and 'seen'.
Thomas Bachem (15):
rebase -i: handle NULL value for rebase.missingCommitsCheck
odb: handle NULL values for core.alternateRefs* config
add-patch: handle NULL value for color slots
bundle-uri: handle NULL value for fetch.bundleCreationToken
grep: handle NULL value for grep.patternType
remote: handle NULL value for remote.<name>.tagOpt
blame: handle NULL values for color.blame.* config
merge: handle NULL value for branch.<name>.mergeOptions
remote: handle NULL value for remotes.<group>
setup: handle NULL value for safe.bareRepository
pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
scalar: handle NULL value for scalar.repo
config: handle NULL value for remote URLs in hasconfig includes
bundle-uri: handle NULL values in bundle lists
config: handle NULL values when listing colors and expiry dates
add-patch.c | 2 +-
builtin/blame.c | 7 +++++++
builtin/config.c | 4 ++--
builtin/merge.c | 2 ++
bundle-uri.c | 12 +++++++++++-
config.c | 3 ++-
grep.c | 2 ++
odb.c | 4 ++--
pseudo-merge.c | 4 ++++
rebase-interactive.c | 10 ++++++++--
remote.c | 9 ++++++++-
scalar.c | 5 ++++-
setup.c | 2 ++
t/t0035-safe-bare-repository.sh | 6 ++++++
t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
t/t3404-rebase-interactive.sh | 12 ++++++++++++
t/t3701-add-interactive.sh | 12 ++++++++++++
t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
t/t5505-remote.sh | 7 +++++++
t/t5506-remote-groups.sh | 5 +++++
t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
t/t5618-alternate-refs.sh | 9 +++++++++
t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
t/t7600-merge.sh | 6 ++++++
t/t7810-grep.sh | 5 +++++
t/t8012-blame-colors.sh | 12 ++++++++++++
t/t9210-scalar.sh | 5 +++++
27 files changed, 207 insertions(+), 11 deletions(-)
base-commit: 3c60ef1e38ee80337025a1b908327f7879386329
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2297%2Fthomasbachem%2Fconfig-null-value-segfaults-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2297/thomasbachem/config-null-value-segfaults-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2297
Range-diff vs v2:
1: 47224a9893 ! 1: 1262222ccd rebase -i: handle NULL value for rebase.missingCommitsCheck
@@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.mi
+ rebase_setup_and_clean missing-commit &&
+ (
+ set_fake_editor &&
-+ FAKE_LINES="1 2 3 4" \
-+ git -c rebase.missingCommitsCheck rebase -i --root 2>actual
++ FAKE_LINES=1 \
++ git -c rebase.missingCommitsCheck rebase -i HEAD~3 2>actual
+ ) &&
-+ test D = $(git cat-file commit HEAD | sed -ne \$p) &&
++ test_commit_message HEAD -m C &&
+ test_grep "missing value for option rebase.missingCommitsCheck" actual &&
+ test_grep ! "commits may have been dropped" actual
+'
2: 9170c4162f = 2: ec9d7fd183 odb: handle NULL values for core.alternateRefs* config
3: d3fd6290e0 = 3: c071e79d11 add-patch: handle NULL value for color slots
4: a822cbb844 = 4: 8773aaef0b bundle-uri: handle NULL value for fetch.bundleCreationToken
5: 6c2a671a92 = 5: 123f03f024 grep: handle NULL value for grep.patternType
6: d4b49d1363 ! 6: 339590cd4b remote: handle NULL value for remote.<name>.tagOpt
@@ Commit message
"git remote" and "git status" segfault: handle_config() passes the
NULL value to strcmp().
- Reject a missing value with config_error_nonbool(), as handle_config()
- already does for remote.<name>.url and remote.<name>.pushurl.
+ The setting takes "--no-tags" or "--tags", and any other value is
+ ignored. Ignore a missing value too, but warn about it.
+ Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
+ Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
## remote.c ##
@@ remote.c: static int handle_config(const char *key, const char *value,
else
error(_("more than one uploadpack given, using the first"));
} else if (!strcmp(subkey, "tagopt")) {
-+ if (!value)
-+ return config_error_nonbool(key);
++ if (!value) {
++ warning(_("missing value for '%s'"), key);
++ return 0;
++ }
if (!strcmp(value, "--no-tags"))
remote->fetch_tags = -1;
else if (!strcmp(value, "--tags"))
@@ t/t5505-remote.sh: test_expect_success 'remote information for the origin' '
)
'
-+test_expect_success 'tagOpt needs a value' '
-+ test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
++test_expect_success 'tagOpt without a value only warns' '
++ git -C test -c remote.origin.tagOpt remote >actual 2>err &&
++ echo origin >expect &&
++ test_cmp expect actual &&
+ test_grep "missing value for .remote.origin.tagopt." err
+'
+
7: 25843b43bf ! 7: ad50c4b35b blame: handle NULL values for color.blame.* config
@@ Commit message
value makes "git blame" segfault: git_blame_config() passes the NULL
value to strlen() or to parse_color_fields().
- Reject a missing value with config_error_nonbool(), as
+ An invalid color.blame.repeatedLines gets a warning and is ignored.
+ Treat a missing value the same way.
+
+ An invalid color.blame.highlightRecent makes "git blame" die, so
+ reject a missing value there with config_error_nonbool(), as
git_blame_config() already does for blame.date and blame.coloring.
+ Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
+ Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
## builtin/blame.c ##
@@ builtin/blame.c: static int git_blame_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "color.blame.repeatedlines")) {
-+ if (!value)
-+ return config_error_nonbool(var);
++ if (!value) {
++ warning(_("missing value for '%s'"),
++ "color.blame.repeatedLines");
++ return 0;
++ }
if (color_parse_mem(value, strlen(value), repeated_meta_color))
warning(_("invalid value for '%s': '%s'"),
"color.blame.repeatedLines", value);
@@ t/t8012-blame-colors.sh: test_expect_success 'blame color by age: new code is di
test_grep qfunc colored
'
-+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
-+ test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
-+ test_grep "missing value for .color.blame.repeatedlines." err &&
++test_expect_success 'color.blame.repeatedLines without a value only warns' '
++ git blame --color-lines hello.c >expect &&
++ git -c color.blame.repeatedLines blame --color-lines hello.c >actual 2>err &&
++ test_cmp expect actual &&
++ test_grep "missing value for .color.blame.repeatedLines." err
++'
++
++test_expect_success 'color.blame.highlightRecent needs a value' '
+ test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.highlightrecent." err
+'
8: d8681a8560 = 8: 90209dc754 merge: handle NULL value for branch.<name>.mergeOptions
9: 51f8eba073 = 9: f30ecf07b4 remote: handle NULL value for remotes.<group>
10: a5b6436943 = 10: bb6eecd20c setup: handle NULL value for safe.bareRepository
11: 1533ef9cc6 = 11: 510dc56c9b pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
12: 6219e9fd7f = 12: 751fae34b2 scalar: handle NULL value for scalar.repo
13: 080106d5ea = 13: 4d5d2ce04e config: handle NULL value for remote URLs in hasconfig includes
14: fb9e63541b = 14: 474aebd572 bundle-uri: handle NULL values in bundle lists
15: 12f841e5ff = 15: f6f3295ff5 config: handle NULL values when listing colors and expiry dates
--
gitgitgadget
^ permalink raw reply [flat|nested] 69+ messages in thread* [PATCH v3 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
` (15 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A rebase.missingCommitsCheck without a value, as in
[rebase]
missingCommitsCheck
or "git -c rebase.missingCommitsCheck", makes "git rebase -i"
segfault as it writes the todo list: get_missing_commit_check_level()
passes the NULL value to strcasecmp().
The setting takes "ignore", "warn" or "error". An unrecognized value,
perhaps one meant for a newer version of Git, gets a warning and is
treated as "ignore". Treat a missing value the same way.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
rebase-interactive.c | 10 ++++++++--
t/t3404-rebase-interactive.sh | 12 ++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 809f76a87b..9eb70b09b3 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -30,8 +30,14 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
{
const char *value;
- if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value) ||
- !strcasecmp("ignore", value))
+ if (repo_config_get_value(the_repository, "rebase.missingcommitscheck", &value))
+ return MISSING_COMMIT_CHECK_IGNORE;
+ if (!value) {
+ warning(_("missing value for option "
+ "rebase.missingCommitsCheck. Ignoring."));
+ return MISSING_COMMIT_CHECK_IGNORE;
+ }
+ if (!strcasecmp("ignore", value))
return MISSING_COMMIT_CHECK_IGNORE;
if (!strcasecmp("warn", value))
return MISSING_COMMIT_CHECK_WARN;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index b3ee96f560..16fdf6b6e3 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1562,6 +1562,18 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'rebase -i warns about and ignores rebase.missingCommitsCheck without a value' '
+ rebase_setup_and_clean missing-commit &&
+ (
+ set_fake_editor &&
+ FAKE_LINES=1 \
+ git -c rebase.missingCommitsCheck rebase -i HEAD~3 2>actual
+ ) &&
+ test_commit_message HEAD -m C &&
+ test_grep "missing value for option rebase.missingCommitsCheck" actual &&
+ test_grep ! "commits may have been dropped" actual
+'
+
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
test_config rebase.missingCommitsCheck ignore &&
rebase_setup_and_clean missing-commit &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 02/15] odb: handle NULL values for core.alternateRefs* config
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
` (14 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A core.alternateRefsCommand or core.alternateRefsPrefixes without a
value makes "git fetch", "git receive-pack" and
"git rev-list --alternate-refs" segfault in a repository that borrows
objects from an alternate: fill_alternate_refs_command() passes the
NULL value to strvec_push() or strvec_split().
Both settings take a string. Read them with
repo_config_get_string_tmp(), which rejects a missing value with
config_error_nonbool(), as the previous commit does for
rebase.missingCommitsCheck.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
odb.c | 4 ++--
t/t5618-alternate-refs.sh | 9 +++++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/odb.c b/odb.c
index 865c404910..ea086be8b3 100644
--- a/odb.c
+++ b/odb.c
@@ -161,7 +161,7 @@ static void fill_alternate_refs_command(struct repository *repo,
{
const char *value;
- if (!repo_config_get_value(repo, "core.alternateRefsCommand", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsCommand", &value)) {
cmd->use_shell = 1;
strvec_push(&cmd->args, value);
@@ -173,7 +173,7 @@ static void fill_alternate_refs_command(struct repository *repo,
strvec_push(&cmd->args, "for-each-ref");
strvec_push(&cmd->args, "--format=%(objectname)");
- if (!repo_config_get_value(repo, "core.alternateRefsPrefixes", &value)) {
+ if (!repo_config_get_string_tmp(repo, "core.alternateRefsPrefixes", &value)) {
strvec_push(&cmd->args, "--");
strvec_split(&cmd->args, value);
}
diff --git a/t/t5618-alternate-refs.sh b/t/t5618-alternate-refs.sh
index 68139a1b3e..422da6dde7 100755
--- a/t/t5618-alternate-refs.sh
+++ b/t/t5618-alternate-refs.sh
@@ -83,4 +83,13 @@ test_expect_success 'refs of a shallow alternate are ignored' '
test_cmp expect actual
'
+test_expect_success 'alternateRefsCommand and alternateRefsPrefixes need a value' '
+ test_must_fail git -c core.alternateRefsCommand \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsCommand." err &&
+ test_must_fail git -c core.alternateRefsPrefixes \
+ rev-list --alternate-refs 2>err &&
+ test_grep "missing value for .core.alternateRefsPrefixes." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 03/15] add-patch: handle NULL value for color slots
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 01/15] rebase -i: handle NULL value for rebase.missingCommitsCheck Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 02/15] odb: handle NULL values for core.alternateRefs* config Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
` (13 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
With color on, a color slot without a value, such as
color.interactive.prompt or color.diff.frag, makes "git add -p",
"git add -i" and the other interactive patch modes segfault:
init_color() passes the NULL value to color_parse().
Read the slot with repo_config_get_string_tmp(), which rejects a
missing value with config_error_nonbool(), as "git diff" already does
for color.diff.frag and the other diff colors.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
add-patch.c | 2 +-
t/t3701-add-interactive.sh | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/add-patch.c b/add-patch.c
index f27edcbe8d..ad581eb4ae 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -295,7 +295,7 @@ static void init_color(struct repository *r,
if (!want_color(use_color))
dst[0] = '\0';
- else if (repo_config_get_value(r, key, &value) ||
+ else if (repo_config_get_string_tmp(r, key, &value) ||
color_parse(value, dst))
strlcpy(dst, default_color, COLOR_MAXLEN);
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6e120a4001..33050ccc53 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -815,6 +815,18 @@ test_expect_success 'colors can be overridden' '
test_cmp expect actual
'
+test_expect_success 'a color without a value is rejected' '
+ git reset --hard &&
+ test_when_finished "git rm -f color-test" &&
+ test_write_lines context old >color-test &&
+ git add color-test &&
+ test_write_lines context new >color-test &&
+
+ test_must_fail git -c color.interactive=always \
+ -c color.interactive.prompt add -p </dev/null 2>err &&
+ test_grep "missing value for .color.interactive.prompt." err
+'
+
test_expect_success 'brackets appear without color' '
git reset --hard &&
test_when_finished "git rm -f bracket-test" &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (2 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 03/15] add-patch: handle NULL value for color slots Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
` (12 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A fetch.bundleCreationToken without a value makes "git fetch" and
"git clone --bundle-uri" segfault when the bundle list uses the
creationToken heuristic: fetch_bundles_by_token() passes the NULL
value to sscanf().
The function already treats a token that does not parse as 0, as if
no bundles had been fetched before, so it downloads them and then
stores the new token. Treat a missing value the same way.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 3 ++-
t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/bundle-uri.c b/bundle-uri.c
index 9797eaed9a..67a717e857 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -572,7 +572,8 @@ static int fetch_bundles_by_token(struct repository *r,
if (!repo_config_get_value(r,
"fetch.bundlecreationtoken",
&creationTokenStr)) {
- if (sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
+ if (!creationTokenStr ||
+ sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) != 1)
maxCreationToken = 0;
if (bundles.items[0]->creationToken <= maxCreationToken) {
free(bundles.items);
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index f81f1f8db7..e86ed7d30c 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -346,6 +346,26 @@ test_expect_success 'clone bundle list (file, any mode, all failures)' '
test_grep ! "refs/bundles/heads/" refs
'
+test_expect_success 'fetch treats fetch.bundleCreationToken without a value as 0' '
+ test_when_finished "rm -rf token-without-value" &&
+ cat >bundle-list <<-EOF &&
+ [bundle]
+ version = 1
+ mode = all
+ heuristic = creationToken
+
+ [bundle "bundle-1"]
+ uri = file://$(pwd)/clone-from/bundle-1.bundle
+ creationToken = 1
+ EOF
+
+ git clone --no-local clone-from token-without-value &&
+ git -C token-without-value \
+ -c fetch.bundleURI="file://$(pwd)/bundle-list" \
+ -c fetch.bundleCreationToken fetch origin &&
+ test_cmp_config -C token-without-value 1 fetch.bundlecreationtoken
+'
+
test_expect_success 'negotiation: bundle with part of wanted commits' '
test_when_finished "rm -f trace*.txt" &&
GIT_TRACE_PACKET="$(pwd)/trace-packet.txt" \
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 05/15] grep: handle NULL value for grep.patternType
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (3 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 04/15] bundle-uri: handle NULL value for fetch.bundleCreationToken Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
` (11 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A grep.patternType without a value makes "git grep", "git log" and
"git show" segfault: grep_config(), which all three use, passes the
NULL value to parse_pattern_type_arg(), which hands it to strcmp().
Reject a missing value with config_error_nonbool(), as grep_config()
already does for the color.grep.* slots.
Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
grep.c | 2 ++
t/t7810-grep.sh | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/grep.c b/grep.c
index 1d75d31421..ab082f963e 100644
--- a/grep.c
+++ b/grep.c
@@ -71,6 +71,8 @@ int grep_config(const char *var, const char *value,
}
if (!strcmp(var, "grep.patterntype")) {
+ if (!value)
+ return config_error_nonbool(var);
opt->pattern_type_option = parse_pattern_type_arg(var, value);
return 0;
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index ac7ea9cf8d..d323927e54 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1551,6 +1551,11 @@ test_expect_success PCRE 'grep invalidpattern properly dies with grep.patternTyp
test_must_fail git -c grep.patterntype=perl grep "a["
'
+test_expect_success 'grep.patternType without a value is rejected' '
+ test_must_fail git -c grep.patternType grep "a" 2>err &&
+ test_grep "missing value for .grep.patterntype." err
+'
+
test_expect_success 'grep -G -E -F pattern' '
echo "ab:a+b*c" >expected &&
git grep -G -E -F "a+b*c" ab >actual &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 06/15] remote: handle NULL value for remote.<name>.tagOpt
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (4 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 05/15] grep: handle NULL value for grep.patternType Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
` (10 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remote.<name>.tagOpt without a value makes "git fetch",
"git remote" and "git status" segfault: handle_config() passes the
NULL value to strcmp().
The setting takes "--no-tags" or "--tags", and any other value is
ignored. Ignore a missing value too, but warn about it.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 4 ++++
t/t5505-remote.sh | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/remote.c b/remote.c
index 49ee2ea318..d3164fee69 100644
--- a/remote.c
+++ b/remote.c
@@ -556,6 +556,10 @@ static int handle_config(const char *key, const char *value,
else
error(_("more than one uploadpack given, using the first"));
} else if (!strcmp(subkey, "tagopt")) {
+ if (!value) {
+ warning(_("missing value for '%s'"), key);
+ return 0;
+ }
if (!strcmp(value, "--no-tags"))
remote->fetch_tags = -1;
else if (!strcmp(value, "--tags"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 5fbfcb0848..8b4c7006ec 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -90,6 +90,13 @@ test_expect_success 'remote information for the origin' '
)
'
+test_expect_success 'tagOpt without a value only warns' '
+ git -C test -c remote.origin.tagOpt remote >actual 2>err &&
+ echo origin >expect &&
+ test_cmp expect actual &&
+ test_grep "missing value for .remote.origin.tagopt." err
+'
+
test_expect_success 'add another remote' '
(
cd test &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 07/15] blame: handle NULL values for color.blame.* config
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (5 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 06/15] remote: handle NULL value for remote.<name>.tagOpt Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
` (9 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A color.blame.repeatedLines or color.blame.highlightRecent without a
value makes "git blame" segfault: git_blame_config() passes the NULL
value to strlen() or to parse_color_fields().
An invalid color.blame.repeatedLines gets a warning and is ignored.
Treat a missing value the same way.
An invalid color.blame.highlightRecent makes "git blame" die, so
reject a missing value there with config_error_nonbool(), as
git_blame_config() already does for blame.date and blame.coloring.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Assisted-by: Claude Fable 5.1
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/blame.c | 7 +++++++
t/t8012-blame-colors.sh | 12 ++++++++++++
2 files changed, 19 insertions(+)
diff --git a/builtin/blame.c b/builtin/blame.c
index 1e753a0b60..55a3151942 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -783,12 +783,19 @@ static int git_blame_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "color.blame.repeatedlines")) {
+ if (!value) {
+ warning(_("missing value for '%s'"),
+ "color.blame.repeatedLines");
+ return 0;
+ }
if (color_parse_mem(value, strlen(value), repeated_meta_color))
warning(_("invalid value for '%s': '%s'"),
"color.blame.repeatedLines", value);
return 0;
}
if (!strcmp(var, "color.blame.highlightrecent")) {
+ if (!value)
+ return config_error_nonbool(var);
parse_color_fields(value);
return 0;
}
diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
index d2653b3c31..2024047c85 100755
--- a/t/t8012-blame-colors.sh
+++ b/t/t8012-blame-colors.sh
@@ -69,4 +69,16 @@ test_expect_success 'blame color by age: new code is different' '
test_grep qfunc colored
'
+test_expect_success 'color.blame.repeatedLines without a value only warns' '
+ git blame --color-lines hello.c >expect &&
+ git -c color.blame.repeatedLines blame --color-lines hello.c >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "missing value for .color.blame.repeatedLines." err
+'
+
+test_expect_success 'color.blame.highlightRecent needs a value' '
+ test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
+ test_grep "missing value for .color.blame.highlightrecent." err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 08/15] merge: handle NULL value for branch.<name>.mergeOptions
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (6 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 07/15] blame: handle NULL values for color.blame.* config Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
` (8 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A branch.<name>.mergeOptions without a value makes "git merge"
segfault on that branch: git_merge_config() passes the NULL value to
xstrdup().
Reject a missing value with config_error_nonbool(), as
git_config_string() already does for the other strings that
git_merge_config() reads.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/merge.c | 2 ++
t/t7600-merge.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index d7bf209114..c6f9356ccb 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
skip_prefix(k, "branch.", &str) &&
skip_prefix(str, branch, &str) &&
!strcmp(str, ".mergeoptions")) {
+ if (!v)
+ return config_error_nonbool(k);
free(branch_mergeoptions);
branch_mergeoptions = xstrdup(v);
return 0;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 8f6109fb91..2da12d6662 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -552,6 +552,12 @@ test_expect_success 'merge c1 with c2 (squash in config)' '
test_cmp squash.1-5 .git/SQUASH_MSG
'
+test_expect_success 'merge rejects branch.main.mergeOptions without a value' '
+ git reset --hard c1 &&
+ test_must_fail git -c branch.main.mergeOptions merge c2 2>err &&
+ test_grep "missing value for .branch.main.mergeoptions." err
+'
+
test_debug 'git log --graph --decorate --oneline --all'
test_expect_success 'override config option -n with --summary' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 09/15] remote: handle NULL value for remotes.<group>
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (7 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 08/15] merge: handle NULL value for branch.<name>.mergeOptions Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
` (7 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A remotes.<group> without a value makes "git fetch <group>" and
"git push <group>" segfault: get_remote_group() dereferences the NULL
value as it splits the list of remotes.
Reject a missing value with config_error_nonbool(), and keep the
whole key for its message by skipping the "remotes." prefix into a
separate variable.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
remote.c | 5 ++++-
t/t5506-remote-groups.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index d3164fee69..80be49acb4 100644
--- a/remote.c
+++ b/remote.c
@@ -2200,8 +2200,11 @@ int get_remote_group(const char *key, const char *value,
void *priv)
{
struct remote_group_data *g = priv;
+ const char *group;
- if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
+ if (skip_prefix(key, "remotes.", &group) && !strcmp(group, g->name)) {
+ if (!value)
+ return config_error_nonbool(key);
/* split list by white space */
while (*value) {
size_t wordlen = strcspn(value, " \t\n");
diff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh
index 16e9a1bc2f..f11eb8511e 100755
--- a/t/t5506-remote-groups.sh
+++ b/t/t5506-remote-groups.sh
@@ -54,6 +54,11 @@ test_expect_success 'nonexistent group produces error' '
! repo_fetched two
'
+test_expect_success 'remotes.<group> without a value is rejected' '
+ test_must_fail git -c remotes.valueless fetch valueless 2>err &&
+ test_grep "missing value for .remotes.valueless." err
+'
+
test_expect_success 'updating group updates all members (remote update)' '
mark group-all &&
update_repos &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 10/15] setup: handle NULL value for safe.bareRepository
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (8 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 09/15] remote: handle NULL value for remotes.<group> Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
` (6 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A safe.bareRepository without a value makes git segfault in every
bare repository it discovers, as in "git -C repo.git log":
allowed_bare_repo_cb() passes the NULL value to strcmp().
Reject a missing value with config_error_nonbool(). The callback
already fails for any value other than "explicit" and "all".
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
setup.c | 2 ++
t/t0035-safe-bare-repository.sh | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/setup.c b/setup.c
index 186815c780..e9a1bdbb50 100644
--- a/setup.c
+++ b/setup.c
@@ -1531,6 +1531,8 @@ static int allowed_bare_repo_cb(const char *key, const char *value,
if (strcasecmp(key, "safe.bareRepository"))
return 0;
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "explicit")) {
*allowed_bare_repo = ALLOWED_BARE_REPO_EXPLICIT;
return 0;
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index 1d3d19f5b4..a0e0c48d7d 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -80,6 +80,12 @@ test_expect_success 'safe.bareRepository on the command line' '
-c safe.bareRepository=all
'
+test_expect_success 'safe.bareRepository without a value is rejected' '
+ test_must_fail git -C outer-repo/bare-repo -c safe.bareRepository \
+ rev-parse --git-dir 2>err &&
+ test_grep "missing value for .safe.barerepository." err
+'
+
test_expect_success 'safe.bareRepository in included file' '
cat >gitconfig-include <<-\EOF &&
[safe]
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (9 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 10/15] setup: handle NULL value for safe.bareRepository Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
` (5 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bitmapPseudoMerge.<name>.pattern without a value makes
"git pack-objects" segfault while it writes a bitmap, e.g. during
"git repack -adb": pseudo_merge_config() dereferences the NULL value
to see whether the pattern starts with "^".
Reject a missing value with config_error_nonbool(), as the callback
already does through git_config_expiry_date() for the threshold keys.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
pseudo-merge.c | 4 ++++
t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/pseudo-merge.c b/pseudo-merge.c
index 22b8600d68..19fae9233a 100644
--- a/pseudo-merge.c
+++ b/pseudo-merge.c
@@ -150,6 +150,10 @@ static int pseudo_merge_config(const char *var, const char *value,
if (!strcmp(key, "pattern")) {
struct strbuf re = STRBUF_INIT;
+ if (!value) {
+ ret = config_error_nonbool(var);
+ goto done;
+ }
if (group->pattern) {
regfree(group->pattern);
free(group->pattern);
diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 5b2a17f90a..e7e6b2f8a8 100755
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -233,6 +233,11 @@ test_expect_success 'out of order thresholds are rejected' '
test_cmp expect err
'
+test_expect_success 'pseudo-merge pattern without a value is rejected' '
+ test_must_fail git -c bitmapPseudoMerge.test.pattern repack -adb 2>err &&
+ test_grep "missing value for .bitmappseudomerge.test.pattern." err
+'
+
test_expect_success 'pseudo-merge pattern with capture groups' '
git init pseudo-merge-captures &&
(
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 12/15] scalar: handle NULL value for scalar.repo
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (10 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 11/15] pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
` (4 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A scalar.repo without a value makes "scalar reconfigure --all"
segfault: get_scalar_repos() passes the NULL value to
string_list_append(), which copies it with xstrdup().
Reject a missing value with config_error_nonbool(). This cannot break
a working setup, as "scalar register" always writes scalar.repo with
a path.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
scalar.c | 5 ++++-
t/t9210-scalar.sh | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/scalar.c b/scalar.c
index a80d8ee3ff..4fe4b2a647 100644
--- a/scalar.c
+++ b/scalar.c
@@ -652,8 +652,11 @@ static int get_scalar_repos(const char *key, const char *value,
{
struct string_list *list = data;
- if (!strcmp(key, "scalar.repo"))
+ if (!strcmp(key, "scalar.repo")) {
+ if (!value)
+ return config_error_nonbool(key);
string_list_append(list, value);
+ }
return 0;
}
diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
index c8463d0ac4..137765d3b8 100755
--- a/t/t9210-scalar.sh
+++ b/t/t9210-scalar.sh
@@ -288,6 +288,11 @@ test_expect_success '`reconfigure -a` removes stale config entries' '
test_cmp expect scalar.repos
'
+test_expect_success '`reconfigure -a` rejects scalar.repo without a value' '
+ test_must_fail scalar -c scalar.repo reconfigure -a 2>err &&
+ test_grep "missing value for .scalar.repo." err
+'
+
test_expect_success 'scalar delete without enlistment shows a usage' '
test_expect_code 129 scalar delete
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 13/15] config: handle NULL value for remote URLs in hasconfig includes
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (11 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 12/15] scalar: handle NULL value for scalar.repo Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
` (3 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
When the configuration has an includeIf "hasconfig:remote.*.url:..."
section, a remote.<name>.url without a value makes nearly every git
command segfault, "git log" too: add_remote_url() passes the NULL
value to string_list_append(), which copies it with xstrdup().
Leave such a URL out of the list that hasconfig:remote.*.url patterns
are matched against. It cannot match a pattern, and wherever git
reads its remotes, handle_config() in remote.c already rejects it
with the usual error. Commands that never look at remotes keep
working.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
config.c | 3 ++-
t/t1300-config.sh | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 5976a418d5..5b823b3dcd 100644
--- a/config.c
+++ b/config.c
@@ -342,7 +342,8 @@ static int add_remote_url(const char *var, const char *value,
if (!parse_config_key(var, "remote", &remote_name, &remote_name_len,
&key) &&
remote_name &&
- !strcmp(key, "url"))
+ !strcmp(key, "url") &&
+ value)
string_list_append(remote_urls, value);
return 0;
}
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e3f8064889..8fb920b1ba 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2970,6 +2970,28 @@ test_expect_success 'includeIf.hasconfig:remote.*.url forbids remote url in such
test_grep "fatal: remote URLs cannot be configured in file directly or indirectly included by includeIf.hasconfig:remote.*.url" err
'
+test_expect_success 'includeIf.hasconfig:remote.*.url skips a url without a value' '
+ git init hasremoteurlTest &&
+ test_when_finished "rm -rf hasremoteurlTest" &&
+
+ cat >include-this <<-\EOF &&
+ [user]
+ this = this-is-included
+ EOF
+ cat >>hasremoteurlTest/.git/config <<-EOF &&
+ [remote "valueless"]
+ url
+ [remote "foo"]
+ url = foourl
+ [includeIf "hasconfig:remote.*.url:foourl"]
+ path = "$(pwd)/include-this"
+ EOF
+
+ echo this-is-included >expect-this &&
+ git -C hasremoteurlTest config --get user.this >actual-this &&
+ test_cmp expect-this actual-this
+'
+
test_expect_success 'negated mode causes failure' '
test_must_fail git config --no-get 2>err &&
test_grep "unknown option \`no-get${SQ}" err
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 14/15] bundle-uri: handle NULL values in bundle lists
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (12 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 13/15] config: handle NULL value for remote URLs in hasconfig includes Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-11 22:49 ` [PATCH v3 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
` (2 subsequent siblings)
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
A bundle list that gives bundle.mode, bundle.heuristic,
bundle.<id>.uri or bundle.<id>.creationToken without a value makes
"git clone --bundle-uri" and "git fetch" segfault:
bundle_list_update() passes the NULL value to strcmp(), relative_url()
or sscanf(). Such a list can also come from a server, through a
bundle URI that it advertises.
Reject a missing value with config_error_nonbool(). That makes the
list invalid, as an unknown mode or a second uri already does, and
the clone or fetch goes on without the bundles.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
bundle-uri.c | 9 +++++++++
t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/bundle-uri.c b/bundle-uri.c
index 67a717e857..41c817eefb 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -164,6 +164,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "mode")) {
+ if (!value)
+ return config_error_nonbool(key);
if (!strcmp(value, "all"))
list->mode = BUNDLE_MODE_ALL;
else if (!strcmp(value, "any"))
@@ -175,6 +177,9 @@ static int bundle_list_update(const char *key, const char *value,
if (!strcmp(subkey, "heuristic")) {
int i;
+
+ if (!value)
+ return config_error_nonbool(key);
for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
if (heuristics[i].heuristic &&
heuristics[i].name &&
@@ -209,6 +214,8 @@ static int bundle_list_update(const char *key, const char *value,
strbuf_release(&id);
if (!strcmp(subkey, "uri")) {
+ if (!value)
+ return config_error_nonbool(key);
if (bundle->uri)
return -1;
bundle->uri = relative_url(list->baseURI, value, NULL);
@@ -216,6 +223,8 @@ static int bundle_list_update(const char *key, const char *value,
}
if (!strcmp(subkey, "creationtoken")) {
+ if (!value)
+ return config_error_nonbool(key);
if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
warning(_("could not parse bundle list key %s with value '%s'"),
"creationToken", value);
diff --git a/t/t5750-bundle-uri-parse.sh b/t/t5750-bundle-uri-parse.sh
index 70ae94c4d0..a91c1b428d 100755
--- a/t/t5750-bundle-uri-parse.sh
+++ b/t/t5750-bundle-uri-parse.sh
@@ -312,4 +312,20 @@ test_expect_success 'parse config format: bundle with url instead of uri' '
test_grep "bundle '\''typo'\'' has no uri" err
'
+test_expect_success 'parse config format edge cases: keys without a value' '
+ for key in mode heuristic
+ do
+ printf "[bundle]\n\tversion = 1\n\t%s\n" "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.$key." err || return 1
+ done &&
+ for key in uri creationtoken
+ do
+ printf "[bundle]\n\tversion = 1\n\tmode = all\n[bundle \"one\"]\n\t%s\n" \
+ "$key" >input &&
+ test_must_fail test-tool bundle-uri parse-config input 2>err &&
+ test_grep "missing value for .bundle.one.$key." err || return 1
+ done
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH v3 15/15] config: handle NULL values when listing colors and expiry dates
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (13 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 14/15] bundle-uri: handle NULL values in bundle lists Thomas Bachem via GitGitGadget
@ 2026-11-11 22:49 ` Thomas Bachem via GitGitGadget
2026-11-12 14:25 ` [PATCH v3 00/15] Fix segfaults on settings given without a value Phillip Wood
2026-11-12 14:25 ` Derrick Stolee
16 siblings, 0 replies; 69+ messages in thread
From: Thomas Bachem via GitGitGadget @ 2026-11-11 22:49 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem, Thomas Bachem
From: Thomas Bachem <mail@thomasbachem.com>
Since 2d4ab5a885 (config: format colors quietly, 2026-02-23) and
9cb4a5e1ba (config: format expiry dates quietly, 2026-02-23),
"git config list --type=color" and "--type=expiry-date" pass every
value to color_parse_quietly() or parse_expiry_date(), which do not
handle NULL. So both segfault when any key in the configuration has
no value.
Skip such a key quietly, as both already do with a value that does
not parse, and as "--type=int" does with a key without a value.
Assisted-by: Claude Opus 5.5
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
builtin/config.c | 4 ++--
t/t1300-config.sh | 10 ++++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 2554322317..f786c0b123 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -366,7 +366,7 @@ static int format_config_expiry_date(struct strbuf *buf,
{
timestamp_t t;
if (quietly) {
- if (parse_expiry_date(value_, &t))
+ if (!value_ || parse_expiry_date(value_, &t))
return -1;
} else if (git_config_expiry_date(&t, key_, value_) < 0) {
return -1;
@@ -384,7 +384,7 @@ static int format_config_color(struct strbuf *buf,
char v[COLOR_MAXLEN];
if (gently) {
- if (color_parse_quietly(value_, v) < 0)
+ if (!value_ || color_parse_quietly(value_, v) < 0)
return -1;
} else if (git_config_color(v, key_, value_) < 0) {
return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 8fb920b1ba..3322b0bf4f 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2648,6 +2648,16 @@ test_expect_success 'list --type=color shows only canonicalizable color values'
test_must_be_empty err
'
+test_expect_success 'list --type=color and --type=expiry-date skip keys without a value' '
+ git -c section.valueless config ${mode_prefix}list --type=color >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err &&
+
+ git -c section.valueless config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+ test_grep ! valueless actual &&
+ test_must_be_empty err
+'
+
test_expect_success '--type rejects unknown specifiers' '
test_must_fail git config --type=nonsense section.foo 2>error &&
test_grep "unrecognized --type argument" error
--
gitgitgadget
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH v3 00/15] Fix segfaults on settings given without a value
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (14 preceding siblings ...)
2026-11-11 22:49 ` [PATCH v3 15/15] config: handle NULL values when listing colors and expiry dates Thomas Bachem via GitGitGadget
@ 2026-11-12 14:25 ` Phillip Wood
2026-11-12 14:25 ` Derrick Stolee
16 siblings, 0 replies; 69+ messages in thread
From: Phillip Wood @ 2026-11-12 14:25 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Derrick Stolee,
Patrick Steinhardt, Colin Hinton, Matt Hunter,
Johannes Schindelin, Usman Akinyemi, Miklos Vajna, Junio C Hamano,
Thomas Bachem
Hi Thomas
On 11/11/2026 22:49, Thomas Bachem via GitGitGadget wrote:
> Changes since v2:
>
> * Where an invalid value is ignored, a missing one now is too, with a
> warning: remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch
> 7). v2 rejected them (Phillip).
>
> * The test in patch 1 rebases onto HEAD~3 and checks the result with
> test_commit_message (Phillip).
That sounds good, and the range-diff below looks as I'd expect. Thanks
for working on this, as far as I'm concerned it is ready for next.
Phillip
> Range-diff vs v2:
>
> 1: 47224a9893 ! 1: 1262222ccd rebase -i: handle NULL value for rebase.missingCommitsCheck
> @@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i respects rebase.mi
> + rebase_setup_and_clean missing-commit &&
> + (
> + set_fake_editor &&
> -+ FAKE_LINES="1 2 3 4" \
> -+ git -c rebase.missingCommitsCheck rebase -i --root 2>actual
> ++ FAKE_LINES=1 \
> ++ git -c rebase.missingCommitsCheck rebase -i HEAD~3 2>actual
> + ) &&
> -+ test D = $(git cat-file commit HEAD | sed -ne \$p) &&
> ++ test_commit_message HEAD -m C &&
> + test_grep "missing value for option rebase.missingCommitsCheck" actual &&
> + test_grep ! "commits may have been dropped" actual
> +'
> 2: 9170c4162f = 2: ec9d7fd183 odb: handle NULL values for core.alternateRefs* config
> 3: d3fd6290e0 = 3: c071e79d11 add-patch: handle NULL value for color slots
> 4: a822cbb844 = 4: 8773aaef0b bundle-uri: handle NULL value for fetch.bundleCreationToken
> 5: 6c2a671a92 = 5: 123f03f024 grep: handle NULL value for grep.patternType
> 6: d4b49d1363 ! 6: 339590cd4b remote: handle NULL value for remote.<name>.tagOpt
> @@ Commit message
> "git remote" and "git status" segfault: handle_config() passes the
> NULL value to strcmp().
>
> - Reject a missing value with config_error_nonbool(), as handle_config()
> - already does for remote.<name>.url and remote.<name>.pushurl.
> + The setting takes "--no-tags" or "--tags", and any other value is
> + ignored. Ignore a missing value too, but warn about it.
>
> + Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Assisted-by: Claude Fable 5.1
> + Assisted-by: Claude Opus 5.5
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>
> ## remote.c ##
> @@ remote.c: static int handle_config(const char *key, const char *value,
> else
> error(_("more than one uploadpack given, using the first"));
> } else if (!strcmp(subkey, "tagopt")) {
> -+ if (!value)
> -+ return config_error_nonbool(key);
> ++ if (!value) {
> ++ warning(_("missing value for '%s'"), key);
> ++ return 0;
> ++ }
> if (!strcmp(value, "--no-tags"))
> remote->fetch_tags = -1;
> else if (!strcmp(value, "--tags"))
> @@ t/t5505-remote.sh: test_expect_success 'remote information for the origin' '
> )
> '
>
> -+test_expect_success 'tagOpt needs a value' '
> -+ test_must_fail git -C test -c remote.origin.tagOpt remote 2>err &&
> ++test_expect_success 'tagOpt without a value only warns' '
> ++ git -C test -c remote.origin.tagOpt remote >actual 2>err &&
> ++ echo origin >expect &&
> ++ test_cmp expect actual &&
> + test_grep "missing value for .remote.origin.tagopt." err
> +'
> +
> 7: 25843b43bf ! 7: ad50c4b35b blame: handle NULL values for color.blame.* config
> @@ Commit message
> value makes "git blame" segfault: git_blame_config() passes the NULL
> value to strlen() or to parse_color_fields().
>
> - Reject a missing value with config_error_nonbool(), as
> + An invalid color.blame.repeatedLines gets a warning and is ignored.
> + Treat a missing value the same way.
> +
> + An invalid color.blame.highlightRecent makes "git blame" die, so
> + reject a missing value there with config_error_nonbool(), as
> git_blame_config() already does for blame.date and blame.coloring.
>
> + Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Assisted-by: Claude Fable 5.1
> + Assisted-by: Claude Opus 5.5
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>
> ## builtin/blame.c ##
> @@ builtin/blame.c: static int git_blame_config(const char *var, const char *value,
> return 0;
> }
> if (!strcmp(var, "color.blame.repeatedlines")) {
> -+ if (!value)
> -+ return config_error_nonbool(var);
> ++ if (!value) {
> ++ warning(_("missing value for '%s'"),
> ++ "color.blame.repeatedLines");
> ++ return 0;
> ++ }
> if (color_parse_mem(value, strlen(value), repeated_meta_color))
> warning(_("invalid value for '%s': '%s'"),
> "color.blame.repeatedLines", value);
> @@ t/t8012-blame-colors.sh: test_expect_success 'blame color by age: new code is di
> test_grep qfunc colored
> '
>
> -+test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
> -+ test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
> -+ test_grep "missing value for .color.blame.repeatedlines." err &&
> ++test_expect_success 'color.blame.repeatedLines without a value only warns' '
> ++ git blame --color-lines hello.c >expect &&
> ++ git -c color.blame.repeatedLines blame --color-lines hello.c >actual 2>err &&
> ++ test_cmp expect actual &&
> ++ test_grep "missing value for .color.blame.repeatedLines." err
> ++'
> ++
> ++test_expect_success 'color.blame.highlightRecent needs a value' '
> + test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
> + test_grep "missing value for .color.blame.highlightrecent." err
> +'
> 8: d8681a8560 = 8: 90209dc754 merge: handle NULL value for branch.<name>.mergeOptions
> 9: 51f8eba073 = 9: f30ecf07b4 remote: handle NULL value for remotes.<group>
> 10: a5b6436943 = 10: bb6eecd20c setup: handle NULL value for safe.bareRepository
> 11: 1533ef9cc6 = 11: 510dc56c9b pseudo-merge: handle NULL value for bitmapPseudoMerge.<name>.pattern
> 12: 6219e9fd7f = 12: 751fae34b2 scalar: handle NULL value for scalar.repo
> 13: 080106d5ea = 13: 4d5d2ce04e config: handle NULL value for remote URLs in hasconfig includes
> 14: fb9e63541b = 14: 474aebd572 bundle-uri: handle NULL values in bundle lists
> 15: 12f841e5ff = 15: f6f3295ff5 config: handle NULL values when listing colors and expiry dates
>
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH v3 00/15] Fix segfaults on settings given without a value
2026-11-11 22:49 ` [PATCH v3 " Thomas Bachem via GitGitGadget
` (15 preceding siblings ...)
2026-11-12 14:25 ` [PATCH v3 00/15] Fix segfaults on settings given without a value Phillip Wood
@ 2026-11-12 14:25 ` Derrick Stolee
16 siblings, 0 replies; 69+ messages in thread
From: Derrick Stolee @ 2026-11-12 14:25 UTC (permalink / raw)
To: Thomas Bachem via GitGitGadget, git
Cc: Phillip Wood, Jeff King, Taylor Blau, Patrick Steinhardt,
Colin Hinton, Matt Hunter, Johannes Schindelin, Usman Akinyemi,
Miklos Vajna, Junio C Hamano, Thomas Bachem
On 11/11/2026 5:49 PM, Thomas Bachem via GitGitGadget wrote:
> A setting given without a value, as in "git -c foo.bar" or a bare "bar"
> under "[foo]", is passed as NULL to the code that reads it. In these places
> git does not check for that and segfaults:
...
> Changes since v2:
>
> * Where an invalid value is ignored, a missing one now is too, with a
> warning: remote.*.tagOpt (patch 6) and color.blame.repeatedLines (patch
> 7). v2 rejected them (Phillip).
>
> * The test in patch 1 rebases onto HEAD~3 and checks the result with
> test_commit_message (Phillip).
I reviewed the range-diff and found it reasonable.
But I was also happy with v2, so I look forward to
Phillip chiming in on v3.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 69+ messages in thread