check_ref_format(): tighten refname rules

This changes the rules for refnames to forbid:

(1) a refname that contains "@{" in it.

Some people and foreign SCM converter may have named their branches
as frotz@24 and we still want to keep supporting it.

However, "git branch frotz@{24}" is a disaster.  It cannot even
checked out because "git checkout frotz@{24}" will interpret it as
"detach the HEAD at twenty-fourth reflog entry of the frotz branch".

(2) a refname that ends with a dot.

We already reject a path component that begins with a dot, primarily
to avoid ambiguous range interpretation.  If we allowed ".B" as a
valid ref, it is unclear if "A...B" means "in dot-B but not in A" or
"either in A or B but not in both".

But for this to be complete, we need also to forbid "A." to avoid "in
B but not in A-dot".  This was not a problem in the original range
notation, but we should have added this restriction when three-dot
notation was introduced.

Unlike "no dot at the beginning of any path component" rule, this
rule does not have to be "no dot at the end of any path component",
because you cannot abbreviate the tail end away, similar to you can
say "dot-B" to mean "refs/heads/dot-B".

For these reasons, it is not likely people created branches with these
names on purpose, but we have allowed such names to be used for quite some
time, and it is possible that people created such branches by mistake or
by accident.

To help people with branches with such unfortunate names to recover,
we still allow "branch -d 'bad.'" to delete such branches, and also allow
"branch -m bad. good" to rename them.

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 51579f677602c34053f04a1d793e5df3dedd69fa..d23fd219da8dce05712d76aad5482ca7a420a533 100644 (file)
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -32,7 +32,9 @@ imposes the following rules on how refs are named:
   caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,
   or open bracket `[` anywhere;
 
-. It cannot end with a slash `/`.
+. They cannot end with a slash `/` nor a dot `.`.
+
+. They cannot contain a sequence `@{`.
 
 These rules makes it easy for shell script based tools to parse
 refnames, pathname expansion by the shell when a refname is used
@@ -51,6 +53,8 @@ refname expressions (see linkgit:git-rev-parse[1]).  Namely:
   It may also be used to select a specific object such as with
   'git-cat-file': "git cat-file blob v1.3.3:refs.c".
 
+. at-open-brace `@{` is used as a notation to access a reflog entry.
+
 With the `--branch` option, it expands a branch name shorthand and
 prints the name of the branch the shorthand refers to.
 

diff --git a/builtin-branch.c b/builtin-branch.c
index afeed68cfd7482c40a276c00ef3b7bfd09433950..330e0c3f1605b46ab6257f0947051c3e562d9cc6 100644 (file)
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -464,12 +464,21 @@ static void rename_branch(const char *oldname, const char *newname, int force)
        struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
        unsigned char sha1[20];
        struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
+       int recovery = 0;
 
        if (!oldname)
                die("cannot rename the current branch while not on any.");
 
-       if (strbuf_check_branch_ref(&oldref, oldname))
-               die("Invalid branch name: '%s'", oldname);
+       if (strbuf_check_branch_ref(&oldref, oldname)) {
+               /*
+                * Bad name --- this could be an attempt to rename a
+                * ref that we used to allow to be created by accident.
+                */
+               if (resolve_ref(oldref.buf, sha1, 1, NULL))
+                       recovery = 1;
+               else
+                       die("Invalid branch name: '%s'", oldname);
+       }
 
        if (strbuf_check_branch_ref(&newref, newname))
                die("Invalid branch name: '%s'", newname);
@@ -484,6 +493,9 @@ static void rename_branch(const char *oldname, const char *newname, int force)
                die("Branch rename failed");
        strbuf_release(&logmsg);
 
+       if (recovery)
+               warning("Renamed a misnamed branch '%s' away", oldref.buf + 11);
+
        /* no need to pass logmsg here as HEAD didn't really move */
        if (!strcmp(oldname, head) && create_symref("HEAD", newref.buf, NULL))
                die("Branch renamed to %s, but HEAD is not updated!", newname);

diff --git a/refs.c b/refs.c
index 8d3c502a151648abefc4afae70d1ec5f7b5d41b6..e355489e512e82df8e7ed22111157220eb01efc8 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -693,7 +693,7 @@ static inline int bad_ref_char(int ch)
 
 int check_ref_format(const char *ref)
 {
-       int ch, level, bad_type;
+       int ch, level, bad_type, last;
        int ret = CHECK_REF_FORMAT_OK;
        const char *cp = ref;
 
@@ -717,19 +717,24 @@ int check_ref_format(const char *ref)
                                return CHECK_REF_FORMAT_ERROR;
                }
 
+               last = ch;
                /* scan the rest of the path component */
                while ((ch = *cp++) != 0) {
                        bad_type = bad_ref_char(ch);
-                       if (bad_type) {
+                       if (bad_type)
                                return CHECK_REF_FORMAT_ERROR;
-                       }
                        if (ch == '/')
                                break;
-                       if (ch == '.' && *cp == '.')
+                       if (last == '.' && ch == '.')
+                               return CHECK_REF_FORMAT_ERROR;
+                       if (last == '@' && ch == '{')
                                return CHECK_REF_FORMAT_ERROR;
+                       last = ch;
                }
                level++;
                if (!ch) {
+                       if (ref <= cp - 2 && cp[-2] == '.')
+                               return CHECK_REF_FORMAT_ERROR;
                        if (level < 2)
                                return CHECK_REF_FORMAT_ONELEVEL;
                        return ret;