Change check_refname_format() to reject unnormalized refnames

mhagger · gitster · commit a40e6fb67a4a · 2011-10-05T13:45:30.000-07:00
Since much of the infrastructure does not work correctly with
unnormalized refnames, change check_refname_format() to reject them.

Similarly, change "git check-ref-format" to reject unnormalized
refnames by default.  But add an option --normalize, which causes "git
check-ref-format" to normalize the refname before checking its format,
and print the normalized refname.  This is exactly the behavior of the
old --print option, which is retained but deprecated.

Signed-off-by: Michael Haggerty &lt;mhagger@alum.mit.edu&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,9 @@ git-check-ref-format - Ensures that a reference name is well formed
 SYNOPSIS
 --------
 [verse]
-'git check-ref-format' [--print]
-       [--[no-]allow-onelevel] [--refspec-pattern] <refname>
+'git check-ref-format' [--normalize]
+       [--[no-]allow-onelevel] [--refspec-pattern]
+       <refname>
 'git check-ref-format' --branch <branchname-shorthand>
 
 DESCRIPTION
@@ -45,7 +46,11 @@ git imposes the following rules on how references are named:
   bracket `[` anywhere.  See the `--refspec-pattern` option below for
   an exception to this rule.
 
-. They cannot end with a slash `/` nor a dot `.`.
+. They cannot begin or end with a slash `/` or contain multiple
+  consecutive slashes (see the `--normalize` option below for an
+  exception to this rule)
+
+. They cannot end with a dot `.`.
 
 . They cannot contain a sequence `@{`.
 
@@ -70,10 +75,6 @@ reference name expressions (see linkgit:gitrevisions[7]):
 
 . at-open-brace `@{` is used as a notation to access a reflog entry.
 
-With the `--print` option, if 'refname' is acceptable, it prints the
-canonicalized name of a hypothetical reference with that name.  That is,
-it prints 'refname' with any extra `/` characters removed.
-
 With the `--branch` option, it expands the ``previous branch syntax''
 `@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
 were on.  This option should be used by porcelains to accept this
@@ -95,6 +96,15 @@ OPTIONS
 	in place of a one full pathname component (e.g.,
 	`foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
 
+--normalize::
+	Normalize 'refname' by removing any leading slash (`/`)
+	characters and collapsing runs of adjacent slashes between
+	name components into a single slash.  Iff the normalized
+	refname is valid then print it to standard output and exit
+	with a status of 0.  (`--print` is a deprecated way to spell
+	`--normalize`.)
+
+
 EXAMPLES
 --------
 
@@ -107,7 +117,7 @@ $ git check-ref-format --branch @{-1}
 * Determine the reference name to use for a new branch:
 +
 ------------
-$ ref=$(git check-ref-format --print "refs/heads/$newbranch") ||
+$ ref=$(git check-ref-format --normalize "refs/heads/$newbranch") ||
 die "we do not like '$newbranch' as a branch name."
 ------------
 
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
 #include "strbuf.h"
 
 static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] [options] <refname>\n"
