undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

remote.c: don't BUG() on 0-length branch names

Browse Source 
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." <perry@ntis.zcu.cz>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Glen Choo 2022-05-31 23:12:33 +00:00 committed by Junio C Hamano
parent 4a2dcb1a08
commit 91e2e8f63e
2 changed files with 12 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
remote.c
View File

@ -195,9 +195,6 @@ static struct branch *find_branch(struct remote_state *remote_state,
struct branches_hash_key lookup;
struct hashmap_entry lookup_entry, *e;
if (!len)
len = strlen(name);
lookup.str = name;
lookup.len = len;
hashmap_entry_init(&lookup_entry, memhash(name, len));
@ -214,7 +211,8 @@ static void die_on_missing_branch(struct repository *repo,
{
/* branch == NULL is always valid because it represents detached HEAD. */
if (branch &&
branch != find_branch(repo->remote_state, branch->name, 0))
branch != find_branch(repo->remote_state, branch->name,
strlen(branch->name)))
die("branch %s was not found in the repository", branch->name);
}

10
t/t5516-fetch-push.sh
View File

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