@matheusbsbs / matheus-bagliano / commits / b2a5627651
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 b2a56276512885f0c7d159543395769168c9f599
2 files changed +64 -18
remote.c
+34 -18
@@ -424,6 +424,28 @@ static void read_config(void)
424 alias_all_urls();
425 }
426
427 +/*
428 + * We need to make sure the tracking branches are well formed, but a
429 + * wildcard refspec in "struct refspec" must have a trailing slash. We
430 + * temporarily drop the trailing '/' while calling check_ref_format(),
431 + * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
432 + * error return is Ok for a wildcard refspec.
433 + */
434 +static int verify_refname(char *name, int is_glob)
435 +{
436 + int result, len = -1;
437 +
438 + if (is_glob) {
439 + len = strlen(name);
440 + assert(name[len - 1] == '/');
441 + name[len - 1] = '\0';
442 + }
443 + result = check_ref_format(name);
444 + if (is_glob)
445 + name[len - 1] = '/';
446 + return result;
447 +}
448 +
449 static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
450 {
451 int i;
@@ -431,11 +453,11 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
453 struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
454
455 for (i = 0; i < nr_refspec; i++) {
434 - size_t llen, rlen;
456 + size_t llen;
457 int is_glob;
458 const char *lhs, *rhs;
459
438 - llen = rlen = is_glob = 0;
460 + llen = is_glob = 0;
461
462 lhs = refspec[i];
463 if (*lhs == '+') {
@@ -455,12 +477,9 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
477 }
478
479 if (rhs) {
458 - rhs++;
459 - rlen = strlen(rhs);
480 + size_t rlen = strlen(++rhs);
481 is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*"));
461 - if (is_glob)
462 - rlen -= 2;
463 - rs[i].dst = xstrndup(rhs, rlen);
482 + rs[i].dst = xstrndup(rhs, rlen - is_glob);
483 }
484
485 llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
@@ -468,7 +487,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
487 if ((rhs && !is_glob) || (!rhs && fetch))
488 goto invalid;
489 is_glob = 1;
471 - llen -= 2;
490 + llen--;
491 } else if (rhs && is_glob) {
492 goto invalid;
493 }
@@ -485,7 +504,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
504 if (!*rs[i].src)
505 ; /* empty is ok */
506 else {
488 - st = check_ref_format(rs[i].src);
507 + st = verify_refname(rs[i].src, is_glob);
508 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
509 goto invalid;
510 }
@@ -500,7 +519,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
519 } else if (!*rs[i].dst) {
520 ; /* ok */
521 } else {
503 - st = check_ref_format(rs[i].dst);
522 + st = verify_refname(rs[i].dst, is_glob);
523 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
524 goto invalid;
525 }
@@ -515,7 +534,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
534 if (!*rs[i].src)
535 ; /* empty is ok */
536 else if (is_glob) {
518 - st = check_ref_format(rs[i].src);
537 + st = verify_refname(rs[i].src, is_glob);
538 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
539 goto invalid;
540 }
@@ -529,13 +548,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
548 * - otherwise it must be a valid looking ref.
549 */
550 if (!rs[i].dst) {
532 - st = check_ref_format(rs[i].src);
551 + st = verify_refname(rs[i].src, is_glob);
552 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
553 goto invalid;
554 } else if (!*rs[i].dst) {
555 goto invalid;
556 } else {
538 - st = check_ref_format(rs[i].dst);
557 + st = verify_refname(rs[i].dst, is_glob);
558 if (st && st != CHECK_REF_FORMAT_ONELEVEL)
559 goto invalid;
560 }
@@ -684,8 +703,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
703 if (!fetch->dst)
704 continue;
705 if (fetch->pattern) {
687 - if (!prefixcmp(needle, key) &&
688 - needle[strlen(key)] == '/') {
706 + if (!prefixcmp(needle, key)) {
707 *result = xmalloc(strlen(value) +
708 strlen(needle) -
709 strlen(key) + 1);
@@ -963,9 +981,7 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
981 continue;
982 }
983
966 - if (rs[i].pattern &&
967 - !prefixcmp(src->name, rs[i].src) &&
968 - src->name[strlen(rs[i].src)] == '/')
984 + if (rs[i].pattern && !prefixcmp(src->name, rs[i].src))
985 return rs + i;
986 }
987 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