Fix segfaults on settings given without a value

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"

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. Three accept it without an error: 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 based on master and merges cleanly there. In 'next' and 'seen', t5618 conflicts with iw/connected-deepen-fetch-no-alternate-refs, and in 'seen' also with kk/quarantine-alternate-refs, as each adds tests at the end of the file. Keeping both sides resolves it.

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            |  2 +-
 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   |  7 +++++++
 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, 183 insertions(+), 10 deletions(-)

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2297%2Fthomasbachem%2Fconfig-null-value-segfaults-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2297/thomasbachem/config-null-value-segfaults-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2297

gitgitgadget

From: Thomas Bachem <[email protected]>

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.

---
 rebase-interactive.c          | 2 +-
 t/t3404-rebase-interactive.sh | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

index 809f76a87b..4eb628ee8f 100644
@@ -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))

index ff11abb2f2..813ad2a83d 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 odb.c                     | 4 ++--
 t/t5618-alternate-refs.sh | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

index 0200e26f21..ed59cfc989 100644
@@ -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);
 		}

index 2fb6d549d3..13abd81e98 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 add-patch.c                |  2 +-
 t/t3701-add-interactive.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

index f27edcbe8d..ad581eb4ae 100644
@@ -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);
 

index 6e120a4001..33050ccc53 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 bundle-uri.c                |  3 ++-
 t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

index 2bb2eb99e4..7de17634cf 100644
@@ -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);

index f81f1f8db7..e86ed7d30c 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 grep.c          | 2 ++
 t/t7810-grep.sh | 5 +++++
 2 files changed, 7 insertions(+)

index 1d75d31421..ab082f963e 100644
@@ -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;
 	}

index ac7ea9cf8d..d323927e54 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 remote.c          | 2 ++
 t/t5505-remote.sh | 5 +++++
 2 files changed, 7 insertions(+)

index 71170f36a9..024cfc547d 100644
@@ -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"))

index 5fbfcb0848..bec2e51afa 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/blame.c         | 4 ++++
 t/t8012-blame-colors.sh | 7 +++++++
 2 files changed, 11 insertions(+)

index 48d5251c6d..653449c0c9 100644
@@ -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;
 	}

index d2653b3c31..5eeb7338a3 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/merge.c  | 2 ++
 t/t7600-merge.sh | 6 ++++++
 2 files changed, 8 insertions(+)

index 5b4eb23a83..fbabda97f8 100644
@@ -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;

index 64fe21717d..d4963f1fee 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 remote.c                 | 5 ++++-
 t/t5506-remote-groups.sh | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

index 024cfc547d..4bd23ddac9 100644
@@ -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");

index 16e9a1bc2f..f11eb8511e 100755
@@ -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

From: Thomas Bachem <[email protected]>

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".

---
 setup.c                         | 2 ++
 t/t0035-safe-bare-repository.sh | 6 ++++++
 2 files changed, 8 insertions(+)

index 29474fc292..567a44bcb7 100644
@@ -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;

index 1d3d19f5b4..a0e0c48d7d 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 pseudo-merge.c                  | 4 ++++
 t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
 2 files changed, 9 insertions(+)

index 22b8600d68..19fae9233a 100644
@@ -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);

index 5b2a17f90a..e7e6b2f8a8 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 scalar.c          | 5 ++++-
 t/t9210-scalar.sh | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

index a80d8ee3ff..4fe4b2a647 100644
@@ -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;
 }

index c8463d0ac4..137765d3b8 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 config.c          |  3 ++-
 t/t1300-config.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

index e0bb29b53d..f6ccfd52dd 100644
@@ -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;
 }

index e3f8064889..8fb920b1ba 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 bundle-uri.c                |  9 +++++++++
 t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)

index 7de17634cf..8c8d17bd2c 100644
@@ -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);

index 70ae94c4d0..a91c1b428d 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/config.c  |  4 ++--
 t/t1300-config.sh | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

index 2554322317..f786c0b123 100644
@@ -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;

index 8fb920b1ba..3322b0bf4f 100755
@@ -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

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

I agree that this is the correct fallback.

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

Hi Thomas

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

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.

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

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 <[email protected]>
          Assisted-by: Claude Fable 5.1
     +    Assisted-by: Claude Opus 5.5
          Signed-off-by: Thomas Bachem <[email protected]>
      
       ## 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

