From 13228c30a6476456ee64eb7cefb7786d82fd2ca7 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 27 Feb 2017 04:25:40 -0500
Subject: interpret_branch_name(): handle auto-namelen for @{-1}

The interpret_branch_name() function takes a ptr/len pair
for the name, but you can pass "0" for "namelen", which will
cause it to check the length with strlen().

However, before we do that auto-namelen magic, we call
interpret_nth_prior_checkout(), which gets fed the bogus
"0". This was broken by 8cd4249c4 (interpret_branch_name:
always respect "namelen" parameter, 2014-01-15).  Though to
be fair to that commit, it was broken in the _opposite_
direction before, where we would always treat "name" as a
string even if a length was passed.

You can see the bug with "git log -g @{-1}". That code path
always passes "0", and without this patch it cannot figure
out which branch's reflog to show.

We can fix it by a small reordering of the code.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index 73a915f..9b5d14b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1263,11 +1263,12 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 {
 	char *at;
 	const char *start;
-	int len = interpret_nth_prior_checkout(name, namelen, buf);
+	int len;
 
 	if (!namelen)
 		namelen = strlen(name);
 
+	len = interpret_nth_prior_checkout(name, namelen, buf);
 	if (!len) {
 		return len; /* syntax Ok, not enough switches */
 	} else if (len > 0) {
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
index e0a6940..58c0b7e 100755
--- a/t/t0100-previous.sh
+++ b/t/t0100-previous.sh
@@ -56,5 +56,13 @@ test_expect_success 'merge @{-100} before checking out that many branches yet' '
 	test_must_fail git merge @{-100}
 '
 
+test_expect_success 'log -g @{-1}' '
+	git checkout -b last_branch &&
+	git checkout -b new_branch &&
+	echo "last_branch@{0}" >expect &&
+	git log -g --format=%gd @{-1} >actual &&
+	test_cmp expect actual
+'
+
 test_done
 
-- 
cgit v0.10.2-6-g49f6


From e322b60d65a14578995839caa5f48f5426236fdf Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:21:23 -0500
Subject: interpret_branch_name: move docstring to header file

We generally put docstrings with function declarations,
because it's the callers who need to know how the function
works. Let's do so for interpret_branch_name().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index 5c035da..81c59d4 100644
--- a/cache.h
+++ b/cache.h
@@ -1292,6 +1292,27 @@ extern char *oid_to_hex_r(char *out, const struct object_id *oid);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as sha1_to_hex */
 
+/*
+ * 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
+ * of the branch from the command line.
+ *
+ * - "@{-N}" finds the name of the Nth previous branch we were on, and
+ *   places the name of the branch in the given buf and returns the
+ *   number of characters parsed if successful.
+ *
+ * - "<branch>@{upstream}" finds the name of the other ref that
+ *   <branch> is configured to merge with (missing <branch> defaults
+ *   to the current branch), and places the name of the branch in the
+ *   given buf and returns the number of characters parsed if
+ *   successful.
+ *
+ * If the input is not of the accepted format, it returns a negative
+ * number to signal an error.
+ *
+ * If the input was ok but there are not N branch switches in the
+ * reflog, it returns 0.
+ */
 extern int interpret_branch_name(const char *str, int len, struct strbuf *);
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
diff --git a/sha1_name.c b/sha1_name.c
index 9b5d14b..28865b3 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1238,27 +1238,6 @@ static int interpret_branch_mark(const char *name, int namelen,
 	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
- * of the branch from the command line.
- *
- * - "@{-N}" finds the name of the Nth previous branch we were on, and
- *   places the name of the branch in the given buf and returns the
- *   number of characters parsed if successful.
- *
- * - "<branch>@{upstream}" finds the name of the other ref that
- *   <branch> is configured to merge with (missing <branch> defaults
- *   to the current branch), and places the name of the branch in the
- *   given buf and returns the number of characters parsed if
- *   successful.
- *
- * If the input is not of the accepted format, it returns a negative
- * number to signal an error.
- *
- * If the input was ok but there are not N branch switches in the
- * reflog, it returns 0.
- */
 int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 {
 	char *at;
-- 
cgit v0.10.2-6-g49f6


From 311fc74826ac91cd6f6ea932460f88c475bbb310 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:21:27 -0500
Subject: strbuf_branchname: drop return value

The return value from strbuf_branchname() is confusing and
useless: it's 0 if the whole name was consumed by an @-mark,
but otherwise is the length of the original name we fed.

No callers actually look at the return value, so let's just
get rid of it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index 28865b3..4c1e911 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1279,17 +1279,14 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 	return -1;
 }
 
-int strbuf_branchname(struct strbuf *sb, const char *name)
+void strbuf_branchname(struct strbuf *sb, const char *name)
 {
 	int len = strlen(name);
 	int used = interpret_branch_name(name, len, sb);
 
-	if (used == len)
-		return 0;
 	if (used < 0)
 		used = 0;
 	strbuf_add(sb, name + used, len - used);
-	return len;
 }
 
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
diff --git a/strbuf.h b/strbuf.h
index 2262b12..3bcde39 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -562,7 +562,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 	strbuf_complete(sb, '\n');
 }
 
-extern int strbuf_branchname(struct strbuf *sb, const char *name);
+extern void strbuf_branchname(struct strbuf *sb, const char *name);
 extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
 
 extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
-- 
cgit v0.10.2-6-g49f6


From 0705fe202dd30009e2033e96a17cb12299bf5ab3 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:21:30 -0500
Subject: strbuf_branchname: add docstring

This function and its companion, strbuf_check_branch_ref(),
did not have their purpose or semantics explained. Let's do
so.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/strbuf.h b/strbuf.h
index 3bcde39..2c40f1f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -562,7 +562,22 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 	strbuf_complete(sb, '\n');
 }
 
+/*
+ * Copy "name" to "sb", expanding any special @-marks as handled by
+ * interpret_branch_name(). The result is a non-qualified branch name
+ * (so "foo" or "origin/master" instead of "refs/heads/foo" or
+ * "refs/remotes/origin/master").
+ *
+ * Note that the resulting name may not be a syntactically valid refname.
+ */
 extern void strbuf_branchname(struct strbuf *sb, const char *name);
+
+/*
+ * Like strbuf_branchname() above, but confirm that the result is
+ * syntactically valid to be used as a local branch name in refs/heads/.
+ *
+ * The return value is "0" if the result is valid, and "-1" otherwise.
+ */
 extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
 
 extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
-- 
cgit v0.10.2-6-g49f6


From 0e9f62dab9fcce57279751bba47718d3f8baf3c8 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:23:01 -0500
Subject: interpret_branch_name: allow callers to restrict expansions

The interpret_branch_name() function converts names like
@{-1} and @{upstream} into branch names. The expanded ref
names are not fully qualified, and may be outside of the
refs/heads/ namespace (e.g., "@" expands to "HEAD", and
"@{upstream}" is likely to be in "refs/remotes/").

This is OK for callers like dwim_ref() which are primarily
interested in resolving the resulting name, no matter where
it is. But callers like "git branch" treat the result as a
branch name in refs/heads/.  When we expand to a ref outside
that namespace, the results are very confusing (e.g., "git
branch @" tries to create refs/heads/HEAD, which is
nonsense).

Callers can't know from the returned string how the
expansion happened (e.g., did the user really ask for a
branch named "HEAD", or did we do a bogus expansion?). One
fix would be to return some out-parameters describing the
types of expansion that occurred. This has the benefit that
the caller can generate precise error messages ("I
understood @{upstream} to mean origin/master, but that is a
remote tracking branch, so you cannot create it as a local
name").

However, out-parameters make the function interface somewhat
cumbersome. Instead, let's do the opposite: let the caller
tell us which elements to expand. That's easier to pass in,
and none of the callers give more precise error messages
than "@{upstream} isn't a valid branch name" anyway (which
should be sufficient).

The strbuf_branchname() function needs a similar parameter,
as most of the callers access interpret_branch_name()
through it.

We can break the callers down into two groups:

  1. Callers that are happy with any kind of ref in the
     result. We pass "0" here, so they continue to work
     without restrictions. This includes merge_name(),
     the reflog handling in add_pending_object_with_path(),
     and substitute_branch_name(). This last is what powers
     dwim_ref().

  2. Callers that have funny corner cases (mostly in
     git-branch and git-checkout). These need to make use of
     the new parameter, but I've left them as "0" in this
     patch, and will address them individually in follow-on
     patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/branch.c b/builtin/branch.c
index 4757075..5aab33a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -215,7 +215,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i]);
+		strbuf_branchname(&bname, argv[i], 0);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 512492a..711a923 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -452,7 +452,7 @@ static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_branchname(&buf, branch->name);
+	strbuf_branchname(&buf, branch->name, 0);
 	if (strcmp(buf.buf, branch->name))
 		branch->name = xstrdup(buf.buf);
 	strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa..84375db 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -434,7 +434,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	char *found_ref;
 	int len, early;
 
