refs: loosen restriction on wildcard "*" refspecs

Loosen restrictions on refspecs by allowing patterns that have a "*"
within a component instead of only as the whole component.

Remove the logic to accept a single "*" as a whole component from
check_refname_format(), and implement an extended form of that logic
in check_refname_component().  Pass the pointer to the flags argument
to the latter, as it has to clear REFNAME_REFSPEC_PATTERN bit when
it sees "*".

Teach check_refname_component() function to allow an asterisk "*"
only when REFNAME_REFSPEC_PATTERN is set in the flags, and drop the
bit after seeing a "*", to ensure that one side of a refspec
contains at most one asterisk.

This will allow us to accept refspecs such as `for/bar*:foo/baz*`.
Any refspec which functioned before shall continue functioning with
the new logic.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index fc02959..9044dfa 100644 (file)
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -94,8 +94,8 @@ OPTIONS
        Interpret <refname> as a reference name pattern for a refspec
        (as used with remote repositories).  If this option is
        enabled, <refname> is allowed to contain a single `*`
-       in place of a one full pathname component (e.g.,
-       `foo/*/bar` but not `foo/bar*`).
+       in the refspec (e.g., `foo/bar*/baz` or `foo/bar*baz/`
+       but not `foo/bar*/baz*`).
 
 --normalize::
        Normalize 'refname' by removing any leading slash (`/`)

diff --git a/refs.c b/refs.c
index 0900f54..3127518 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -21,12 +21,13 @@ struct ref_lock {
  * 2: ., look for a preceding . to reject .. in refs
  * 3: {, look for a preceding @ to reject @{ in refs
  * 4: A bad character: ASCII control characters, and
- *    "*", ":", "?", "[", "\", "^", "~", SP, or TAB
+ *    ":", "?", "[", "\", "^", "~", SP, or TAB
+ * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set
  */
 static unsigned char refname_disposition[256] = {
        1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
        4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-       4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
+       4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
@@ -73,12 +74,13 @@ static unsigned char refname_disposition[256] = {
  * - any path component of it begins with ".", or
  * - it has double dots "..", or
  * - it has ASCII control characters, or
- * - it has "*", ":", "?", "[", "\", "^", "~", SP, or TAB anywhere, or
+ * - it has ":", "?", "[", "\", "^", "~", SP, or TAB anywhere, or
+ * - it has "*" anywhere unless REFNAME_REFSPEC_PATTERN is set, or
  * - it ends with a "/", or
  * - it ends with ".lock", or
  * - it contains a "@{" portion
  */
-static int check_refname_component(const char *refname, int flags)
+static int check_refname_component(const char *refname, int *flags)
 {
        const char *cp;
        char last = '\0';
@@ -99,6 +101,16 @@ static int check_refname_component(const char *refname, int flags)
                        break;
                case 4:
                        return -1;
+               case 5:
+                       if (!(*flags & REFNAME_REFSPEC_PATTERN))
+                               return -1; /* refspec can't be a pattern */
+
+                       /*
+                        * Unset the pattern flag so that we only accept
+                        * a single asterisk for one side of refspec.
+                        */
+                       *flags &= ~ REFNAME_REFSPEC_PATTERN;
+                       break;
                }
                last = ch;
        }
@@ -123,18 +135,10 @@ int check_refname_format(const char *refname, int flags)
 
        while (1) {
                /* We are at the start of a path component. */
-               component_len = check_refname_component(refname, flags);
-               if (component_len <= 0) {
-                       if ((flags & REFNAME_REFSPEC_PATTERN) &&
-                                       refname[0] == '*' &&
-                                       (refname[1] == '\0' || refname[1] == '/')) {
-                               /* Accept one wildcard as a full refname component. */
-                               flags &= ~REFNAME_REFSPEC_PATTERN;
-                               component_len = 1;
-                       } else {
-                               return -1;
-                       }
-               }
+               component_len = check_refname_component(refname, &flags);
+               if (component_len <= 0)
+                       return -1;
+
                component_count++;
                if (refname[component_len] == '\0')
                        break;

diff --git a/refs.h b/refs.h
index cf642e6..417f2c8 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -224,8 +224,8 @@ extern int for_each_reflog(each_ref_fn, void *);
  * to the rules described in Documentation/git-check-ref-format.txt.
  * If 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.  No leading or repeated slashes are accepted.
+ * allow a single "*" wildcard character in the refspec. No leading or
+ * repeated slashes are accepted.
  */
 extern int check_refname_format(const char *refname, int flags);
 

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index e5dc62e..0790edf 100755 (executable)
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -62,9 +62,11 @@ invalid_ref 'heads/foo\bar'
 invalid_ref "$(printf 'heads/foo\t')"
 invalid_ref "$(printf 'heads/foo\177')"
 valid_ref "$(printf 'heads/fu\303\237')"
-invalid_ref 'heads/*foo/bar' --refspec-pattern
-invalid_ref 'heads/foo*/bar' --refspec-pattern
-invalid_ref 'heads/f*o/bar' --refspec-pattern
+valid_ref 'heads/*foo/bar' --refspec-pattern
+valid_ref 'heads/foo*/bar' --refspec-pattern
+valid_ref 'heads/f*o/bar' --refspec-pattern
+invalid_ref 'heads/f*o*/bar' --refspec-pattern
+invalid_ref 'heads/foo*/bar*' --refspec-pattern
 
 ref='foo'
 invalid_ref "$ref"

diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index de6db86..f541f30 100755 (executable)
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -71,15 +71,18 @@ test_refspec fetch ':refs/remotes/frotz/HEAD-to-me'
 test_refspec push ':refs/remotes/frotz/delete me'              invalid
 test_refspec fetch ':refs/remotes/frotz/HEAD to me'            invalid
 
-test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' invalid
-test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' invalid
+test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah'
+test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah'
 
-test_refspec fetch 'refs/heads*/for-linus:refs/remotes/mine/*' invalid
-test_refspec push 'refs/heads*/for-linus:refs/remotes/mine/*' invalid
+test_refspec fetch 'refs/heads*/for-linus:refs/remotes/mine/*'
+test_refspec push 'refs/heads*/for-linus:refs/remotes/mine/*'
 
 test_refspec fetch 'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
 test_refspec push 'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
 
+test_refspec fetch 'refs/heads/*g*/for-linus:refs/remotes/mine/*' invalid
+test_refspec push 'refs/heads/*g*/for-linus:refs/remotes/mine/*' invalid
+
 test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*'
 test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*'