{"thread":{"id":"13087","subject":"[PATCH] git-remote: reject adding remotes with invalid names","startedAt":"2008-04-13T09:56:54Z","lastAt":"2008-04-13T09:56:54Z","messageCount":1,"participants":["Jonas Fonseca"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74250","messageId":"20080413095654.GA26713@diku.dk","threadId":"13087","inReplyTo":null,"subject":"[PATCH] git-remote: reject adding remotes with invalid names","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2008-04-13T09:56:54Z","receivedAt":"2008-04-13T09:56:54Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"This can happen if the arguments to git-remote add is switched by the\nuser, and git would only show an error if fetching was also requested.\nFix it by using the refspec parsing engine to check if the requested\nname can be parsed as a remote before add it.\n\nAlso cleanup so that the \"remote.<name>.url\" config name buffer is only\ninitialized once.\n\nSigned-off-by: Jonas Fonseca <fonseca@diku.dk>\n---\n\n This happened for me the other day, and with the recent discussion\n about the usability of remote it might be an acceptible improvement.\n\n builtin-remote.c  |   10 +++++++---\n remote.c          |   21 ++++++++++++++++++---\n remote.h          |    1 +\n t/t5505-remote.sh |    6 ++++++\n 4 files changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 06d33e5..a3ee1ac 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -90,18 +90,22 @@ static int add(int argc, const char **argv)\n \tstrbuf_init(&buf, 0);\n \tstrbuf_init(&buf2, 0);\n \n+\tstrbuf_addf(&buf2, \"refs/heads/test:refs/remotes/%s/test\", name);\n+\tif (!valid_fetch_refspec(buf2.buf))\n+\t\tdie(\"'%s' is not a valid remote name\", name);\n+\n \tstrbuf_addf(&buf, \"remote.%s.url\", name);\n \tif (git_config_set(buf.buf, url))\n \t\treturn 1;\n \n+\tstrbuf_reset(&buf);\n+\tstrbuf_addf(&buf, \"remote.%s.fetch\", name);\n+\n \tif (track.nr == 0)\n \t\tpath_list_append(\"*\", &track);\n \tfor (i = 0; i < track.nr; i++) {\n \t\tstruct path_list_item *item = track.items + i;\n \n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"remote.%s.fetch\", name);\n-\n \t\tstrbuf_reset(&buf2);\n \t\tif (mirror)\n \t\t\tstrbuf_addf(&buf2, \"refs/%s:refs/%s\",\ndiff --git a/remote.c b/remote.c\nindex 369dc33..06ad156 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -409,7 +409,7 @@ static void read_config(void)\n \talias_all_urls();\n }\n \n-static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch)\n+static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)\n {\n \tint i;\n \tint st;\n@@ -519,17 +519,32 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \treturn rs;\n \n  invalid:\n+\tif (verify) {\n+\t\tfree(rs);\n+\t\treturn NULL;\n+\t}\n \tdie(\"Invalid refspec '%s'\", refspec[i]);\n }\n \n+int valid_fetch_refspec(const char *fetch_refspec_str)\n+{\n+\tconst char *fetch_refspec[] = { fetch_refspec_str };\n+\tstruct refspec *refspec;\n+\n+\trefspec = parse_refspec_internal(1, fetch_refspec, 1, 1);\n+\tif (refspec)\n+\t\tfree(refspec);\n+\treturn !!refspec;\n+}\n+\n struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec)\n {\n-\treturn parse_refspec_internal(nr_refspec, refspec, 1);\n+\treturn parse_refspec_internal(nr_refspec, refspec, 1, 0);\n }\n \n struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)\n {\n-\treturn parse_refspec_internal(nr_refspec, refspec, 0);\n+\treturn parse_refspec_internal(nr_refspec, refspec, 0, 0);\n }\n \n static int valid_remote_nick(const char *name)\ndiff --git a/remote.h b/remote.h\nindex 7e9ae79..a38774b 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -67,6 +67,7 @@ void free_refs(struct ref *ref);\n  */\n void ref_remove_duplicates(struct ref *ref_map);\n \n+int valid_fetch_refspec(const char *refspec);\n struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);\n struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);\n \ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 0a7fea8..af2d077 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -253,4 +253,10 @@ test_expect_success '\"remote show\" does not show symbolic refs' '\n \n '\n \n+test_expect_success 'reject adding remote with an invalid name' '\n+\n+\t! git remote add some:url desired-name\n+\n+'\n+\n test_done\n-- \n1.5.5.104.ge4331.dirty\n\n-- \nJonas Fonseca\n"}]}