git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/4] remote: check branch names

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 11, 2024, 15:18 UTC
Message-ID
<dba31245607f85c48947da60fe0955a6ed3e2c43.1726067917.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1789.git.1726067917.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Make sure the names passed to "git remote add -t <branch>" and "git remote set-branches <branch>" are syntactically valid so that we do not create invalid refspecs. This check needs to be performed before creating the remote or modifying the existing configuration so a new function is added rather than modifying add_branch()

Tests are added for both commands that to ensure (i) we report all the invalid branch names passed on the command line, (ii) the branch names are validated before modifying the configuration and (iii) wildcards are accepted.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/remote.c  | 19 +++++++++++++++++++
 t/t5505-remote.sh | 28 ++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
diff --git a/builtin/remote.c b/builtin/remote.c
index 318701496ed..fd84bfbfe7a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -132,6 +132,19 @@ static void add_branch(const char *key, const char *branchname,
 	git_config_set_multivar(key, tmp->buf, "^$", 0);
 }
 
+static int check_branch_names(const char **branches)
+{
+	int ret = 0;
+
+	for (const char **b = branches; *b; b++) {
+		if (check_refname_format(*b, REFNAME_ALLOW_ONELEVEL |
+						REFNAME_REFSPEC_PATTERN))
+			ret = error(_("invalid branch name '%s'"), *b);
+	}
+
+	return ret;
+}
+
 static const char mirror_advice[] =
 N_("--mirror is dangerous and deprecated; please\n"
    "\t use --mirror=fetch or --mirror=push instead");
@@ -203,6 +216,9 @@ static int add(int argc, const char **argv, const char *prefix)
 	if (!valid_remote_name(name))
 		die(_("'%s' is not a valid remote name"), name);
 
+	if (check_branch_names(track.v))
+		exit(128);
+
 	strbuf_addf(&buf, "remote.%s.url", name);
 	git_config_set(buf.buf, url);
 
@@ -1601,6 +1617,9 @@ static int set_remote_branches(const char *remotename, const char **branches,
 		exit(2);
 	}
 
+	if (check_branch_names(branches))
+		exit(128);
+
 	if (!add_mode && remove_all_fetch_refspecs(key.buf)) {
 		error(_("could not remove existing fetch refspec"));
 		strbuf_release(&key);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index cfbd6139e00..709cbe65924 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1195,6 +1195,34 @@ test_expect_success 'remote set-branches with --mirror' '
 	test_cmp expect.replace actual.replace
 '
 
+test_expect_success 'remote set-branches rejects invalid branch name' '
+	git remote add test https://git.example.com/repo &&
+	test_when_finished "git config --unset-all remote.test.fetch; \
+			    git config --unset remote.test.url" &&
+	test_must_fail git remote set-branches test "topic/*" in..valid \
+				feature "b a d" 2>err &&
+	cat >expect <<-EOF &&
+	error: invalid branch name ${SQ}in..valid${SQ}
+	error: invalid branch name ${SQ}b a d${SQ}
+	EOF
+	test_cmp expect err &&
+	git config --get-all remote.test.fetch >actual &&
+	echo "+refs/heads/*:refs/remotes/test/*" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'remote add -t rejects invalid branch name' '
+	test_must_fail git remote add test -t .bad -t "topic/*" -t in:valid \
+				 https://git.example.com/repo 2>err &&
+	cat >expect <<-EOF &&
+	error: invalid branch name ${SQ}.bad${SQ}
+	error: invalid branch name ${SQ}in:valid${SQ}
+	EOF
+	test_cmp expect err &&
+	test_expect_code 1 git config --get-regexp ^remote\\.test\\. >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'new remote' '
 	git remote add someremote foo &&
 	echo foo >expect &&
-- 
gitgitgadget
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 15 of 22 in “remote: branch setting fixes”
  1. 0/4 remote: branch setting fixesPhillip Wood via GitGitGadget, Sep 11, 2024
  2. 1/4 remote: fix set-branches when no branches are setPhillip Wood via GitGitGadget, Sep 11, 2024
  3. Junio C HamanoSep 11, 2024
  4. Patrick SteinhardtSep 12, 2024
  5. 2/4 remote: print an error if refspec cannot be removedPhillip Wood via GitGitGadget, Sep 11, 2024
  6. Junio C HamanoSep 11, 2024
  7. Patrick SteinhardtSep 12, 2024
  8. Junio C HamanoSep 12, 2024
  9. Patrick SteinhardtSep 13, 2024
  10. phillip.wood123@gmail.comSep 13, 2024
  11. Junio C HamanoSep 13, 2024
  12. 3/4 remote add: use strvec to store tracking branchesPhillip Wood via GitGitGadget, Sep 11, 2024
  13. Junio C HamanoSep 11, 2024
  14. Patrick SteinhardtSep 12, 2024
  15. 4/4 remote: check branch namesPhillip Wood via GitGitGadget, Sep 11, 2024
  16. Junio C HamanoSep 11, 2024
  17. Patrick SteinhardtSep 12, 2024
  18. Junio C HamanoSep 12, 2024
  19. phillip.wood123@gmail.comSep 13, 2024
  20. Junio C HamanoSep 13, 2024
  21. phillip.wood123@gmail.comSep 18, 2024
  22. Junio C HamanoSep 18, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.