remote: Introduce git_remote_is_valid_name()

Fix libgit2/libgit2sharp#318

include/git2/remote.h

@@ -450,6 +450,14 @@ GIT_EXTERN(int) git_remote_update_fetchhead(git_remote *remote);
  */
 GIT_EXTERN(void) git_remote_set_update_fetchhead(git_remote *remote, int value);
 
+/**
+ * Ensure the remote name is well-formed.
+ *
+ * @param remote_name name to be checked.
+ * @return 1 if the reference name is acceptable; 0 if it isn't
+ */
+GIT_EXTERN(int) git_remote_is_valid_name(const char *remote_name);
+
 /** @} */
 GIT_END_DECL
 #endif

src/refs.c

@@ -1622,7 +1622,7 @@ static int ensure_segment_validity(const char *name)
 
 	/* A refname component can not end with ".lock" */
 	if (current - name >= lock_len &&
-		!git__strncmp(current - lock_len, GIT_FILELOCK_EXTENSION, lock_len))
+		!memcmp(current - lock_len, GIT_FILELOCK_EXTENSION, lock_len))
 			return -1;
 
 	return (int)(current - name);
@@ -1697,11 +1697,10 @@ int git_reference__normalize_name(
 			segments_count++;
 		}
 
-		/* This means that there's a leading slash in the refname */
-		if (segment_len == 0 && segments_count == 0) {
+		/* No empty segment is allowed when not normalizing */
+		if (segment_len == 0 && !normalize)
 			goto cleanup;
-		}
-
+		
 		if (current[segment_len] == '\0')
 			break;
 

src/remote.c

@@ -59,21 +59,9 @@ static int download_tags_value(git_remote *remote, git_config *cfg)
 
 static int ensure_remote_name_is_valid(const char *name)
 {
-	git_buf buf = GIT_BUF_INIT;
-	git_refspec refspec;
-	int error = -1;
-
-	if (!name || *name == '\0')
-		goto cleanup;
-
-	git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", name);
-	error = git_refspec__parse(&refspec, git_buf_cstr(&buf), true);
-
-	git_buf_free(&buf);
-	git_refspec__free(&refspec);
+	int error = 0;
 
-cleanup:
-	if (error) {
+	if (!git_remote_is_valid_name(name)) {
 		giterr_set(
 			GITERR_CONFIG,
 			"'%s' is not a valid remote name.", name);
@@ -1380,3 +1368,23 @@ void git_remote_set_update_fetchhead(git_remote *remote, int value)
 {
 	remote->update_fetchhead = value;
 }
+
+int git_remote_is_valid_name(
+	const char *remote_name)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_refspec refspec;
+	int error = -1;
+
+	if (!remote_name || *remote_name == '\0')
+		return 0;
+
+	git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", remote_name);
+	error = git_refspec__parse(&refspec, git_buf_cstr(&buf), true);
+
+	git_buf_free(&buf);
+	git_refspec__free(&refspec);
+
+	giterr_clear();
+	return error == 0;
+}

tests-clar/network/remote/isvalidname.c

+#include "clar_libgit2.h"
+
+void test_network_remote_isvalidname__can_detect_invalid_formats(void)
+{
+	cl_assert_equal_i(false, git_remote_is_valid_name("/"));
+	cl_assert_equal_i(false, git_remote_is_valid_name("//"));
+	cl_assert_equal_i(false, git_remote_is_valid_name(".lock"));
+	cl_assert_equal_i(false, git_remote_is_valid_name("a.lock"));
+	cl_assert_equal_i(false, git_remote_is_valid_name("/no/leading/slash"));
+	cl_assert_equal_i(false, git_remote_is_valid_name("no/trailing/slash/"));
+}
+
+void test_network_remote_isvalidname__wont_hopefully_choke_on_valid_formats(void)
+{
+	cl_assert_equal_i(true, git_remote_is_valid_name("webmatrix"));
+	cl_assert_equal_i(true, git_remote_is_valid_name("yishaigalatzer/rules"));
+}

tests-clar/network/remote/remotes.c

@@ -361,13 +361,27 @@ void test_network_remote_remotes__check_structure_version(void)
 	cl_assert_equal_i(GITERR_INVALID, err->klass);
 }
 
-void test_network_remote_remotes__cannot_create_a_remote_which_name_conflicts_with_an_existing_remote(void)
+void assert_cannot_create_remote(const char *name, int expected_error)
 {
 	git_remote *remote = NULL;
 
-	cl_assert_equal_i(
-		GIT_EEXISTS,
-		git_remote_create(&remote, _repo, "test", "git://github.com/libgit2/libgit2"));
+	cl_git_fail_with(
+		git_remote_create(&remote, _repo, name, "git://github.com/libgit2/libgit2"),
+		expected_error);
 
 	cl_assert_equal_p(remote, NULL);
 }
+
+void test_network_remote_remotes__cannot_create_a_remote_which_name_conflicts_with_an_existing_remote(void)
+{
+	assert_cannot_create_remote("test", GIT_EEXISTS);
+}
+
+
+void test_network_remote_remotes__cannot_create_a_remote_which_name_is_invalid(void)
+{
+	assert_cannot_create_remote("/", GIT_EINVALIDSPEC);
+	assert_cannot_create_remote("//", GIT_EINVALIDSPEC);
+	assert_cannot_create_remote(".lock", GIT_EINVALIDSPEC);
+	assert_cannot_create_remote("a.lock", GIT_EINVALIDSPEC);
+}

tests-clar/refs/isvalidname.c

@@ -12,6 +12,7 @@ void test_refs_isvalidname__can_detect_invalid_formats(void)
 	cl_assert_equal_i(false, git_reference_is_valid_name("lower_case"));
 	cl_assert_equal_i(false, git_reference_is_valid_name("/stupid/name/master"));
 	cl_assert_equal_i(false, git_reference_is_valid_name("/"));
+	cl_assert_equal_i(false, git_reference_is_valid_name("//"));
 	cl_assert_equal_i(false, git_reference_is_valid_name(""));
 	cl_assert_equal_i(false, git_reference_is_valid_name("refs/heads/sub.lock/webmatrix"));
 }

tests-clar/refs/normalize.c

@@ -89,6 +89,8 @@ void test_refs_normalize__symbolic(void)
 	ensure_refname_invalid(
 		GIT_REF_FORMAT_ALLOW_ONELEVEL, "heads\foo");
 	ensure_refname_invalid(
+		GIT_REF_FORMAT_ALLOW_ONELEVEL, "/");
+	ensure_refname_invalid(
 		GIT_REF_FORMAT_ALLOW_ONELEVEL, "///");
 
 	ensure_refname_normalized(