Skip to content

Commit 4e9f932

Browse files
gitstergitster
committed
Merge branch 'jk/interpret-branch-name-fix'
Fix a handful of bugs around interpreting $branch@{upstream} notation and its lookalike, when $branch part has interesting characters, e.g. "@", and ":". * 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
2 parents f583ace + 9892d5d commit 4e9f932

File tree

3 files changed

+124
-45
lines changed

3 files changed

+124
-45
lines changed

‎sha1_name.c‎

Lines changed: 73 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ static inline int upstream_mark(const char *string, int len)
430430
}
431431

432432
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);
433-
static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
433+
static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf);
434434

435435
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
436436
{
@@ -492,7 +492,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
492492
struct strbuf buf = STRBUF_INIT;
493493
int detached;
494494

495-
if (interpret_nth_prior_checkout(str, &buf) > 0) {
495+
if (interpret_nth_prior_checkout(str, len, &buf) > 0) {
496496
detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
497497
strbuf_release(&buf);
498498
if (detached)
@@ -929,17 +929,20 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
929929
* Parse @{-N} syntax, return the number of characters parsed
930930
* if successful; otherwise signal an error with negative value.
931931
*/
932-
static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
932+
static int interpret_nth_prior_checkout(const char *name, int namelen,
933+
struct strbuf *buf)
933934
{
934935
long nth;
935936
int retval;
936937
struct grab_nth_branch_switch_cbdata cb;
937938
const char *brace;
938939
char *num_end;
939940

941+
if (namelen < 4)
942+
return -1;
940943
if (name[0] != '@' || name[1] != '{' || name[2] != '-')
941944
return -1;
942-
brace = strchr(name, '}');
945+
brace = memchr(name, '}', namelen);
943946
if (!brace)
944947
return -1;
945948
nth = strtol(name + 3, &num_end, 10);
@@ -1012,7 +1015,7 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str
10121015
return -1;
10131016

10141017
/* make sure it's a single @, or @@{.*}, not @foo */
1015-
next = strchr(name + len + 1, '@');
1018+
next = memchr(name + len + 1, '@', namelen - len - 1);
10161019
if (next && next[1] != '{')
10171020
return -1;
10181021
if (!next)
@@ -1046,6 +1049,57 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
10461049
return ret - used + len;
10471050
}
10481051

1052+
static void set_shortened_ref(struct strbuf *buf, const char *ref)
1053+
{
1054+
char *s = shorten_unambiguous_ref(ref, 0);
1055+
strbuf_reset(buf);
1056+
strbuf_addstr(buf, s);
1057+
free(s);
1058+
}
1059+
1060+
static const char *get_upstream_branch(const char *branch_buf, int len)
1061+
{
1062+
char *branch = xstrndup(branch_buf, len);
1063+
struct branch *upstream = branch_get(*branch ? branch : NULL);
1064+
1065+
/*
1066+
* Upstream can be NULL only if branch refers to HEAD and HEAD
1067+
* points to something different than a branch.
1068+
*/
1069+
if (!upstream)
1070+
die(_("HEAD does not point to a branch"));
1071+
if (!upstream->merge || !upstream->merge[0]->dst) {
1072+
if (!ref_exists(upstream->refname))
1073+
die(_("No such branch: '%s'"), branch);
1074+
if (!upstream->merge) {
1075+
die(_("No upstream configured for branch '%s'"),
1076+
upstream->name);
1077+
}
1078+
die(
1079+
_("Upstream branch '%s' not stored as a remote-tracking branch"),
1080+
upstream->merge[0]->src);
1081+
}
1082+
free(branch);
1083+
1084+
return upstream->merge[0]->dst;
1085+
}
1086+
1087+
static int interpret_upstream_mark(const char *name, int namelen,
1088+
int at, struct strbuf *buf)
1089+
{
1090+
int len;
1091+
1092+
len = upstream_mark(name + at, namelen - at);
1093+
if (!len)
1094+
return -1;
1095+
1096+
if (memchr(name, ':', at))
1097+
return -1;
1098+
1099+
set_shortened_ref(buf, get_upstream_branch(name, at));
1100+
return len + at;
1101+
}
1102+
10491103
/*
10501104
* This reads short-hand syntax that not only evaluates to a commit
10511105
* object name, but also can act as if the end user spelled the name
@@ -1069,10 +1123,9 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu
10691123
*/
10701124
int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
10711125
{
1072-
char *cp;
1073-
struct branch *upstream;
1074-
int len = interpret_nth_prior_checkout(name, buf);
1075-
int tmp_len;
1126+
char *at;
1127+
const char *start;
1128+
int len = interpret_nth_prior_checkout(name, namelen, buf);
10761129

10771130
if (!namelen)
10781131
namelen = strlen(name);
@@ -1086,44 +1139,20 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
10861139
return reinterpret(name, namelen, len, buf);
10871140
}
10881141

1089-
cp = strchr(name, '@');
1090-
if (!cp)
1091-
return -1;
1092-
1093-
len = interpret_empty_at(name, namelen, cp - name, buf);
1094-
if (len > 0)
1095-
return reinterpret(name, namelen, len, buf);
1142+
for (start = name;
1143+
(at = memchr(start, '@', namelen - (start - name)));
1144+
start = at + 1) {
10961145

1097-
tmp_len = upstream_mark(cp, namelen - (cp - name));
1098-
if (!tmp_len)
1099-
return -1;
1146+
len = interpret_empty_at(name, namelen, at - name, buf);
1147+
if (len > 0)
1148+
return reinterpret(name, namelen, len, buf);
11001149

1101-
len = cp + tmp_len - name;
1102-
cp = xstrndup(name, cp - name);
1103-
upstream = branch_get(*cp ? cp : NULL);
1104-
/*
1105-
* Upstream can be NULL only if cp refers to HEAD and HEAD
1106-
* points to something different than a branch.
1107-
*/
1108-
if (!upstream)
1109-
die(_("HEAD does not point to a branch"));
1110-
if (!upstream->merge || !upstream->merge[0]->dst) {
1111-
if (!ref_exists(upstream->refname))
1112-
die(_("No such branch: '%s'"), cp);
1113-
if (!upstream->merge) {
1114-
die(_("No upstream configured for branch '%s'"),
1115-
upstream->name);
1116-
}
1117-
die(
1118-
_("Upstream branch '%s' not stored as a remote-tracking branch"),
1119-
upstream->merge[0]->src);
1150+
len = interpret_upstream_mark(name, namelen, at - name, buf);
1151+
if (len > 0)
1152+
return len;
11201153
}
1121-
free(cp);
1122-
cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
1123-
strbuf_reset(buf);
1124-
strbuf_addstr(buf, cp);
1125-
free(cp);
1126-
return len;
1154+
1155+
return -1;
11271156
}
11281157

11291158
int strbuf_branchname(struct strbuf *sb, const char *name)

‎t/t1507-rev-parse-upstream.sh‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ test_expect_success 'setup' '
1717
test_commit 4 &&
1818
git branch --track my-side origin/side &&
1919
git branch --track local-master master &&
20+
git branch --track fun@ny origin/side &&
21+
git branch --track @funny origin/side &&
22+
git branch --track funny@ origin/side &&
2023
git remote add -t master master-only .. &&
2124
git fetch master-only &&
2225
git branch bad-upstream &&
@@ -54,6 +57,24 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
5457
test refs/remotes/origin/side = "$(full_name my-side@{u})"
5558
'
5659

60+
test_expect_success 'upstream of branch with @ in middle' '
61+
full_name fun@ny@{u} >actual &&
62+
echo refs/remotes/origin/side >expect &&
63+
test_cmp expect actual
64+
'
65+
66+
test_expect_success 'upstream of branch with @ at start' '
67+
full_name @funny@{u} >actual &&
68+
echo refs/remotes/origin/side >expect &&
69+
test_cmp expect actual
70+
'
71+
72+
test_expect_success 'upstream of branch with @ at end' '
73+
full_name funny@@{u} >actual &&
74+
echo refs/remotes/origin/side >expect &&
75+
test_cmp expect actual
76+
'
77+
5778
test_expect_success 'refs/heads/my-side@{upstream} does not resolve to my-side{upstream}' '
5879
test_must_fail full_name refs/heads/my-side@{upstream}
5980
'
@@ -210,4 +231,20 @@ test_expect_success 'log -g other@{u}@{now}' '
210231
test_cmp expect actual
211232
'
212233

234+
test_expect_success '@{reflog}-parsing does not look beyond colon' '
235+
echo content >@{yesterday} &&
236+
git add @{yesterday} &&
237+
git commit -m "funny reflog file" &&
238+
git hash-object @{yesterday} >expect &&
239+
git rev-parse HEAD:@{yesterday} >actual
240+
'
241+
242+
test_expect_success '@{upstream}-parsing does not look beyond colon' '
243+
echo content >@{upstream} &&
244+
git add @{upstream} &&
245+
git commit -m "funny upstream file" &&
246+
git hash-object @{upstream} >expect &&
247+
git rev-parse HEAD:@{upstream} >actual
248+
'
249+
213250
test_done

‎t/t1508-at-combinations.sh‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@ check() {
99
if test '$2' = 'commit'
1010
then
1111
git log -1 --format=%s '$1' >actual
12-
else
12+
elif test '$2' = 'ref'
13+
then
1314
git rev-parse --symbolic-full-name '$1' >actual
15+
else
16+
git cat-file -p '$1' >actual
1417
fi &&
1518
test_cmp expect actual
1619
"
@@ -82,4 +85,14 @@ check HEAD ref refs/heads/old-branch
8285
check "HEAD@{1}" commit new-two
8386
check "@{1}" commit old-one
8487

88+
test_expect_success 'create path with @' '
89+
echo content >normal &&
90+
echo content >fun@ny &&
91+
git add normal fun@ny &&
92+
git commit -m "funny path"
93+
'
94+
95+
check "@:normal" blob content
96+
check "@:fun@ny" blob content
97+
8598
test_done

0 commit comments

Comments
 (0)