@matheusbsbs / matheus-bagliano / commits / 47c6ef1c8d
Fork 0

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>

Junio C Hamano committed Jul 26, 2008 at 23:15 UTC 47c6ef1c8def9a20b4ff40825456b45f5e63b51f
2 files changed +64 -18
remote.c
+34 -18
@@ -427,6 +427,28 @@ static void read_config(void)
427 alias_all_urls();
428 }
429
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 +
452 static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
453 {
454 int i;
@@ -434,11 +456,11 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
456 struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
457
458 for (i = 0; i < nr_refspec; i++) {
437 - size_t llen, rlen;
459 + size_t llen;
460 int is_glob;
461 const char *lhs, *rhs;
462
441 - llen = rlen = is_glob = 0;
463 + llen = is_glob = 0;
464
465 lhs = refspec[i];
466 if (*lhs == '+') {
@@ -458,12 +480,9 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
480 }
481
482 if (rhs) {
461 - rhs++;
462 - rlen = strlen(rhs);
483 + size_t rlen = strlen(++rhs);
484 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);
486 }
487
488 llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
@@ -471,7 +490,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
490 if ((rhs && !is_glob) || (!rhs && fetch))
491 goto invalid;
492 is_glob = 1;
474 - llen -= 2;
493 + llen--;
494 } else if (rhs && is_glob) {
495 goto invalid;
496 }
@@ -488,7 +507,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
507 if (!*rs[i].src)
508 ; /* empty is ok */
509 else {
491 - st = check_ref_format(rs[i].src);
510 + st = verify_refname(rs[i].src, is_glob);
511 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
512 goto invalid;
513 }
@@ -503,7 +522,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
522 } else if (!*rs[i].dst) {
523 ; /* ok */
524 } else {
506 - st = check_ref_format(rs[i].dst);
525 + st = verify_refname(rs[i].dst, is_glob);
526 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
527 goto invalid;
528 }
@@ -518,7 +537,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
537 if (!*rs[i].src)
538 ; /* empty is ok */
539 else if (is_glob) {
521 - st = check_ref_format(rs[i].src);
540 + st = verify_refname(rs[i].src, is_glob);
541 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
542 goto invalid;
543 }
@@ -532,13 +551,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
551 * - otherwise it must be a valid looking ref.
552 */
553 if (!rs[i].dst) {
535 - st = check_ref_format(rs[i].src);
554 + st = verify_refname(rs[i].src, is_glob);
555 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
556 goto invalid;
557 } else if (!*rs[i].dst) {
558 goto invalid;
559 } else {
541 - st = check_ref_format(rs[i].dst);
560 + st = verify_refname(rs[i].dst, is_glob);
561 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
562 goto invalid;
563 }
@@ -687,8 +706,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
706 if (!fetch->dst)
707 continue;
708 if (fetch->pattern) {
690 - if (!prefixcmp(needle, key) &&
691 - needle[strlen(key)] == '/') {
709 + if (!prefixcmp(needle, key)) {
710 *result = xmalloc(strlen(value) +
711 strlen(needle) -
712 strlen(key) + 1);
@@ -966,9 +984,7 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
984 continue;
985 }
986
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))
988 return rs + i;
989 }
990 if (matching_refs != -1)
t/t5513-fetch-track.sh new
+30
@@ -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