+"git check-ref-format [--normalize] [options] <refname>\n"
 "   or: git check-ref-format --branch <branchname-shorthand>";
 
 /*
@@ -51,7 +51,7 @@ static int check_ref_format_branch(const char *arg)
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	int print = 0;
+	int normalize = 0;
 	int flags = 0;
 	const char *refname;
 
@@ -62,8 +62,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 		return check_ref_format_branch(argv[2]);
 
 	for (i = 1; i < argc && argv[i][0] == '-'; i++) {
-		if (!strcmp(argv[i], "--print"))
-			print = 1;
+		if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
+			normalize = 1;
 		else if (!strcmp(argv[i], "--allow-onelevel"))
 			flags |= REFNAME_ALLOW_ONELEVEL;
 		else if (!strcmp(argv[i], "--no-allow-onelevel"))
@@ -77,13 +77,12 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 		usage(builtin_check_ref_format_usage);
 
 	refname = argv[i];
+	if (normalize)
+		refname = collapse_slashes(refname);
 	if (check_refname_format(refname, flags))
 		return 1;
-
-	if (print) {
-		refname = collapse_slashes(refname);
+	if (normalize)
 		printf("%s\n", refname);
-	}
 
 	return 0;
 }
diff --git a/refs.c b/refs.c
@@ -908,9 +908,6 @@ int check_refname_format(const char *ref, int flags)
 	int component_len, component_count = 0;
 
 	while (1) {
-		while (*ref == '/')
-			ref++; /* tolerate leading and repeated slashes */
-
 		/* We are at the start of a path component. */
 		component_len = check_refname_component(ref);
 		if (component_len < 0) {
diff --git a/refs.h b/refs.h
@@ -106,7 +106,7 @@ extern int for_each_reflog(each_ref_fn, void *);
  * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
  * reference names.  If REFNAME_REFSPEC_PATTERN is set in flags, then
  * allow a "*" wildcard character in place of one of the name
- * components.
+ * components.  No leading or repeated slashes are accepted.
  */
 extern int check_refname_format(const char *ref, int flags);
 
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
@@ -28,11 +28,17 @@ invalid_ref() {
 invalid_ref ''
 invalid_ref '/'
 invalid_ref '/' --allow-onelevel
+invalid_ref '/' --normalize
+invalid_ref '/' '--allow-onelevel --normalize'
 valid_ref 'foo/bar/baz'
-valid_ref 'refs///heads/foo'
+valid_ref 'foo/bar/baz' --normalize
+invalid_ref 'refs///heads/foo'
+valid_ref 'refs///heads/foo' --normalize
 invalid_ref 'heads/foo/'
-valid_ref '/heads/foo'
-valid_ref '///heads/foo'
+invalid_ref '/heads/foo'
+valid_ref '/heads/foo' --normalize
+invalid_ref '///heads/foo'
+valid_ref '///heads/foo' --normalize
 invalid_ref './foo'
 invalid_ref './foo/bar'
 invalid_ref 'foo/./bar'
@@ -60,12 +66,15 @@ invalid_ref "$ref"
 valid_ref "$ref" --allow-onelevel
 invalid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
 
 ref='foo/bar'
 valid_ref "$ref"
 valid_ref "$ref" --allow-onelevel
 valid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+valid_ref "$ref" --normalize
 
 ref='foo/*'
 invalid_ref "$ref"
@@ -78,6 +87,8 @@ invalid_ref "$ref"
 invalid_ref "$ref" --allow-onelevel
 valid_ref "$ref" --refspec-pattern
 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--refspec-pattern --normalize'
 
 ref='foo/*/bar'
 invalid_ref "$ref"
@@ -105,9 +116,13 @@ invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
 
 ref='/foo'
 invalid_ref "$ref"
-valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --allow-onelevel
 invalid_ref "$ref" --refspec-pattern
-valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--refspec-pattern --normalize'
+valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
 
 test_expect_success "check-ref-format --branch @{-1}" '
 	T=$(git write-tree) &&
@@ -141,12 +156,12 @@ test_expect_success 'check-ref-format --branch from subdir' '
 
 valid_ref_normalized() {
 	test_expect_success "ref name '$1' simplifies to '$2'" "
-		refname=\$(git check-ref-format --print '$1') &&
+		refname=\$(git check-ref-format --normalize '$1') &&
 		test \"\$refname\" = '$2'"
 }
 invalid_ref_normalized() {
-	test_expect_success "check-ref-format --print rejects '$1'" "
-		test_must_fail git check-ref-format --print '$1'"
+	test_expect_success "check-ref-format --normalize rejects '$1'" "
+		test_must_fail git check-ref-format --normalize '$1'"
 }
 
 valid_ref_normalized 'heads/foo' 'heads/foo'
