Merge branch 'jk/interpret-branch-name-fix' into maint

A handful of bugs around interpreting $branch@{upstream} notation
and its lookalike, when $branch part has interesting characters,
e.g. "@", and ":", have been fixed.

* jk/interpret-branch-name-fix:
  interpret_branch_name: find all possible @-marks
  interpret_branch_name: avoid @{upstream} past colon
  interpret_branch_name: always respect "namelen" parameter
  interpret_branch_name: rename "cp" variable to "at"
  interpret_branch_name: factor out upstream handling

diff --git a/sha1_name.c b/sha1_name.c
index e9c299943b817b5b3b5b700ac3979211d5688e2e..15854e35eced2c8b0c3e796aedcfbe553c6ac38f 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -430,7 +430,7 @@ static inline int upstream_mark(const char *string, int len)
 }
 
 static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);
-static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
+static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf);
 
 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 {
@@ -492,7 +492,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
                struct strbuf buf = STRBUF_INIT;
                int detached;
 
-               if (interpret_nth_prior_checkout(str, &buf) > 0) {
+               if (interpret_nth_prior_checkout(str, len, &buf) > 0) {
                        detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
                        strbuf_release(&buf);
                        if (detached)
@@ -931,7 +931,8 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
  * Parse @{-N} syntax, return the number of characters parsed
  * if successful; otherwise signal an error with negative value.
  */
-static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
+static int interpret_nth_prior_checkout(const char *name, int namelen,
+                                       struct strbuf *buf)
 {
        long nth;
        int retval;
@@ -939,9 +940,11 @@ static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
        const char *brace;
        char *num_end;
 
+       if (namelen < 4)
+               return -1;
        if (name[0] != '@' || name[1] != '{' || name[2] != '-')
                return -1;
-       brace = strchr(name, '}');
+       brace = memchr(name, '}', namelen);
        if (!brace)
                return -1;
        nth = strtol(name + 3, &num_end, 10);
@@ -1014,7 +1017,7 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str
                return -1;
 
        /* make sure it's a single @, or @@{.*}, not @foo */
-       next = strchr(name + len + 1, '@');
+       next = memchr(name + len + 1, '@', namelen - len - 1);
        if (next && next[1] != '{')
                return -1;
        if (!next)
@@ -1048,6 +1051,57 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
        return ret - used + len;
 }
 
+static void set_shortened_ref(struct strbuf *buf, const char *ref)
+{
+       char *s = shorten_unambiguous_ref(ref, 0);
+       strbuf_reset(buf);
+       strbuf_addstr(buf, s);
+       free(s);
+}
+
+static const char *get_upstream_branch(const char *branch_buf, int len)
+{
+       char *branch = xstrndup(branch_buf, len);
+       struct branch *upstream = branch_get(*branch ? branch : NULL);
+
+       /*
+        * Upstream can be NULL only if branch refers to HEAD and HEAD
+        * points to something different than a branch.
+        */
+       if (!upstream)
+               die(_("HEAD does not point to a branch"));
+       if (!upstream->merge || !upstream->merge[0]->dst) {
+               if (!ref_exists(upstream->refname))
+                       die(_("No such branch: '%s'"), branch);
+               if (!upstream->merge) {
+                       die(_("No upstream configured for branch '%s'"),
+                               upstream->name);
+               }
+               die(
+                       _("Upstream branch '%s' not stored as a remote-tracking branch"),
+                       upstream->merge[0]->src);
+       }
+       free(branch);
+
+       return upstream->merge[0]->dst;
+}
+
+static int interpret_upstream_mark(const char *name, int namelen,
+                                  int at, struct strbuf *buf)
+{
+       int len;
+
+       len = upstream_mark(name + at, namelen - at);
+       if (!len)
+               return -1;
+
+       if (memchr(name, ':', at))
+               return -1;
+
+       set_shortened_ref(buf, get_upstream_branch(name, at));
+       return len + at;
+}
+
 /*
  * This reads short-hand syntax that not only evaluates to a commit
  * object name, but also can act as if the end user spelled the name
@@ -1071,10 +1125,9 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
  */
 int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 {
-       char *cp;
-       struct branch *upstream;
-       int len = interpret_nth_prior_checkout(name, buf);
-       int tmp_len;
+       char *at;
+       const char *start;
+       int len = interpret_nth_prior_checkout(name, namelen, buf);
 
        if (!namelen)
                namelen = strlen(name);
@@ -1088,44 +1141,20 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
                        return reinterpret(name, namelen, len, buf);
        }
 
-       cp = strchr(name, '@');
-       if (!cp)
-               return -1;
-
-       len = interpret_empty_at(name, namelen, cp - name, buf);
-       if (len > 0)
-               return reinterpret(name, namelen, len, buf);
+       for (start = name;
+            (at = memchr(start, '@', namelen - (start - name)));
+            start = at + 1) {
 
-       tmp_len = upstream_mark(cp, namelen - (cp - name));
-       if (!tmp_len)
-               return -1;
+               len = interpret_empty_at(name, namelen, at - name, buf);
+               if (len > 0)
+                       return reinterpret(name, namelen, len, buf);
 
-       len = cp + tmp_len - name;
-       cp = xstrndup(name, cp - name);
-       upstream = branch_get(*cp ? cp : NULL);
-       /*
-        * Upstream can be NULL only if cp refers to HEAD and HEAD
-        * points to something different than a branch.
-        */
-       if (!upstream)
-               die(_("HEAD does not point to a branch"));
-       if (!upstream->merge || !upstream->merge[0]->dst) {
-               if (!ref_exists(upstream->refname))
-                       die(_("No such branch: '%s'"), cp);
-               if (!upstream->merge) {
-                       die(_("No upstream configured for branch '%s'"),
-                               upstream->name);
-               }
-               die(
-                       _("Upstream branch '%s' not stored as a remote-tracking branch"),
-                       upstream->merge[0]->src);
+               len = interpret_upstream_mark(name, namelen, at - name, buf);
+               if (len > 0)
+                       return len;
        }
-       free(cp);
-       cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
-       strbuf_reset(buf);
-       strbuf_addstr(buf, cp);
-       free(cp);
-       return len;
+
+       return -1;
 }
 
 int strbuf_branchname(struct strbuf *sb, const char *name)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2a19e797eb7cffba28deb4e99b3708c5b26602ae..178694ee63937a9ec1f8d182d54441c2d20bd4a3 100755 (executable)
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -17,6 +17,9 @@ test_expect_success 'setup' '
         test_commit 4 &&
         git branch --track my-side origin/side &&
         git branch --track local-master master &&
+        git branch --track fun@ny origin/side &&
+        git branch --track @funny origin/side &&
+        git branch --track funny@ origin/side &&
         git remote add -t master master-only .. &&
         git fetch master-only &&
         git branch bad-upstream &&
@@ -54,6 +57,24 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
        test refs/remotes/origin/side = "$(full_name my-side@{u})"
 '
 
+test_expect_success 'upstream of branch with @ in middle' '
+       full_name fun@ny@{u} >actual &&
+       echo refs/remotes/origin/side >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'upstream of branch with @ at start' '
+       full_name @funny@{u} >actual &&
+       echo refs/remotes/origin/side >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'upstream of branch with @ at end' '
+       full_name funny@@{u} >actual &&
+       echo refs/remotes/origin/side >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'refs/heads/my-side@{upstream} does not resolve to my-side{upstream}' '
        test_must_fail full_name refs/heads/my-side@{upstream}
 '
@@ -210,4 +231,20 @@ test_expect_success 'log -g other@{u}@{now}' '
        test_cmp expect actual
 '
 
+test_expect_success '@{reflog}-parsing does not look beyond colon' '
+       echo content >@{yesterday} &&
+       git add @{yesterday} &&
+       git commit -m "funny reflog file" &&
+       git hash-object @{yesterday} >expect &&
+       git rev-parse HEAD:@{yesterday} >actual
+'
+
+test_expect_success '@{upstream}-parsing does not look beyond colon' '
+       echo content >@{upstream} &&
+       git add @{upstream} &&
+       git commit -m "funny upstream file" &&
+       git hash-object @{upstream} >expect &&
+       git rev-parse HEAD:@{upstream} >actual
+'
+
 test_done

diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
index ceb844985f97e9472ffcc3fe2f375912d126a4e8..078e1195dfa1625796fdbbf1b054ebf8a1ba2621 100755 (executable)
--- a/t/t1508-at-combinations.sh
+++ b/t/t1508-at-combinations.sh
@@ -9,8 +9,11 @@ check() {
                if test '$2' = 'commit'
                then
                        git log -1 --format=%s '$1' >actual
-               else
+               elif test '$2' = 'ref'
+               then
                        git rev-parse --symbolic-full-name '$1' >actual
+               else
+                       git cat-file -p '$1' >actual
                fi &&
                test_cmp expect actual
        "
@@ -82,4 +85,14 @@ check HEAD ref refs/heads/old-branch
 check "HEAD@{1}" commit new-two
 check "@{1}" commit old-one
 
+test_expect_success 'create path with @' '
+       echo content >normal &&
+       echo content >fun@ny &&
+       git add normal fun@ny &&
+       git commit -m "funny path"
+'
+
+check "@:normal" blob content
+check "@:fun@ny" blob content
+
 test_done