Merge pull request #5659 from libgit2/ethomson/name_is_valid

Deprecate `is_valid_name` functions; replace with `name_is_valid` functions

20 files changed, 365 insertions, 105 deletions

diff --git a/docs/changelog.md b/docs/changelog.md
index 288ef2d11..98bde2f6c 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -1,3 +1,18 @@
+vNext
+-----
+
+### Changes or improvements
+
+* Branch and tag name validation functions have been introduced:
+  `git_branch_name_is_valid` will check if a branch name is valid,
+  and `git_tag_name_is_valid` will check if a tag name is valid.
+
+* Some remote and reference validity checking functions have been
+  introduced with error reporting semantics.  `git_remote_name_is_valid`
+  replaces `git_remote_is_valid_name`.  `git_reference_name_is_valid`
+  replaces `git_reference_is_valid_name`.  Tthe former functions are
+  deprecated.
+
 v1.1
 ----
 
diff --git a/include/git2/branch.h b/include/git2/branch.h
index ba6235900..0c0cc7ff7 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -304,6 +304,18 @@ GIT_EXTERN(int) git_branch_remote_name(
  */
  GIT_EXTERN(int) git_branch_upstream_remote(git_buf *buf, git_repository *repo, const char *refname);
 
+/**
+ * Determine whether a branch name is valid, meaning that (when prefixed
+ * with `refs/heads/`) that it is a valid reference name, and that any
+ * additional branch name restrictions are imposed (eg, it cannot start
+ * with a `-`).
+ *
+ * @param valid output pointer to set with validity of given branch name
+ * @param name a branch name to test
+ * @return 0 on success or an error code
+ */
+GIT_EXTERN(int) git_branch_name_is_valid(int *valid, const char *name);
+
 /** @} */
 GIT_END_DECL
 #endif
diff --git a/include/git2/deprecated.h b/include/git2/deprecated.h
index 3f285018d..4e43c4536 100644
--- a/include/git2/deprecated.h
+++ b/include/git2/deprecated.h
@@ -340,10 +340,32 @@ GIT_EXTERN(size_t) git_object__size(git_object_t type);
 
 /**@}*/
 
-/** @name Deprecated Reference Constants
+/** @name Deprecated Remote Functions
  *
- * These enumeration values are retained for backward compatibility.  The
- * newer versions of these values should be preferred in all new code.
+ * These functions are retained for backward compatibility.  The newer
+ * versions of these functions should be preferred in all new code.
+ *
+ * There is no plan to remove these backward compatibility functions at
+ * this time.
+ */
+/**@{*/
+
+/**
+ * Ensure the remote name is well-formed.
+ *
+ * @deprecated Use git_remote_name_is_valid
+ * @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);
+
+/**@}*/
+
+/** @name Deprecated Reference Functions and Constants
+ *
+ * These functions and enumeration values are retained for backward
+ * compatibility.  The newer versions of these values should be
+ * preferred in all new code.
  *
  * There is no plan to remove these backward compatibility values at
  * this time.
@@ -364,6 +386,23 @@ GIT_EXTERN(size_t) git_object__size(git_object_t type);
 #define GIT_REF_FORMAT_REFSPEC_PATTERN GIT_REFERENCE_FORMAT_REFSPEC_PATTERN
 #define GIT_REF_FORMAT_REFSPEC_SHORTHAND GIT_REFERENCE_FORMAT_REFSPEC_SHORTHAND
 
+/**
+ * Ensure the reference name is well-formed.
+ *
+ * Valid reference names must follow one of two patterns:
+ *
+ * 1. Top-level names must contain only capital letters and underscores,
+ *    and must begin and end with a letter. (e.g. "HEAD", "ORIG_HEAD").
+ * 2. Names prefixed with "refs/" can be almost anything.  You must avoid
+ *    the characters '~', '^', ':', '\\', '?', '[', and '*', and the
+ *    sequences ".." and "@{" which have special meaning to revparse.
+ *
+ * @deprecated Use git_reference_name_is_valid
+ * @param refname name to be checked.
+ * @return 1 if the reference name is acceptable; 0 if it isn't
+ */
+GIT_EXTERN(int) git_reference_is_valid_name(const char *refname);
+
 GIT_EXTERN(int) git_tag_create_frombuffer(
 	git_oid *oid,
 	git_repository *repo,
diff --git a/include/git2/refs.h b/include/git2/refs.h
index c9cce2212..a20a1265b 100644
--- a/include/git2/refs.h
+++ b/include/git2/refs.h
@@ -743,10 +743,11 @@ GIT_EXTERN(int) git_reference_peel(
  *    the characters '~', '^', ':', '\\', '?', '[', and '*', and the
  *    sequences ".." and "@{" which have special meaning to revparse.
  *
+ * @param valid output pointer to set with validity of given reference name
  * @param refname name to be checked.
- * @return 1 if the reference name is acceptable; 0 if it isn't
+ * @return 0 on success or an error code
  */
-GIT_EXTERN(int) git_reference_is_valid_name(const char *refname);
+GIT_EXTERN(int) git_reference_name_is_valid(int *valid, const char *refname);
 
 /**
  * Get the reference's short name
diff --git a/include/git2/remote.h b/include/git2/remote.h
index 51d9c7235..b82bd250b 100644
--- a/include/git2/remote.h
+++ b/include/git2/remote.h
@@ -915,10 +915,11 @@ GIT_EXTERN(int) git_remote_rename(
 /**
  * Ensure the remote name is well-formed.
  *
+ * @param valid output pointer to set with validity of given remote name
  * @param remote_name name to be checked.
- * @return 1 if the reference name is acceptable; 0 if it isn't
+ * @return 0 on success or an error code
  */
-GIT_EXTERN(int) git_remote_is_valid_name(const char *remote_name);
+int git_remote_name_is_valid(int *valid, const char *remote_name);
 
 /**
 * Delete an existing persisted remote.
diff --git a/include/git2/tag.h b/include/git2/tag.h
index 4e5fe1db1..a3921369d 100644
--- a/include/git2/tag.h
+++ b/include/git2/tag.h
@@ -365,6 +365,18 @@ GIT_EXTERN(int) git_tag_peel(
  */
 GIT_EXTERN(int) git_tag_dup(git_tag **out, git_tag *source);
 
+/**
+ * Determine whether a tag name is valid, meaning that (when prefixed
+ * with `refs/tags/`) that it is a valid reference name, and that any
+ * additional tag name restrictions are imposed (eg, it cannot start
+ * with a `-`).
+ *
+ * @param valid output pointer to set with validity of given tag name
+ * @param name a tag name to test
+ * @return 0 on success or an error code
+ */
+GIT_EXTERN(int) git_tag_name_is_valid(int *valid, const char *name);
+
 /** @} */
 GIT_END_DECL
 #endif
diff --git a/src/branch.c b/src/branch.c
index 715f6cf99..000a63ad8 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -723,3 +723,32 @@ int git_branch_is_head(
 
 	return is_same;
 }
+
+int git_branch_name_is_valid(int *valid, const char *name)
+{
+	git_buf ref_name = GIT_BUF_INIT;
+	int error = 0;
+
+	GIT_ASSERT(valid);
+
+	*valid = 0;
+
+	/*
+	 * Discourage branch name starting with dash,
+	 * https://github.com/git/git/commit/6348624010888b
+	 * and discourage HEAD as branch name,
+	 * https://github.com/git/git/commit/a625b092cc5994
+	 */
+	if (!name || name[0] == '-' || !git__strcmp(name, "HEAD"))
+		goto done;
+
+	if ((error = git_buf_puts(&ref_name, GIT_REFS_HEADS_DIR)) < 0 ||
+	    (error = git_buf_puts(&ref_name, name)) < 0)
+		goto done;
+
+	error = git_reference_name_is_valid(valid, ref_name.ptr);
+
+done:
+	git_buf_dispose(&ref_name);
+	return error;
+}
diff --git a/src/fetch.c b/src/fetch.c
index f4a4c9f81..dedbb54fa 100644
--- a/src/fetch.c
+++ b/src/fetch.c
@@ -21,9 +21,12 @@
 
 static int maybe_want(git_remote *remote, git_remote_head *head, git_odb *odb, git_refspec *tagspec, git_remote_autotag_option_t tagopt)
 {
-	int match = 0;
+	int match = 0, valid;
 
-	if (!git_reference_is_valid_name(head->name))
+	if (git_reference_name_is_valid(&valid, head->name) < 0)
+		return -1;
+
+	if (!valid)
 		return 0;
 
 	if (tagopt == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
diff --git a/src/refs.c b/src/refs.c
index 51635a9e4..497b066d2 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -239,7 +239,7 @@ int git_reference_lookup_resolved(
 
 int git_reference_dwim(git_reference **out, git_repository *repo, const char *refname)
 {
-	int error = 0, i;
+	int error = 0, i, valid;
 	bool fallbackmode = true, foundvalid = false;
 	git_reference *ref;
 	git_buf refnamebuf = GIT_BUF_INIT, name = GIT_BUF_INIT;
@@ -265,10 +265,11 @@ int git_reference_dwim(git_reference **out, git_repository *repo, const char *re
 
 		git_buf_clear(&refnamebuf);
 
-		if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0)
+		if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0 ||
+		    (error = git_reference_name_is_valid(&valid, git_buf_cstr(&refnamebuf))) < 0)
 			goto cleanup;
 
-		if (!git_reference_is_valid_name(git_buf_cstr(&refnamebuf))) {
+		if (!valid) {
 			error = GIT_EINVALIDSPEC;
 			continue;
 		}
@@ -1287,19 +1288,30 @@ cleanup:
 	return error;
 }
 
-int git_reference__is_valid_name(const char *refname, unsigned int flags)
+int git_reference__name_is_valid(
+	int *valid,
+	const char *refname,
+	unsigned int flags)
 {
-	if (git_reference__normalize_name(NULL, refname, flags) < 0) {
-		git_error_clear();
-		return false;
-	}
+	int error;
 
-	return true;
+	GIT_ASSERT(valid && refname);
+
+	*valid = 0;
+
+	error = git_reference__normalize_name(NULL, refname, flags);
+
+	if (!error)
+		*valid = 1;
+	else if (error == GIT_EINVALIDSPEC)
+		error = 0;
+
+	return error;
 }
 
-int git_reference_is_valid_name(const char *refname)
+int git_reference_name_is_valid(int *valid, const char *refname)
 {
-	return git_reference__is_valid_name(refname, GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL);
+	return git_reference__name_is_valid(valid, refname, GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL);
 }
 
 const char *git_reference__shorthand(const char *name)
@@ -1345,3 +1357,18 @@ int git_reference__is_unborn_head(bool *unborn, const git_reference *ref, git_re
 
 	return 0;
 }
+
+/* Deprecated functions */
+
+#ifndef GIT_DEPRECATE_HARD
+
+int git_reference_is_valid_name(const char *refname)
+{
+	int valid = 0;
+
+	git_reference__name_is_valid(&valid, refname, GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL);
+
+	return valid;
+}
+
+#endif
diff --git a/src/refs.h b/src/refs.h
index e0ee03b0e..376a512f8 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -85,7 +85,7 @@ git_reference *git_reference__realloc(git_reference **ptr_to_ref, const char *na
 
 int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags);
 int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid, const git_signature *sig, const char *log_message);
-int git_reference__is_valid_name(const char *refname, unsigned int flags);
+int git_reference__name_is_valid(int *valid, const char *name, unsigned int flags);
 int git_reference__is_branch(const char *ref_name);
 int git_reference__is_remote(const char *ref_name);
 int git_reference__is_tag(const char *ref_name);
diff --git a/src/refspec.c b/src/refspec.c
index 854240a84..4245cbbda 100644
--- a/src/refspec.c
+++ b/src/refspec.c
@@ -21,7 +21,8 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 	size_t llen;
 	int is_glob = 0;
 	const char *lhs, *rhs;
-	int flags;
+	int valid = 0;
+	unsigned int flags;
 
 	assert(refspec && input);
 
@@ -75,57 +76,69 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 
 	if (is_fetch) {
 		/*
-			* LHS
-			* - empty is allowed; it means HEAD.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * LHS
+		 * - empty is allowed; it means HEAD.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!*refspec->src)
 			; /* empty is ok */
-		else if (!git_reference__is_valid_name(refspec->src, flags))
+		else if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+			goto on_error;
+		else if (!valid)
 			goto invalid;
+
 		/*
-			* RHS
-			* - missing is ok, and is same as empty.
-			* - empty is ok; it means not to store.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * RHS
+		 * - missing is ok, and is same as empty.
+		 * - empty is ok; it means not to store.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!refspec->dst)
 			; /* ok */
 		else if (!*refspec->dst)
 			; /* ok */
-		else if (!git_reference__is_valid_name(refspec->dst, flags))
+		else if (git_reference__name_is_valid(&valid, refspec->dst, flags) < 0)
+			goto on_error;
+		else if (!valid)
 			goto invalid;
 	} else {
 		/*
-			* LHS
-			* - empty is allowed; it means delete.
-			* - when wildcarded, it must be a valid looking ref.
-			* - otherwise, it must be an extended SHA-1, but
-			*   there is no existing way to validate this.
-			*/
+		 * LHS
+		 * - empty is allowed; it means delete.
+		 * - when wildcarded, it must be a valid looking ref.
+		 * - otherwise, it must be an extended SHA-1, but
+		 *   there is no existing way to validate this.
+		 */
 		if (!*refspec->src)
 			; /* empty is ok */
 		else if (is_glob) {
-			if (!git_reference__is_valid_name(refspec->src, flags))
+			if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		}
 		else {
 			; /* anything goes, for now */
 		}
+
 		/*
-			* RHS
-			* - missing is allowed, but LHS then must be a
-			*   valid looking ref.
-			* - empty is not allowed.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * RHS
+		 * - missing is allowed, but LHS then must be a
+		 *   valid looking ref.
+		 * - empty is not allowed.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!refspec->dst) {
-			if (!git_reference__is_valid_name(refspec->src, flags))
+			if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		} else if (!*refspec->dst) {
 			goto invalid;
 		} else {
-			if (!git_reference__is_valid_name(refspec->dst, flags))
+			if (git_reference__name_is_valid(&valid, refspec->dst, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		}
 
@@ -141,11 +154,14 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 
 	return 0;
 
- invalid:
-        git_error_set(
-                GIT_ERROR_INVALID,
-                "'%s' is not a valid refspec.", input);
-        git_refspec__dispose(refspec);
+invalid:
+	git_error_set(GIT_ERROR_INVALID,
+	              "'%s' is not a valid refspec.", input);
+	git_refspec__dispose(refspec);
+	return GIT_EINVALIDSPEC;
+
+on_error:
+	git_refspec__dispose(refspec);
 	return -1;
 }
 
diff --git a/src/remote.c b/src/remote.c
index 51e99dc94..f63824861 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -82,9 +82,11 @@ static int download_tags_value(git_remote *remote, git_config *cfg)
 
 static int ensure_remote_name_is_valid(const char *name)
 {
-	int error = 0;
+	int valid, error;
+
+	error = git_remote_name_is_valid(&valid, name);
 
-	if (!git_remote_is_valid_name(name)) {
+	if (!error && !valid) {
 		git_error_set(
 			GIT_ERROR_CONFIG,
 			"'%s' is not a valid remote name.", name ? name : "(null)");
@@ -110,12 +112,8 @@ 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 (git_error_last()->klass != GIT_ERROR_NOMEMORY)
-			error = GIT_EINVALIDSPEC;
-
+	if ((error = git_refspec__parse(&spec, refspec, fetch)) < 0)
 		return error;
-	}
 
 	git_refspec__dispose(&spec);
 
@@ -1362,7 +1360,7 @@ static int update_tips_for_spec(
 		git_vector *refs,
 		const char *log_message)
 {
-	int error = 0, autotag;
+	int error = 0, autotag, valid;
 	unsigned int i = 0;
 	git_buf refname = GIT_BUF_INIT;
 	git_oid old;
@@ -1390,7 +1388,10 @@ static int update_tips_for_spec(
 		git_buf_clear(&refname);
 
 		/* Ignore malformed ref names (which also saves us from tag^{} */
-		if (!git_reference_is_valid_name(head->name))
+		if (git_reference_name_is_valid(&valid, head->name) < 0)
+			goto on_error;
+
+		if (!valid)
 			continue;
 
 		/* If we have a tag, see if the auto-follow rules say to update it */
@@ -1499,6 +1500,7 @@ static int next_head(const git_remote *remote, git_vector *refs,
 	git_remote_head *head;
 	git_refspec *spec, *passive_spec;
 	size_t i, j, k;
+	int valid;
 
 	active = &remote->active_refspecs;
 	passive = &remote->passive_refspecs;
@@ -1510,7 +1512,10 @@ static int next_head(const git_remote *remote, git_vector *refs,
 	for (; i < refs->length; i++) {
 		head = git_vector_get(refs, i);
 
-		if (!git_reference_is_valid_name(head->name))
+		if (git_reference_name_is_valid(&valid, head->name) < 0)
+			return -1;
+
+		if (!valid)
 			continue;
 
 		for (; j < active->length; j++) {
@@ -2089,24 +2094,34 @@ cleanup:
 	return error;
 }
 
-int git_remote_is_valid_name(
-	const char *remote_name)
+int git_remote_name_is_valid(int *valid, const char *remote_name)
 {
 	git_buf buf = GIT_BUF_INIT;
-	git_refspec refspec;
-	int error = -1;
+	git_refspec refspec = {0};
+	int error;
+
+	GIT_ASSERT(valid);
+
+	*valid = 0;
 
 	if (!remote_name || *remote_name == '\0')
 		return 0;
 
-	git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", remote_name);
+	if ((error = git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", remote_name)) < 0)
+		goto done;
+
 	error = git_refspec__parse(&refspec, git_buf_cstr(&buf), true);
 
+	if (!error)
+		*valid = 1;
+	else if (error == GIT_EINVALIDSPEC)
+		error = 0;
+
+done:
 	git_buf_dispose(&buf);
 	git_refspec__dispose(&refspec);
 
-	git_error_clear();
-	return error == 0;
+	return error;
 }
 
 git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname)
@@ -2605,3 +2620,17 @@ char *apply_insteadof(git_config *config, const char *url, int direction)
 
 	return result.ptr;
 }
+
+/* Deprecated functions */
+
+#ifndef GIT_DEPRECATE_HARD
+
+int git_remote_is_valid_name(const char *remote_name)
+{
+	int valid = 0;
+
+	git_remote_name_is_valid(&valid, remote_name);
+	return valid;
+}
+
+#endif
diff --git a/src/repository.c b/src/repository.c
index 513dbd61f..f0d4b06aa 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -2359,7 +2359,7 @@ int git_repository_initialbranch(git_buf *out, git_repository *repo)
 	git_config *config;
 	git_config_entry *entry = NULL;
 	const char *branch;
-	int error;
+	int valid, error;
 
 	if ((error = git_repository_config__weakptr(&config, repo)) < 0)
 		return error;
@@ -2375,10 +2375,11 @@ int git_repository_initialbranch(git_buf *out, git_repository *repo)
 	}
 
 	if ((error = git_buf_puts(out, GIT_REFS_HEADS_DIR)) < 0 ||
-	    (error = git_buf_puts(out, branch)) < 0)
+	    (error = git_buf_puts(out, branch)) < 0 ||
+	    (error = git_reference_name_is_valid(&valid, out->ptr)) < 0)
 	    goto done;
 
-	if (!git_reference_is_valid_name(out->ptr)) {
+	if (!valid) {
 		git_error_set(GIT_ERROR_INVALID, "the value of init.defaultBranch is not a valid reference name");
 		error = -1;
 	}
diff --git a/src/tag.c b/src/tag.c
index 037dc6664..8a4d6eec8 100644
--- a/src/tag.c
+++ b/src/tag.c
@@ -522,6 +522,31 @@ int git_tag_peel(git_object **tag_target, const git_tag *tag)
 	return git_object_peel(tag_target, (const git_object *)tag, GIT_OBJECT_ANY);
 }
 
+int git_tag_name_is_valid(int *valid, const char *name)
+{
+	git_buf ref_name = GIT_BUF_INIT;
+	int error = 0;
+
+	GIT_ASSERT(valid);
+
+	/*
+	 * Discourage tag name starting with dash,
+	 * https://github.com/git/git/commit/4f0accd638b8d2
+	 */
+	if (!name || name[0] == '-')
+		goto done;
+
+	if ((error = git_buf_puts(&ref_name, GIT_REFS_TAGS_DIR)) < 0 ||
+	    (error = git_buf_puts(&ref_name, name)) < 0)
+		goto done;
+
+	error = git_reference_name_is_valid(valid, ref_name.ptr);
+
+done:
+	git_buf_dispose(&ref_name);
+	return error;
+}
+
 /* Deprecated Functions */
 
 #ifndef GIT_DEPRECATE_HARD
diff --git a/tests/network/refspecs.c b/tests/network/refspecs.c
index 5c8eb1502..d9e0d9e8d 100644
--- a/tests/network/refspecs.c
+++ b/tests/network/refspecs.c
@@ -13,7 +13,7 @@ static void assert_refspec(unsigned int direction, const char *input, bool is_ex
 	if (is_expected_to_be_valid)
 		cl_assert_equal_i(0, error);
 	else
-		cl_assert_equal_i(GIT_ERROR, error);
+		cl_assert_equal_i(GIT_EINVALIDSPEC, error);
 }
 
 void test_network_refspecs__parsing(void)
diff --git a/tests/network/remote/isvalidname.c b/tests/network/remote/isvalidname.c
index c26fbd0a5..a3080f67c 100644
--- a/tests/network/remote/isvalidname.c
+++ b/tests/network/remote/isvalidname.c
@@ -1,17 +1,24 @@
 #include "clar_libgit2.h"
 
+static int is_valid_name(const char *name)
+{
+	int valid = 0;
+	cl_git_pass(git_remote_name_is_valid(&valid, name));
+	return valid;
+}
+
 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/"));
+	cl_assert_equal_i(false, is_valid_name("/"));
+	cl_assert_equal_i(false, is_valid_name("//"));
+	cl_assert_equal_i(false, is_valid_name(".lock"));
+	cl_assert_equal_i(false, is_valid_name("a.lock"));
+	cl_assert_equal_i(false, is_valid_name("/no/leading/slash"));
+	cl_assert_equal_i(false, 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"));
+	cl_assert_equal_i(true, is_valid_name("webmatrix"));
+	cl_assert_equal_i(true, is_valid_name("yishaigalatzer/rules"));
 }
diff --git a/tests/online/push_util.c b/tests/online/push_util.c
index fe9af2c43..b39f78114 100644
--- a/tests/online/push_util.c
+++ b/tests/online/push_util.c
@@ -58,12 +58,14 @@ int record_update_tips_cb(const char *refname, const git_oid *a, const git_oid *
 int create_deletion_refspecs(git_vector *out, const git_remote_head **heads, size_t heads_len)
 {
 	git_buf del_spec = GIT_BUF_INIT;
+	int valid;
 	size_t i;
 
 	for (i = 0; i < heads_len; i++) {
 		const git_remote_head *head = heads[i];
 		/* Ignore malformed ref names (which also saves us from tag^{} */
-		if (!git_reference_is_valid_name(head->name))
+		cl_git_pass(git_reference_name_is_valid(&valid, head->name));
+		if (!valid)
 			return 0;
 
 		/* Create a refspec that deletes a branch in the remote */
diff --git a/tests/refs/branches/name.c b/tests/refs/branches/name.c
index 176f836a4..290916eec 100644
--- a/tests/refs/branches/name.c
+++ b/tests/refs/branches/name.c
@@ -43,3 +43,20 @@ void test_refs_branches_name__error_when_ref_is_no_branch(void)
 	cl_git_pass(git_reference_lookup(&ref,repo,"refs/notes/fanout"));
 	cl_git_fail(git_branch_name(&name,ref));
 }
+
+static int name_is_valid(const char *name)
+{
+	int valid;
+	cl_git_pass(git_branch_name_is_valid(&valid, name));
+	return valid;
+}
+
+void test_refs_branches_is_name_valid(void)
+{
+	cl_assert_equal_i(true, name_is_valid("master"));
+	cl_assert_equal_i(true, name_is_valid("test/master"));
+
+	cl_assert_equal_i(false, name_is_valid(""));
+	cl_assert_equal_i(false, name_is_valid("HEAD"));
+	cl_assert_equal_i(false, name_is_valid("-dash"));
+}
diff --git a/tests/refs/isvalidname.c b/tests/refs/isvalidname.c
index 65c70ba4d..063f0f798 100644
--- a/tests/refs/isvalidname.c
+++ b/tests/refs/isvalidname.c
@@ -1,31 +1,38 @@
 #include "clar_libgit2.h"
 
+static bool is_valid_name(const char *name)
+{
+	int valid;
+	cl_git_pass(git_reference_name_is_valid(&valid, name));
+	return valid;
+}
+
 void test_refs_isvalidname__can_detect_invalid_formats(void)
 {
-	cl_assert_equal_i(false, git_reference_is_valid_name("refs/tags/0.17.0^{}"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("TWO/LEVELS"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("ONE.LEVEL"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("HEAD/"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("NO_TRAILING_UNDERSCORE_"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("_NO_LEADING_UNDERSCORE"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("HEAD/aa"));
-	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"));
+	cl_assert_equal_i(false, is_valid_name("refs/tags/0.17.0^{}"));
+	cl_assert_equal_i(false, is_valid_name("TWO/LEVELS"));
+	cl_assert_equal_i(false, is_valid_name("ONE.LEVEL"));
+	cl_assert_equal_i(false, is_valid_name("HEAD/"));
+	cl_assert_equal_i(false, is_valid_name("NO_TRAILING_UNDERSCORE_"));
+	cl_assert_equal_i(false, is_valid_name("_NO_LEADING_UNDERSCORE"));
+	cl_assert_equal_i(false, is_valid_name("HEAD/aa"));
+	cl_assert_equal_i(false, is_valid_name("lower_case"));
+	cl_assert_equal_i(false, is_valid_name("/stupid/name/master"));
+	cl_assert_equal_i(false, is_valid_name("/"));
+	cl_assert_equal_i(false, is_valid_name("//"));
+	cl_assert_equal_i(false, is_valid_name(""));
+	cl_assert_equal_i(false, is_valid_name("refs/heads/sub.lock/webmatrix"));
 }
 
 void test_refs_isvalidname__wont_hopefully_choke_on_valid_formats(void)
 {
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/tags/0.17.0"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/LEVELS"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("HEAD"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("ONE_LEVEL"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/stash"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/remotes/origin/bim_with_3d@11296"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master{yesterday"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master}yesterday"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master{yesterday}"));
+	cl_assert_equal_i(true, is_valid_name("refs/tags/0.17.0"));
+	cl_assert_equal_i(true, is_valid_name("refs/LEVELS"));
+	cl_assert_equal_i(true, is_valid_name("HEAD"));
+	cl_assert_equal_i(true, is_valid_name("ONE_LEVEL"));
+	cl_assert_equal_i(true, is_valid_name("refs/stash"));
+	cl_assert_equal_i(true, is_valid_name("refs/remotes/origin/bim_with_3d@11296"));
+	cl_assert_equal_i(true, is_valid_name("refs/master{yesterday"));
+	cl_assert_equal_i(true, is_valid_name("refs/master}yesterday"));
+	cl_assert_equal_i(true, is_valid_name("refs/master{yesterday}"));
 }
diff --git a/tests/refs/tags/name.c b/tests/refs/tags/name.c
new file mode 100644
index 000000000..0ca5df7d6
--- /dev/null
+++ b/tests/refs/tags/name.c
@@ -0,0 +1,17 @@
+#include "clar_libgit2.h"
+
+static int name_is_valid(const char *name)
+{
+	int valid;
+	cl_git_pass(git_tag_name_is_valid(&valid, name));
+	return valid;
+}
+
+void test_refs_tags_is_name_valid(void)
+{
+	cl_assert_equal_i(true, name_is_valid("sometag"));
+	cl_assert_equal_i(true, name_is_valid("test/sometag"));
+
+	cl_assert_equal_i(false, name_is_valid(""));
+	cl_assert_equal_i(false, name_is_valid("-dash"));
+}