remote: check branch names

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
modifying any of the existing refspecs 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 the existing refspecs are modified and (iii)
wildcards are accepted.

Signed-off-by: Phillip Wood <[email protected]>

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);

t/t5505-remote.sh

@@ -1195,6 +1195,31 @@ 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 "topic/*" -t in:valid \
+				 https://git.example.com/repo 2>err &&
+	echo "error: invalid branch name ${SQ}in:valid${SQ}" >expect &&
+	test_cmp expect err &&
+	test_must_fail 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 &&