From: Thomas Bachem <[email protected]>

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.

---
 rebase-interactive.c          | 10 ++++++++--
 t/t3404-rebase-interactive.sh | 12 ++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

index 809f76a87b..9eb70b09b3 100644
@@ -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;

index b3ee96f560..5b9fc67c19 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 odb.c                     | 4 ++--
 t/t5618-alternate-refs.sh | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

index 865c404910..ea086be8b3 100644
@@ -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);
 		}

index 68139a1b3e..422da6dde7 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 add-patch.c                |  2 +-
 t/t3701-add-interactive.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

index f27edcbe8d..ad581eb4ae 100644
@@ -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);
 

index 6e120a4001..33050ccc53 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 bundle-uri.c                |  3 ++-
 t/t5558-clone-bundle-uri.sh | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

index 9797eaed9a..67a717e857 100644
@@ -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);

index f81f1f8db7..e86ed7d30c 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 grep.c          | 2 ++
 t/t7810-grep.sh | 5 +++++
 2 files changed, 7 insertions(+)

index 1d75d31421..ab082f963e 100644
@@ -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;
 	}

index ac7ea9cf8d..d323927e54 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 remote.c          | 2 ++
 t/t5505-remote.sh | 5 +++++
 2 files changed, 7 insertions(+)

index 49ee2ea318..0db29d6dda 100644
@@ -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"))

index 5fbfcb0848..bec2e51afa 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/blame.c         | 4 ++++
 t/t8012-blame-colors.sh | 7 +++++++
 2 files changed, 11 insertions(+)

index 1e753a0b60..15cacd1002 100644
@@ -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;
 	}

index d2653b3c31..5eeb7338a3 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/merge.c  | 2 ++
 t/t7600-merge.sh | 6 ++++++
 2 files changed, 8 insertions(+)

index d7bf209114..c6f9356ccb 100644
@@ -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;

index 8f6109fb91..2da12d6662 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 remote.c                 | 5 ++++-
 t/t5506-remote-groups.sh | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

index 0db29d6dda..fd7a31cdcf 100644
@@ -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");

index 16e9a1bc2f..f11eb8511e 100755
@@ -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

From: Thomas Bachem <[email protected]>

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".

---
 setup.c                         | 2 ++
 t/t0035-safe-bare-repository.sh | 6 ++++++
 2 files changed, 8 insertions(+)

index 186815c780..e9a1bdbb50 100644
@@ -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;

index 1d3d19f5b4..a0e0c48d7d 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 pseudo-merge.c                  | 4 ++++
 t/t5333-pseudo-merge-bitmaps.sh | 5 +++++
 2 files changed, 9 insertions(+)

index 22b8600d68..19fae9233a 100644
@@ -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);

index 5b2a17f90a..e7e6b2f8a8 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 scalar.c          | 5 ++++-
 t/t9210-scalar.sh | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

index a80d8ee3ff..4fe4b2a647 100644
@@ -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;
 }

index c8463d0ac4..137765d3b8 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 config.c          |  3 ++-
 t/t1300-config.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

index 5976a418d5..5b823b3dcd 100644
@@ -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;
 }

index e3f8064889..8fb920b1ba 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 bundle-uri.c                |  9 +++++++++
 t/t5750-bundle-uri-parse.sh | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)

index 67a717e857..41c817eefb 100644
@@ -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);

index 70ae94c4d0..a91c1b428d 100755
@@ -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

From: Thomas Bachem <[email protected]>

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.

---
 builtin/config.c  |  4 ++--
 t/t1300-config.sh | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

index 2554322317..f786c0b123 100644
@@ -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;

index 8fb920b1ba..3322b0bf4f 100755
@@ -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

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.

                                     !strcmp(value, heuristics[i].name)) {
                                         list->heuristic = 
heuristics[i].heuristic;
                                         return 0;
                                 }
                         }

                         /* Ignore unknown heuristics. */
                         return 0;

		return 0;
	}

Hi Thomas

Hi Thomas

Hi Thomas

                         remote->fetch_tags = 2;
         } else if (!strcmp(subkey, "proxy")) {

Hi Thomas

Sounds good

	FAKE_LINES=1 \
		git -c rebase.missingCommitCheck rebase -i HEAD~3

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

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

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?

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