Skip to content

Commit 47c6ef1

Browse files
gitstergitster
committed
make sure parsed wildcard refspec ends with slash
A wildcard refspec is internally parsed into a refspec structure with src and dst strings. Many parts of the code assumed that these do not include the trailing "/*" when matching the wildcard pattern with an actual ref we see at the remote. What this meant was that we needed to make sure not just that the prefix matched, and also that a slash followed the part that matched. But a codepath that scans the result from ls-remote and finds matching refs forgot to check the "matching part must be followed by a slash" rule. This resulted in "refs/heads/b1" from the remote side to mistakenly match the source side of "refs/heads/b/*:refs/remotes/b/*" refspec. Worse, the refspec crafted internally by "git-clone", and a hardcoded preparsed refspec that is used to implement "git-fetch --tags", violated this "parsed widcard refspec does not end with slash" rule; simply adding the "matching part must be followed by a slash" rule then would have broken codepaths that use these refspecs. This commit changes the rule to require a trailing slash to parsed wildcard refspecs. IOW, "refs/heads/b/*:refs/remotes/b/*" is parsed as src = "refs/heads/b/" and dst = "refs/remotes/b/". This allows us to simplify the matching logic because we only need to do a prefixcmp() to notice "refs/heads/b/one" matches and "refs/heads/b1" does not. Acked-by: Daniel Barkalow <barkalow@iabervon.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent ef115e2 commit 47c6ef1

File tree

2 files changed

+64
-18
lines changed

2 files changed

+64
-18
lines changed

‎remote.c‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -427,18 +427,40 @@ static void read_config(void)
427427
alias_all_urls();
428428
}
429429

430+
/*
431+
* We need to make sure the tracking branches are well formed, but a
432+
* wildcard refspec in "struct refspec" must have a trailing slash. We
433+
* temporarily drop the trailing '/' while calling check_ref_format(),
434+
* and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
435+
* error return is Ok for a wildcard refspec.
436+
*/
437+
static int verify_refname(char *name, int is_glob)
438+
{
439+
int result, len = -1;
440+
441+
if (is_glob) {
442+
len = strlen(name);
443+
assert(name[len - 1] == '/');
444+
name[len - 1] = '\0';
445+
}
446+
result = check_ref_format(name);
447+
if (is_glob)
448+
name[len - 1] = '/';
449+
return result;
450+
}
451+
430452
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
431453
{
432454
int i;
433455
int st;
434456
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
435457

436458
for (i = 0; i < nr_refspec; i++) {
437-
size_t llen, rlen;
459+
size_t llen;
438460
int is_glob;
439461
const char *lhs, *rhs;
440462

441-
llen = rlen = is_glob = 0;
463+
llen = is_glob = 0;
442464

443465
lhs = refspec[i];
444466
if (*lhs == '+') {
@@ -458,20 +480,17 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
458480
}
459481

460482
if (rhs) {
461-
rhs++;
462-
rlen = strlen(rhs);
483+
size_t rlen = strlen(++rhs);
463484
is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*"));
464-
if (is_glob)
465-
rlen -= 2;
466-
rs[i].dst = xstrndup(rhs, rlen);
485+
rs[i].dst = xstrndup(rhs, rlen - is_glob);
467486
}
468487

469488
llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
470489
if (2 <= llen && !memcmp(lhs + llen - 2, "/*", 2)) {
471490
if ((rhs && !is_glob) || (!rhs && fetch))
472491
goto invalid;
473492
is_glob = 1;
474-
llen -= 2;
493+
llen--;
475494
} else if (rhs && is_glob) {
476495
goto invalid;
477496
}
@@ -488,7 +507,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
488507
if (!*rs[i].src)
489508
; /* empty is ok */
490509
else {
491-
st = check_ref_format(rs[i].src);
510+
st = verify_refname(rs[i].src, is_glob);
492511
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
493512
goto invalid;
494513
}
@@ -503,7 +522,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
503522
} else if (!*rs[i].dst) {
504523
; /* ok */
505524
} else {
506-
st = check_ref_format(rs[i].dst);
525+
st = verify_refname(rs[i].dst, is_glob);
507526
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
508527
goto invalid;
509528
}
@@ -518,7 +537,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
518537
if (!*rs[i].src)
519538
; /* empty is ok */
520539
else if (is_glob) {
521-
st = check_ref_format(rs[i].src);
540+
st = verify_refname(rs[i].src, is_glob);
522541
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
523542
goto invalid;
524543
}
@@ -532,13 +551,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
532551
* - otherwise it must be a valid looking ref.
533552
*/
534553
if (!rs[i].dst) {
535-
st = check_ref_format(rs[i].src);
554+
st = verify_refname(rs[i].src, is_glob);
536555
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
537556
goto invalid;
538557
} else if (!*rs[i].dst) {
539558
goto invalid;
540559
} else {
541-
st = check_ref_format(rs[i].dst);
560+
st = verify_refname(rs[i].dst, is_glob);
542561
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
543562
goto invalid;
544563
}
@@ -687,8 +706,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
687706
if (!fetch->dst)
688707
continue;
689708
if (fetch->pattern) {
690-
if (!prefixcmp(needle, key) &&
691-
needle[strlen(key)] == '/') {
709+
if (!prefixcmp(needle, key)) {
692710
*result = xmalloc(strlen(value) +
693711
strlen(needle) -
694712
strlen(key) + 1);
@@ -966,9 +984,7 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
966984
continue;
967985
}
968986

969-
if (rs[i].pattern &&
970-
!prefixcmp(src->name, rs[i].src) &&
971-
src->name[strlen(rs[i].src)] == '/')
987+
if (rs[i].pattern && !prefixcmp(src->name, rs[i].src))
972988
return rs + i;
973989
}
974990
if (matching_refs != -1)

‎t/t5513-fetch-track.sh‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/bin/sh
2+
3+
test_description='fetch follows remote tracking branches correctly'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success setup '
8+
>file &&
9+
git add . &&
10+
test_tick &&
11+
git commit -m Initial &&
12+
git branch b-0 &&
13+
git branch b1 &&
14+
git branch b/one &&
15+
test_create_repo other &&
16+
(
17+
cd other &&
18+
git config remote.origin.url .. &&
19+
git config remote.origin.fetch "+refs/heads/b/*:refs/remotes/b/*"
20+
)
21+
'
22+
23+
test_expect_success fetch '
24+
(
25+
cd other && git fetch origin &&
26+
test "$(git for-each-ref --format="%(refname)")" = refs/remotes/b/one
27+
)
28+
'
29+
30+
test_done

0 commit comments

Comments
 (0)