Skip to content

Commit

Permalink
make sure parsed wildcard refspec ends with slash
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Junio C Hamano committed Jul 27, 2008
1 parent ef115e2 commit 47c6ef1
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 18 deletions.
52 changes: 34 additions & 18 deletions remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -427,18 +427,40 @@ static void read_config(void)
alias_all_urls();
}

/*
* We need to make sure the tracking branches are well formed, but a
* wildcard refspec in "struct refspec" must have a trailing slash. We
* temporarily drop the trailing '/' while calling check_ref_format(),
* and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
* error return is Ok for a wildcard refspec.
*/
static int verify_refname(char *name, int is_glob)
{
int result, len = -1;

if (is_glob) {
len = strlen(name);
assert(name[len - 1] == '/');
name[len - 1] = '\0';
}
result = check_ref_format(name);
if (is_glob)
name[len - 1] = '/';
return result;
}

static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
{
int i;
int st;
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);

for (i = 0; i < nr_refspec; i++) {
size_t llen, rlen;
size_t llen;
int is_glob;
const char *lhs, *rhs;

llen = rlen = is_glob = 0;
llen = is_glob = 0;

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

if (rhs) {
rhs++;
rlen = strlen(rhs);
size_t rlen = strlen(++rhs);
is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*"));
if (is_glob)
rlen -= 2;
rs[i].dst = xstrndup(rhs, rlen);
rs[i].dst = xstrndup(rhs, rlen - is_glob);
}

llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
if (2 <= llen && !memcmp(lhs + llen - 2, "/*", 2)) {
if ((rhs && !is_glob) || (!rhs && fetch))
goto invalid;
is_glob = 1;
llen -= 2;
llen--;
} else if (rhs && is_glob) {
goto invalid;
}
Expand All @@ -488,7 +507,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else {
st = check_ref_format(rs[i].src);
st = verify_refname(rs[i].src, is_glob);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
goto invalid;
}
Expand All @@ -503,7 +522,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
} else if (!*rs[i].dst) {
; /* ok */
} else {
st = check_ref_format(rs[i].dst);
st = verify_refname(rs[i].dst, is_glob);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
goto invalid;
}
Expand All @@ -518,7 +537,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
st = check_ref_format(rs[i].src);
st = verify_refname(rs[i].src, is_glob);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
goto invalid;
}
Expand All @@ -532,13 +551,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
st = check_ref_format(rs[i].src);
st = verify_refname(rs[i].src, is_glob);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
st = check_ref_format(rs[i].dst);
st = verify_refname(rs[i].dst, is_glob);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
goto invalid;
}
Expand Down Expand Up @@ -687,8 +706,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
if (!fetch->dst)
continue;
if (fetch->pattern) {
if (!prefixcmp(needle, key) &&
needle[strlen(key)] == '/') {
if (!prefixcmp(needle, key)) {
*result = xmalloc(strlen(value) +
strlen(needle) -
strlen(key) + 1);
Expand Down Expand Up @@ -966,9 +984,7 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
continue;
}

if (rs[i].pattern &&
!prefixcmp(src->name, rs[i].src) &&
src->name[strlen(rs[i].src)] == '/')
if (rs[i].pattern && !prefixcmp(src->name, rs[i].src))
return rs + i;
}
if (matching_refs != -1)
Expand Down
30 changes: 30 additions & 0 deletions t/t5513-fetch-track.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/bin/sh

test_description='fetch follows remote tracking branches correctly'

. ./test-lib.sh

test_expect_success setup '
>file &&
git add . &&
test_tick &&
git commit -m Initial &&
git branch b-0 &&
git branch b1 &&
git branch b/one &&
test_create_repo other &&
(
cd other &&
git config remote.origin.url .. &&
git config remote.origin.fetch "+refs/heads/b/*:refs/remotes/b/*"
)
'

test_expect_success fetch '
(
cd other && git fetch origin &&
test "$(git for-each-ref --format="%(refname)")" = refs/remotes/b/one
)
'

test_done

0 comments on commit 47c6ef1

Please sign in to comment.