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.

3 files changed, 14 insertions, 2 deletions

diff --git a/include/git2/remote.h b/include/git2/remote.h
index 86498a31a..577a1c853 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 d58927f28..99b5bacc0 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 8619f2e00..2fa21d460 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)