diff mbox series

[v2,08/14] builtin/config.c: test misuse of format_config()

Message ID 74f43fc727ebb07874b902b0ec64dd0954ffff30.1685472134.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series config: remove global state from config iteration | expand

Commit Message

Glen Choo May 30, 2023, 6:42 p.m. UTC
From: Glen Choo <chooglen@google.com>

current_config_*() functions aren't meant to be called outside of
config callbacks because they read state that is only set when iterating
through config. However, several sites in builtin/config.c are
indirectly calling these functions outside of config callbacks thanks to
the format_config() helper. Show the current, bad behavior via tests
so that the fixes in a subsequent commit will be clearer.

The misbehaving cases are:

* "git config --get-urlmatch --show-scope" results in an "unknown"
   scope, where it arguably should show the config file's scope. It's
   clear that this wasn't intended, though: we knew that
   "--get-urlmatch" couldn't show config source metadata, which is why
   "--show-origin" was marked incompatible with "--get-urlmatch" when
   it was introduced [1]. It was most likely a mistake that we allowed
   "--show-scope" to sneak through.

* Similarly, "git config --default" doesn't set config source metadata ,
  so "--show-scope" also results in "unknown", and "--show-origin"
  results in a BUG().

[1] https://lore.kernel.org/git/20160205112001.GA13397@sigill.intra.peff.net/

Signed-off-by: Glen Choo <chooglen@google.com>
---
 t/t1300-config.sh | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
diff mbox series

Patch

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 2575279ab84..57fe250b78f 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1627,6 +1627,21 @@  test_expect_success 'urlmatch' '
 	test_cmp expect actual
 '
 
+test_expect_success 'urlmatch with --show-scope' '
+	cat >.git/config <<-\EOF &&
+	[http "https://weak.example.com"]
+		sslVerify = false
+		cookieFile = /tmp/cookie.txt
+	EOF
+
+	cat >expect <<-EOF &&
+	unknown	http.cookiefile /tmp/cookie.txt
+	unknown	http.sslverify false
+	EOF
+	git config --get-urlmatch --show-scope HTTP https://weak.example.com >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'urlmatch favors more specific URLs' '
 	cat >.git/config <<-\EOF &&
 	[http "https://example.com/"]
@@ -2014,6 +2029,10 @@  test_expect_success '--show-origin blob ref' '
 	test_cmp expect output
 '
 
+test_expect_success '--show-origin with --default' '
+	test_must_fail git config --show-origin --default foo some.key
+'
+
 test_expect_success '--show-scope with --list' '
 	cat >expect <<-EOF &&
 	global	user.global=true
@@ -2082,6 +2101,12 @@  test_expect_success '--show-scope with --show-origin' '
 	test_cmp expect output
 '
 
+test_expect_success '--show-scope with --default' '
+	git config --show-scope --default foo some.key >actual &&
+	echo "unknown	foo" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'override global and system config' '
 	test_when_finished rm -f \"\$HOME\"/.gitconfig &&
 	cat >"$HOME"/.gitconfig <<-EOF &&