Merge pull request libgit2#5659 from libgit2/ethomson/name_is_valid

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

Author: ethomson

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
 ----
 

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

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,

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

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.

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

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;
+}

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) {

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 @@ int git_reference_peel(
 	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

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);