From 91e2e8f63ebd92295ff0eb5f4095f9e1fba8bab0 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 31 May 2022 23:12:33 +0000 Subject: remote.c: don't BUG() on 0-length branch names 4a2dcb1a08 (remote: die if branch is not found in repository, 2021-11-17) introduced a regression where multiple config entries with an empty branch name, e.g. [branch ""] remote = foo merge = bar could cause Git to fail when it tries to look up branch tracking information. We parse the config key to get (branch name, branch name length), but when the branch name subsection is empty, we get a bogus branch name, e.g. "branch..remote" gives (".remote", 0). We continue to use the bogus branch name as if it were valid, and prior to 4a2dcb1a08, this wasn't an issue because length = 0 caused the branch name to effectively be "" everywhere. However, that commit handles length = 0 inconsistently when we create the branch: - When find_branch() is called to check if the branch exists in the branch hash map, it interprets a length of 0 to mean that it should call strlen on the char pointer. - But the code path that inserts into the branch hash map interprets a length of 0 to mean that the string is 0-length. This results in the bug described above: - "branch..remote" looks for ".remote" in the branch hash map. Since we do not find it, we insert the "" entry into the hash map. - "branch..merge" looks for ".merge" in the branch hash map. Since we do not find it, we again try to insert the "" entry into the hash map. However, the entries in the branch hash map are supposed to be appended to, not overwritten. - Since overwriting an entry is a BUG(), Git fails instead of silently ignoring the empty branch name. Fix the bug by removing the convenience strlen functionality, so that 0 means that the string is 0-length. We still insert a bogus branch name into the hash map, but this will be fixed in a later commit. Reported-by: "Ing. Martin Prantl Ph.D." Helped-by: Jeff King Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 't/t5516-fetch-push.sh') diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 7831a38dde..de0bab398e 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -602,6 +602,16 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' ' check_push_result two_repo $the_commit heads/main ' +test_expect_success 'push ignores empty branch name entries' ' + mk_test one_repo heads/main && + test_config remote.one.url one_repo && + test_config branch..remote one && + test_config branch..merge refs/heads/ && + test_config branch.main.remote one && + test_config branch.main.merge refs/heads/main && + git push +' + test_expect_success 'push with dry-run' ' mk_test testrepo heads/main && -- cgit v1.2.1 From f1dfbd9ee010e5cdf0d931d16b4b2892b33331e5 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 31 May 2022 23:12:34 +0000 Subject: remote.c: reject 0-length branch names Branch names can't be empty, so config keys with an empty branch name, e.g. "branch..remote", are silently ignored. Since these config keys will never be useful, make it a fatal error when remote.c finds a key that starts with "branch." and has an empty subsection. Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 't/t5516-fetch-push.sh') diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index de0bab398e..ff38563fb7 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -602,13 +602,23 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' ' check_push_result two_repo $the_commit heads/main ' -test_expect_success 'push ignores empty branch name entries' ' +test_expect_success 'push rejects empty branch name entries' ' mk_test one_repo heads/main && test_config remote.one.url one_repo && test_config branch..remote one && test_config branch..merge refs/heads/ && test_config branch.main.remote one && test_config branch.main.merge refs/heads/main && + test_must_fail git push 2>err && + grep "bad config variable .branch\.\." err +' + +test_expect_success 'push ignores "branch." config without subsection' ' + mk_test one_repo heads/main && + test_config remote.one.url one_repo && + test_config branch.autoSetupMerge true && + test_config branch.main.remote one && + test_config branch.main.merge refs/heads/main && git push ' -- cgit v1.2.1