Commit c6e942fb3d10d9f8f2e22833ddddbd945c0d6604

Carlos Martín Nieto 2015-05-17T15:19:22

remote: validate refspecs before adding to config When we moved from acting on the instance to acting on the configuration, we dropped the validation of the passed refspec, which can lead to writing an invalid refspec to the configuration. Bring that validation back.

diff --git a/include/git2/remote.h b/include/git2/remote.h
index 86498a3..577a1c8 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -168,7 +168,7 @@ GIT_EXTERN(int) git_remote_set_pushurl(git_repository *repo, const char *remote,
  * @param repo the repository in which to change the configuration
  * @param remote the name of the remote to change
  * @param refspec the new fetch refspec
- * @return 0 or an error value
+ * @return 0, GIT_EINVALIDSPEC if refspec is invalid or an error value
  */
 GIT_EXTERN(int) git_remote_add_fetch(git_repository *repo, const char *remote, const char *refspec);
 
@@ -192,7 +192,7 @@ GIT_EXTERN(int) git_remote_get_fetch_refspecs(git_strarray *array, const git_rem
  * @param repo the repository in which to change the configuration
  * @param remote the name of the remote to change
  * @param refspec the new push refspec
- * @return 0 or an error value
+ * @return 0, GIT_EINVALIDSPEC if refspec is invalid or an error value
  */
 GIT_EXTERN(int) git_remote_add_push(git_repository *repo, const char *remote, const char *refspec);
 
diff --git a/src/remote.c b/src/remote.c
index d58927f..99b5bac 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -97,6 +97,7 @@ static int write_add_refspec(git_repository *repo, const char *name, const char 
 {
 	git_config *cfg;
 	git_buf var = GIT_BUF_INIT;
+	git_refspec spec;
 	const char *fmt;
 	int error;
 
@@ -108,6 +109,15 @@ static int write_add_refspec(git_repository *repo, const char *name, const char 
 	if ((error = ensure_remote_name_is_valid(name)) < 0)
 		return error;
 
+	if ((error = git_refspec__parse(&spec, refspec, fetch)) < 0) {
+		if (giterr_last()->klass != GITERR_NOMEMORY)
+			error = GIT_EINVALIDSPEC;
+
+		return error;
+	}
+
+	git_refspec__free(&spec);
+
 	if ((error = git_buf_printf(&var, fmt, name)) < 0)
 		return error;
 
diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c
index 8619f2e..2fa21d4 100644
--- a/tests/network/remote/remotes.c
+++ b/tests/network/remote/remotes.c
@@ -128,6 +128,8 @@ void test_network_remote_remotes__add_fetchspec(void)
 	cl_assert_equal_s(git_refspec_dst(_refspec), "refs/*");
 	cl_assert_equal_s(git_refspec_string(_refspec), "refs/*:refs/*");
 	cl_assert_equal_b(_refspec->push, false);
+
+	cl_git_fail_with(GIT_EINVALIDSPEC, git_remote_add_fetch(_repo, "test", "refs/*/foo/*:refs/*"));
 }
 
 void test_network_remote_remotes__dup(void)