Skip to content

Commit 91e2e8f

Browse files
chooglengitster
authored andcommitted
remote.c: don't BUG() on 0-length branch names
4a2dcb1 (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 4a2dcb1, 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." <[email protected]> Helped-by: Jeff King <[email protected]> Signed-off-by: Glen Choo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4a2dcb1 commit 91e2e8f

File tree

2 files changed

+12
-4
lines changed

2 files changed

+12
-4
lines changed

remote.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,6 @@ static struct branch *find_branch(struct remote_state *remote_state,
195195
struct branches_hash_key lookup;
196196
struct hashmap_entry lookup_entry, *e;
197197

198-
if (!len)
199-
len = strlen(name);
200-
201198
lookup.str = name;
202199
lookup.len = len;
203200
hashmap_entry_init(&lookup_entry, memhash(name, len));
@@ -214,7 +211,8 @@ static void die_on_missing_branch(struct repository *repo,
214211
{
215212
/* branch == NULL is always valid because it represents detached HEAD. */
216213
if (branch &&
217-
branch != find_branch(repo->remote_state, branch->name, 0))
214+
branch != find_branch(repo->remote_state, branch->name,
215+
strlen(branch->name)))
218216
die("branch %s was not found in the repository", branch->name);
219217
}
220218

t/t5516-fetch-push.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,16 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' '
602602
check_push_result two_repo $the_commit heads/main
603603
'
604604

605+
test_expect_success 'push ignores empty branch name entries' '
606+
mk_test one_repo heads/main &&
607+
test_config remote.one.url one_repo &&
608+
test_config branch..remote one &&
609+
test_config branch..merge refs/heads/ &&
610+
test_config branch.main.remote one &&
611+
test_config branch.main.merge refs/heads/main &&
612+
git push
613+
'
614+
605615
test_expect_success 'push with dry-run' '
606616
607617
mk_test testrepo heads/main &&

0 commit comments

Comments
 (0)