git-remote: reject adding remotes with invalid names

This can happen if the arguments to git-remote add is switched by the
user, and git would only show an error if fetching was also requested.
Fix it by using the refspec parsing engine to check if the requested
name can be parsed as a remote before add it.

Also cleanup so that the "remote.<name>.url" config name buffer is only
initialized once.

Signed-off-by: Jonas Fonseca <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: jonas

builtin-remote.c

@@ -88,18 +88,22 @@ static int add(int argc, const char **argv)
 	strbuf_init(&buf, 0);
 	strbuf_init(&buf2, 0);
 
+	strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
+	if (!valid_fetch_refspec(buf2.buf))
+		die("'%s' is not a valid remote name", name);
+
 	strbuf_addf(&buf, "remote.%s.url", name);
 	if (git_config_set(buf.buf, url))
 		return 1;
 
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "remote.%s.fetch", name);
+
 	if (track.nr == 0)
 		path_list_append("*", &track);
 	for (i = 0; i < track.nr; i++) {
 		struct path_list_item *item = track.items + i;
 
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "remote.%s.fetch", name);
-
 		strbuf_reset(&buf2);
 		if (mirror)
 			strbuf_addf(&buf2, "refs/%s:refs/%s",

remote.c

@@ -409,7 +409,7 @@ static void read_config(void)
 	alias_all_urls();
 }
 
-static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch)
+static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
 {
 	int i;
 	int st;
@@ -519,17 +519,32 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
 	return rs;
 
  invalid:
+	if (verify) {
+		free(rs);
+		return NULL;
+	}
 	die("Invalid refspec '%s'", refspec[i]);
 }
 
+int valid_fetch_refspec(const char *fetch_refspec_str)
+{
+	const char *fetch_refspec[] = { fetch_refspec_str };
+	struct refspec *refspec;
+
+	refspec = parse_refspec_internal(1, fetch_refspec, 1, 1);
+	if (refspec)
+		free(refspec);
+	return !!refspec;
+}
+
 struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec)
 {
-	return parse_refspec_internal(nr_refspec, refspec, 1);
+	return parse_refspec_internal(nr_refspec, refspec, 1, 0);
 }
 
 struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
 {
-	return parse_refspec_internal(nr_refspec, refspec, 0);
+	return parse_refspec_internal(nr_refspec, refspec, 0, 0);
 }
 
 static int valid_remote_nick(const char *name)

remote.h

@@ -67,6 +67,7 @@ void free_refs(struct ref *ref);
  */
 void ref_remove_duplicates(struct ref *ref_map);
 
+int valid_fetch_refspec(const char *refspec);
 struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
 struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);
 

t/t5505-remote.sh

@@ -253,4 +253,10 @@ test_expect_success '"remote show" does not show symbolic refs' '
 
 '
 
+test_expect_success 'reject adding remote with an invalid name' '
+
+	! git remote add some:url desired-name
+
+'
+
 test_done