-	strbuf_branchname(&bname, remote);
+	strbuf_branchname(&bname, remote, 0);
 	remote = bname.buf;
 
 	memset(branch_head, 0, sizeof(branch_head));
diff --git a/cache.h b/cache.h
index 81c59d4..7aea885 100644
--- a/cache.h
+++ b/cache.h
@@ -1312,8 +1312,17 @@ extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as s
  *
  * If the input was ok but there are not N branch switches in the
  * reflog, it returns 0.
- */
-extern int interpret_branch_name(const char *str, int len, struct strbuf *);
+ *
+ * If "allowed" is non-zero, it is a treated as a bitfield of allowable
+ * expansions: local branches ("refs/heads/"), remote branches
+ * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
+ * allowed, even ones to refs outside of those namespaces.
+ */
+#define INTERPRET_BRANCH_LOCAL (1<<0)
+#define INTERPRET_BRANCH_REMOTE (1<<1)
+#define INTERPRET_BRANCH_HEAD (1<<2)
+extern int interpret_branch_name(const char *str, int len, struct strbuf *,
+				 unsigned allowed);
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
 extern int validate_headref(const char *ref);
diff --git a/refs.c b/refs.c
index 9bd0bc1..da847f6 100644
--- a/refs.c
+++ b/refs.c
@@ -404,7 +404,7 @@ int refname_match(const char *abbrev_name, const char *full_name)
 static char *substitute_branch_name(const char **string, int *len)
 {
 	struct strbuf buf = STRBUF_INIT;
-	int ret = interpret_branch_name(*string, *len, &buf);
+	int ret = interpret_branch_name(*string, *len, &buf, 0);
 
 	if (ret == *len) {
 		size_t size;
diff --git a/revision.c b/revision.c
index b37dbec..771d079 100644
--- a/revision.c
+++ b/revision.c
@@ -147,7 +147,7 @@ static void add_pending_object_with_path(struct rev_info *revs,
 		revs->no_walk = 0;
 	if (revs->reflog_info && obj->type == OBJ_COMMIT) {
 		struct strbuf buf = STRBUF_INIT;
-		int len = interpret_branch_name(name, 0, &buf);
+		int len = interpret_branch_name(name, 0, &buf, 0);
 		int st;
 
 		if (0 < len && name[len] && buf.len)
diff --git a/sha1_name.c b/sha1_name.c
index 4c1e911..7f754b6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1176,7 +1176,8 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str
 	return 1;
 }
 
-static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
+static int reinterpret(const char *name, int namelen, int len,
+		       struct strbuf *buf, unsigned allowed)
 {
 	/* we have extra data, which might need further processing */
 	struct strbuf tmp = STRBUF_INIT;
@@ -1184,7 +1185,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
 	int ret;
 
 	strbuf_add(buf, name + len, namelen - len);
-	ret = interpret_branch_name(buf->buf, buf->len, &tmp);
+	ret = interpret_branch_name(buf->buf, buf->len, &tmp, allowed);
 	/* that data was not interpreted, remove our cruft */
 	if (ret < 0) {
 		strbuf_setlen(buf, used);
@@ -1205,11 +1206,27 @@ static void set_shortened_ref(struct strbuf *buf, const char *ref)
 	free(s);
 }
 
+static int branch_interpret_allowed(const char *refname, unsigned allowed)
+{
+	if (!allowed)
+		return 1;
+
+	if ((allowed & INTERPRET_BRANCH_LOCAL) &&
+	    starts_with(refname, "refs/heads/"))
+		return 1;
+	if ((allowed & INTERPRET_BRANCH_REMOTE) &&
+	    starts_with(refname, "refs/remotes/"))
+		return 1;
+
+	return 0;
+}
+
 static int interpret_branch_mark(const char *name, int namelen,
 				 int at, struct strbuf *buf,
 				 int (*get_mark)(const char *, int),
 				 const char *(*get_data)(struct branch *,
-							 struct strbuf *))
+							 struct strbuf *),
+				 unsigned allowed)
 {
 	int len;
 	struct branch *branch;
@@ -1234,11 +1251,15 @@ static int interpret_branch_mark(const char *name, int namelen,
 	if (!value)
 		die("%s", err.buf);
 
+	if (!branch_interpret_allowed(value, allowed))
+		return -1;
+
 	set_shortened_ref(buf, value);
 	return len + at;
 }
 
-int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
+int interpret_branch_name(const char *name, int namelen, struct strbuf *buf,
+			  unsigned allowed)
 {
 	char *at;
 	const char *start;
@@ -1247,31 +1268,38 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 	if (!namelen)
 		namelen = strlen(name);
 
-	len = interpret_nth_prior_checkout(name, namelen, buf);
-	if (!len) {
-		return len; /* syntax Ok, not enough switches */
-	} else if (len > 0) {
-		if (len == namelen)
-			return len; /* consumed all */
-		else
-			return reinterpret(name, namelen, len, buf);
+	if (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {
+		len = interpret_nth_prior_checkout(name, namelen, buf);
+		if (!len) {
+			return len; /* syntax Ok, not enough switches */
+		} else if (len > 0) {
+			if (len == namelen)
+				return len; /* consumed all */
+			else
+				return reinterpret(name, namelen, len, buf, allowed);
+		}
 	}
 
 	for (start = name;
 	     (at = memchr(start, '@', namelen - (start - name)));
 	     start = at + 1) {
 
-		len = interpret_empty_at(name, namelen, at - name, buf);
-		if (len > 0)
-			return reinterpret(name, namelen, len, buf);
+		if (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {
+			len = interpret_empty_at(name, namelen, at - name, buf);
+			if (len > 0)
+				return reinterpret(name, namelen, len, buf,
+						   allowed);
+		}
 
 		len = interpret_branch_mark(name, namelen, at - name, buf,
-					    upstream_mark, branch_get_upstream);
+					    upstream_mark, branch_get_upstream,
+					    allowed);
 		if (len > 0)
 			return len;
 
 		len = interpret_branch_mark(name, namelen, at - name, buf,
-					    push_mark, branch_get_push);
+					    push_mark, branch_get_push,
+					    allowed);
 		if (len > 0)
 			return len;
 	}
@@ -1279,10 +1307,10 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 	return -1;
 }
 
-void strbuf_branchname(struct strbuf *sb, const char *name)
+void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 {
 	int len = strlen(name);
-	int used = interpret_branch_name(name, len, sb);
+	int used = interpret_branch_name(name, len, sb, allowed);
 
 	if (used < 0)
 		used = 0;
@@ -1291,7 +1319,7 @@ void strbuf_branchname(struct strbuf *sb, const char *name)
 
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
-	strbuf_branchname(sb, name);
+	strbuf_branchname(sb, name, 0);
 	if (name[0] == '-')
 		return -1;
 	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
diff --git a/strbuf.h b/strbuf.h
index 2c40f1f..4d225ff 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -569,8 +569,12 @@ static inline void strbuf_complete_line(struct strbuf *sb)
  * "refs/remotes/origin/master").
  *
  * Note that the resulting name may not be a syntactically valid refname.
+ *
+ * If "allowed" is non-zero, restrict the set of allowed expansions. See
+ * interpret_branch_name() for details.
  */
-extern void strbuf_branchname(struct strbuf *sb, const char *name);
+extern void strbuf_branchname(struct strbuf *sb, const char *name,
+			      unsigned allowed);
 
 /*
  * Like strbuf_branchname() above, but confirm that the result is
-- 
cgit v0.10.2-6-g49f6


From a356e8e2a724012c8120bfa69133b6118b1565f4 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:23:06 -0500
Subject: t3204: test git-branch @-expansion corner cases

git-branch feeds the branch names from the command line to
strbuf_branchname(), but we do not yet tell that function
which kinds of expansions should be allowed. Let's create a
set of tests that cover both the allowed and disallowed
cases.

That shows off some breakages where we currently create or
delete the wrong ref (and will make sure that we do not
break any cases that _should_ be working when we do add more
restrictions).

Note that we check branch creation and deletion, but do not
bother with renames. Those follow the same code path as
creation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh
new file mode 100755
index 0000000..e671a7a
--- /dev/null
+++ b/t/t3204-branch-name-interpretation.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='interpreting exotic branch name arguments
+
+Branch name arguments are usually names which are taken to be inside of
+refs/heads/, but we interpret some magic syntax like @{-1}, @{upstream}, etc.
+This script aims to check the behavior of those corner cases.
+'
+. ./test-lib.sh
+
+expect_branch() {
+	git log -1 --format=%s "$1" >actual &&
+	echo "$2" >expect &&
+	test_cmp expect actual
+}
+
+expect_deleted() {
+	test_must_fail git rev-parse --verify "$1"
+}
+
+test_expect_success 'set up repo' '
+	test_commit one &&
+	test_commit two &&
+	git remote add origin foo.git
+'
+
+test_expect_success 'update branch via @{-1}' '
+	git branch previous one &&
+
+	git checkout previous &&
+	git checkout master &&
+
+	git branch -f @{-1} two &&
+	expect_branch previous two
+'
+
+test_expect_success 'update branch via local @{upstream}' '
+	git branch local one &&
+	git branch --set-upstream-to=local &&
+
+	git branch -f @{upstream} two &&
+	expect_branch local two
+'
+
+test_expect_failure 'disallow updating branch via remote @{upstream}' '
+	git update-ref refs/remotes/origin/remote one &&
+	git branch --set-upstream-to=origin/remote &&
+
+	test_must_fail git branch -f @{upstream} two
+'
+
+test_expect_success 'create branch with pseudo-qualified name' '
+	git branch refs/heads/qualified two &&
+	expect_branch refs/heads/refs/heads/qualified two
+'
+
+test_expect_success 'delete branch via @{-1}' '
+	git branch previous-del &&
+
+	git checkout previous-del &&
+	git checkout master &&
+
+	git branch -D @{-1} &&
+	expect_deleted previous-del
+'
+
+test_expect_success 'delete branch via local @{upstream}' '
+	git branch local-del &&
+	git branch --set-upstream-to=local-del &&
+
+	git branch -D @{upstream} &&
+	expect_deleted local-del
+'
+
+test_expect_success 'delete branch via remote @{upstream}' '
+	git update-ref refs/remotes/origin/remote-del two &&
+	git branch --set-upstream-to=origin/remote-del &&
+
+	git branch -r -D @{upstream} &&
+	expect_deleted origin/remote-del
+'
+
+# Note that we create two oddly named local branches here. We want to make
+# sure that we do not accidentally delete either of them, even if
+# shorten_unambiguous_ref() tweaks the name to avoid ambiguity.
+test_expect_failure 'delete @{upstream} expansion matches -r option' '
+	git update-ref refs/remotes/origin/remote-del two &&
+	git branch --set-upstream-to=origin/remote-del &&
+	git update-ref refs/heads/origin/remote-del two &&
+	git update-ref refs/heads/remotes/origin/remote-del two &&
+
+	test_must_fail git branch -D @{upstream} &&
+	expect_branch refs/heads/origin/remote-del two &&
+	expect_branch refs/heads/remotes/origin/remote-del two
+'
+
+test_expect_failure 'disallow deleting remote branch via @{-1}' '
+	git update-ref refs/remotes/origin/previous one &&
+
+	git checkout -b origin/previous two &&
+	git checkout master &&
+
+	test_must_fail git branch -r -D @{-1} &&
+	expect_branch refs/remotes/origin/previous one &&
+	expect_branch refs/heads/origin/previous two
+'
+
+# The thing we are testing here is that "@" is the real branch refs/heads/@,
+# and not refs/heads/HEAD. These tests should not imply that refs/heads/@ is a
+# sane thing, but it _is_ technically allowed for now. If we disallow it, these
+# can be switched to test_must_fail.
+test_expect_failure 'create branch named "@"' '
+	git branch -f @ one &&
+	expect_branch refs/heads/@ one
+'
+
+test_expect_failure 'delete branch named "@"' '
+	git update-ref refs/heads/@ two &&
+	git branch -D @ &&
+	expect_deleted refs/heads/@
+'
+
+test_done
-- 
cgit v0.10.2-6-g49f6


From 6b145e016aaf512d0026cbd2c78fa28476f043b4 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:23:10 -0500
Subject: branch: restrict @-expansions when deleting

We use strbuf_branchname() to expand the branch name from
the command line, so you can delete the branch given by
@{-1}, for example.  However, we allow other nonsense like
"@", and we do not respect our "-r" flag (so we may end up
deleting an oddly-named local ref instead of a remote one).

We can fix this by passing the appropriate "allowed" flag to
strbuf_branchname().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/branch.c b/builtin/branch.c
index 5aab33a..0c92461 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -190,17 +190,20 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	int ret = 0;
 	int remote_branch = 0;
 	struct strbuf bname = STRBUF_INIT;
+	unsigned allowed_interpret;
 
 	switch (kinds) {
 	case FILTER_REFS_REMOTES:
 		fmt = "refs/remotes/%s";
 		/* For subsequent UI messages */
 		remote_branch = 1;
+		allowed_interpret = INTERPRET_BRANCH_REMOTE;
 
 		force = 1;
 		break;
 	case FILTER_REFS_BRANCHES:
 		fmt = "refs/heads/%s";
+		allowed_interpret = INTERPRET_BRANCH_LOCAL;
 		break;
 	default:
 		die(_("cannot use -a with -d"));
@@ -215,7 +218,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i], 0);
+		strbuf_branchname(&bname, argv[i], allowed_interpret);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
diff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh
index e671a7a..4f4af1f 100755
--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh
@@ -83,7 +83,7 @@ test_expect_success 'delete branch via remote @{upstream}' '
 # Note that we create two oddly named local branches here. We want to make
 # sure that we do not accidentally delete either of them, even if
 # shorten_unambiguous_ref() tweaks the name to avoid ambiguity.
-test_expect_failure 'delete @{upstream} expansion matches -r option' '
+test_expect_success 'delete @{upstream} expansion matches -r option' '
 	git update-ref refs/remotes/origin/remote-del two &&
 	git branch --set-upstream-to=origin/remote-del &&
 	git update-ref refs/heads/origin/remote-del two &&
@@ -94,7 +94,7 @@ test_expect_failure 'delete @{upstream} expansion matches -r option' '
 	expect_branch refs/heads/remotes/origin/remote-del two
 '
 
-test_expect_failure 'disallow deleting remote branch via @{-1}' '
+test_expect_success 'disallow deleting remote branch via @{-1}' '
 	git update-ref refs/remotes/origin/previous one &&
 
 	git checkout -b origin/previous two &&
@@ -114,7 +114,7 @@ test_expect_failure 'create branch named "@"' '
 	expect_branch refs/heads/@ one
 '
 
-test_expect_failure 'delete branch named "@"' '
+test_expect_success 'delete branch named "@"' '
 	git update-ref refs/heads/@ two &&
 	git branch -D @ &&
 	expect_deleted refs/heads/@
-- 
cgit v0.10.2-6-g49f6


From 7d5c960bf6f98601dce992f1ffaf4c7ab932e6dc Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:23:14 -0500
Subject: strbuf_check_ref_format(): expand only local branches

This function asks strbuf_branchname() to expand any @-marks
in the branchname, and then we blindly stick refs/heads/ in
front of the result. This is obviously nonsense if the
expansion is "HEAD" or a ref in refs/remotes/.

The most obvious end-user effect is that creating or
renaming a branch with an expansion may have confusing
results (e.g., creating refs/heads/origin/master from
"@{upstream}" when the operation should be disallowed).

We can fix this by telling strbuf_branchname() that we are
only interested in local expansions. Any unexpanded bits are
then fed to check_ref_format(), which either disallows them
(in the case of "@{upstream}") or lets them through
("refs/heads/@" is technically valid, if a bit silly).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index 7f754b6..26ceec1 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1319,7 +1319,7 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
-	strbuf_branchname(sb, name, 0);
+	strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
 	if (name[0] == '-')
 		return -1;
 	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
diff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh
index 4f4af1f..05e88f9 100755
--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh
@@ -42,7 +42,7 @@ test_expect_success 'update branch via local @{upstream}' '
 	expect_branch local two
 '
 
-test_expect_failure 'disallow updating branch via remote @{upstream}' '
+test_expect_success 'disallow updating branch via remote @{upstream}' '
 	git update-ref refs/remotes/origin/remote one &&
 	git branch --set-upstream-to=origin/remote &&
 
@@ -109,7 +109,7 @@ test_expect_success 'disallow deleting remote branch via @{-1}' '
 # and not refs/heads/HEAD. These tests should not imply that refs/heads/@ is a
 # sane thing, but it _is_ technically allowed for now. If we disallow it, these
 # can be switched to test_must_fail.
-test_expect_failure 'create branch named "@"' '
+test_expect_success 'create branch named "@"' '
 	git branch -f @ one &&
 	expect_branch refs/heads/@ one
 '
-- 
cgit v0.10.2-6-g49f6


From fd4692ff7040e690546ed139a419995e76a96196 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 2 Mar 2017 03:23:18 -0500
Subject: checkout: restrict @-expansions when finding branch

When we parse "git checkout $NAME", we try to interpret
$NAME as a local branch-name. If it is, then we point HEAD
to that branch. Otherwise, we detach the HEAD at whatever
commit $NAME points to.

We do the interpretation by calling strbuf_branchname(), and
then blindly sticking "refs/heads/" on the front. This leads
to nonsense results when expansions like "@{upstream}" or
"@" point to something besides a local branch. We end up
with a local branch name like "refs/heads/origin/master" or
"refs/heads/HEAD".

Normally this has no user-visible effect because those
branches don't exist, and so we fallback to feeding the
result to get_sha1(), which resolves them correctly.

But as the new test in t3204 shows, there are corner cases
where the effect is observable, and we check out the wrong
local branch rather than detaching to the correct one.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 711a923..767ca9e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -452,7 +452,7 @@ static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_branchname(&buf, branch->name, 0);
+	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
 	if (strcmp(buf.buf, branch->name))
 		branch->name = xstrdup(buf.buf);
 	strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
diff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh
index 05e88f9..698d9cc 100755
--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh
@@ -120,4 +120,14 @@ test_expect_success 'delete branch named "@"' '
 	expect_deleted refs/heads/@
 '
 
+test_expect_success 'checkout does not treat remote @{upstream} as a branch' '
+	git update-ref refs/remotes/origin/checkout one &&
+	git branch --set-upstream-to=origin/checkout &&
+	git update-ref refs/heads/origin/checkout two &&
+	git update-ref refs/heads/remotes/origin/checkout two &&
+
+	git checkout @{upstream} &&
+	expect_branch HEAD one
+'
+
 test_done
-- 
cgit v0.10.2-6-g49f6