diff --git a/builtin/remote.c b/builtin/remote.c index d1f9292ed2b292..fd84bfbfe7ab03 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"); @@ -158,7 +171,7 @@ static int add(int argc, const char **argv, const char *prefix) { int fetch = 0, fetch_tags = TAGS_DEFAULT; unsigned mirror = MIRROR_NONE; - struct string_list track = STRING_LIST_INIT_NODUP; + struct strvec track = STRVEC_INIT; const char *master = NULL; struct remote *remote; struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; @@ -171,8 +184,8 @@ static int add(int argc, const char **argv, const char *prefix) N_("import all tags and associated objects when fetching\n" "or do not fetch any tag at all (--no-tags)"), TAGS_SET), - OPT_STRING_LIST('t', "track", &track, N_("branch"), - N_("branch(es) to track")), + OPT_STRVEC('t', "track", &track, N_("branch"), + N_("branch(es) to track")), OPT_STRING('m', "master", &master, N_("branch"), N_("master branch")), OPT_CALLBACK_F(0, "mirror", &mirror, "(push|fetch)", N_("set up remote as a mirror to push to or fetch from"), @@ -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); @@ -210,10 +226,9 @@ static int add(int argc, const char **argv, const char *prefix) strbuf_reset(&buf); strbuf_addf(&buf, "remote.%s.fetch", name); if (track.nr == 0) - string_list_append(&track, "*"); + strvec_push(&track, "*"); for (i = 0; i < track.nr; i++) { - add_branch(buf.buf, track.items[i].string, - name, mirror, &buf2); + add_branch(buf.buf, track.v[i], name, mirror, &buf2); } } @@ -246,7 +261,7 @@ static int add(int argc, const char **argv, const char *prefix) strbuf_release(&buf); strbuf_release(&buf2); - string_list_clear(&track, 0); + strvec_clear(&track); return 0; } @@ -1567,8 +1582,12 @@ static int update(int argc, const char **argv, const char *prefix) static int remove_all_fetch_refspecs(const char *key) { - return git_config_set_multivar_gently(key, NULL, NULL, - CONFIG_FLAGS_MULTI_REPLACE); + int res = git_config_set_multivar_gently(key, NULL, NULL, + CONFIG_FLAGS_MULTI_REPLACE); + if (res == CONFIG_NOTHING_SET) + res = 0; + + return res; } static void add_branches(struct remote *remote, const char **branches, @@ -1598,7 +1617,11 @@ 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); return 1; } diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 08424e878e104c..709cbe65924467 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -1131,7 +1131,9 @@ test_expect_success 'remote set-branches' ' +refs/heads/next:refs/remotes/scratch/next +refs/heads/seen:refs/remotes/scratch/seen EOF - + cat <<-\EOF >expect.replace-missing && + +refs/heads/topic:refs/remotes/scratch/topic + EOF git clone .git/ setbranches && ( cd setbranches && @@ -1161,14 +1163,20 @@ test_expect_success 'remote set-branches' ' git remote set-branches --add scratch seen && git config --get-all remote.scratch.fetch >config-result && - sort ../actual.respect-ffonly + sort ../actual.respect-ffonly && + + git config --unset-all remote.scratch.fetch && + git remote set-branches scratch topic && + git config --get-all remote.scratch.fetch \ + >../actual.replace-missing ) && test_cmp expect.initial actual.initial && test_cmp expect.add actual.add && test_cmp expect.replace actual.replace && test_cmp expect.add-two actual.add-two && test_cmp expect.setup-ffonly actual.setup-ffonly && - test_cmp expect.respect-ffonly actual.respect-ffonly + test_cmp expect.respect-ffonly actual.respect-ffonly && + test_cmp expect.replace-missing actual.replace-missing ' test_expect_success 'remote set-branches with --mirror' ' @@ -1187,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 &&