Fix segfaults on settings given without a value by thomasbachem · Pull Request #2297 · gitgitgadget/git

thomasbachem · 2026-11-05T08:35:00Z

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 ba176db (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 3c60ef1 (The 17th batch, 2026-11-08) and merges cleanly into 'master', 'next' and 'seen'.

Cc: Phillip Wood phillip.wood@dunelm.org.uk
Cc: Jeff King peff@peff.net
Cc: Taylor Blau me@ttaylorr.com
Cc: Derrick Stolee stolee@gmail.com
Cc: Patrick Steinhardt ps@pks.im
Cc: Colin Hinton colinlewishinton@gmail.com
Cc: Matt Hunter m@lfurio.us
Cc: Johannes Schindelin Johannes.Schindelin@gmx.de
Cc: Usman Akinyemi usmanakinyemi202@gmail.com
Cc: Miklos Vajna vmiklos@collabora.com
Cc: Junio C Hamano gitster@pobox.com

thomasbachem · 2026-11-05T08:35:07Z

/preview

gitgitgadget · 2026-11-05T08:36:58Z

Preview email sent as pull.2297.git.1793867795.gitgitgadget@gmail.com

thomasbachem · 2026-11-07T08:41:45Z

/submit

gitgitgadget · 2026-11-07T08:44:21Z

Submitted as pull.2297.git.1794040967.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2297/thomasbachem/config-null-value-segfaults-v1

To fetch this version to local tag pr-2297/thomasbachem/config-null-value-segfaults-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2297/thomasbachem/config-null-value-segfaults-v1

gitgitgadget · 2026-11-09T20:54:37Z

@@ -652,8 +652,11 @@ static int get_scalar_repos(const char *key, const char *value,
 {


Derrick Stolee wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-09T20:54:52Z

@@ -165,6 +165,8 @@ static int bundle_list_update(const char *key, const char *value,
 		}


Derrick Stolee wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-09T20:55:03Z

@@ -572,7 +572,8 @@ static int fetch_bundles_by_token(struct repository *r,
 	if (!repo_config_get_value(r,


Derrick Stolee wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-09T20:55:13Z

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-09T20:55:35Z

@@ -30,7 +30,7 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
 {


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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 &&

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

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.

Thomas Bachem wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-09T20:55:37Z

User Phillip Wood <phillip.wood123@gmail.com> has been added to the cc: list.

thomasbachem · 2026-11-10T07:30:59Z

/preview

gitgitgadget · 2026-11-10T07:32:43Z

Preview email sent as pull.2297.v2.git.1794295919.gitgitgadget@gmail.com

thomasbachem · 2026-11-10T07:35:49Z

/submit

gitgitgadget · 2026-11-10T07:38:23Z

Submitted as pull.2297.v2.git.1794296205.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2297/thomasbachem/config-null-value-segfaults-v2

To fetch this version to local tag pr-2297/thomasbachem/config-null-value-segfaults-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2297/thomasbachem/config-null-value-segfaults-v2

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

Since 2d4ab5a (config: format colors quietly, 2026-02-23) and 9cb4a5e (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>

gitgitgadget · 2026-11-10T18:36:03Z

@@ -164,6 +164,8 @@ static int bundle_list_update(const char *key, const char *value,
 		}


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-10T18:36:06Z

User Phillip Wood <phillip.wood123@gmail.com> has been added to the cc: list.

gitgitgadget · 2026-11-10T18:36:22Z

@@ -668,6 +668,8 @@ static int git_merge_config(const char *k, const char *v,
 	    skip_prefix(k, "branch.", &str) &&


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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' '

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

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?

Phillip Wood wrote on the Git mailing list (how to reply to this email):

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

Ben Knoble wrote on the Git mailing list (how to reply to this email):

> 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?

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

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.

gitgitgadget · 2026-11-10T18:36:33Z

@@ -783,12 +782,16 @@ static int git_blame_config(const char *var, const char *value,
 		return 0;


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-10T18:36:45Z

@@ -556,6 +556,8 @@ static int handle_config(const char *key, const char *value,
 		else


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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 &&

gitgitgadget · 2026-11-10T18:36:56Z

@@ -30,8 +30,14 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
 {


Phillip Wood wrote on the Git mailing list (how to reply to this email):

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 &&

gitgitgadget · 2026-11-10T18:37:08Z

Phillip Wood wrote on the Git mailing list (how to reply to this email):

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
>

gitgitgadget · 2026-11-10T23:42:34Z

Thomas Bachem wrote on the Git mailing list (how to reply to this email):

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

gitgitgadget · 2026-11-11T18:05:33Z

User Ben Knoble <ben.knoble@gmail.com> has been added to the cc: list.

thomasbachem · 2026-11-11T22:43:49Z

/preview

gitgitgadget · 2026-11-11T22:45:17Z

Preview email sent as pull.2297.v3.git.1794437083.gitgitgadget@gmail.com

thomasbachem · 2026-11-11T22:48:18Z

/submit

gitgitgadget · 2026-11-11T22:50:54Z

Submitted as pull.2297.v3.git.1794437355.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2297/thomasbachem/config-null-value-segfaults-v3

To fetch this version to local tag pr-2297/thomasbachem/config-null-value-segfaults-v3:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2297/thomasbachem/config-null-value-segfaults-v3

gitgitgadget Bot reviewed Nov 9, 2026

View reviewed changes

thomasbachem force-pushed the config-null-value-segfaults branch from 7ac4e3a to 12f841e Compare November 10, 2026 07:30

thomasbachem added 15 commits November 10, 2026 15:44

gitgitgadget Bot reviewed Nov 10, 2026

View reviewed changes

thomasbachem force-pushed the config-null-value-segfaults branch from 12f841e to f6f3295 Compare November 11, 2026 22:43

		@@ -652,8 +652,11 @@ static int get_scalar_repos(const char key, const char value,
		{

		@@ -165,6 +165,8 @@ static int bundle_list_update(const char key, const char value,
		}

		@@ -572,7 +572,8 @@ static int fetch_bundles_by_token(struct repository *r,
		if (!repo_config_get_value(r,

		@@ -30,7 +30,7 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
		{

		@@ -164,6 +164,8 @@ static int bundle_list_update(const char key, const char value,
		}

		@@ -668,6 +668,8 @@ static int git_merge_config(const char k, const char v,
		skip_prefix(k, "branch.", &str) &&

		@@ -783,12 +782,16 @@ static int git_blame_config(const char var, const char value,
		return 0;

		@@ -556,6 +556,8 @@ static int handle_config(const char key, const char value,
		else

		@@ -30,8 +30,14 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
		{

Conversation

thomasbachem commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasbachem commented Nov 5, 2026

Uh oh!

gitgitgadget Bot commented Nov 5, 2026

Uh oh!

thomasbachem commented Nov 7, 2026

Uh oh!

gitgitgadget Bot commented Nov 7, 2026

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Nov 9, 2026

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Nov 9, 2026

Uh oh!

thomasbachem commented Nov 10, 2026

Uh oh!

gitgitgadget Bot commented Nov 10, 2026

Uh oh!

thomasbachem commented Nov 10, 2026

Uh oh!

gitgitgadget Bot commented Nov 10, 2026

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Nov 10, 2026

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Nov 10, 2026

Uh oh!

gitgitgadget Bot commented Nov 10, 2026

Uh oh!

gitgitgadget Bot commented Nov 11, 2026

Uh oh!

thomasbachem commented Nov 5, 2026 •

edited

Loading