From c091b3d415f95d3e4e62acddb084e211af46acbf Mon Sep 17 00:00:00 2001 From: Daniel Barkalow Date: Mon, 17 Mar 2008 22:05:23 -0400 Subject: [PATCH] Tighten refspec processing This changes the pattern matching code to not store the required final / before the *, and then to require each side to be a valid ref (or empty). In particular, any refspec that looks like it should be a pattern but doesn't quite meet the requirements will be found to be invalid as a fallback non-pattern. This was cherry picked from commit ef00d15 (Tighten refspec processing, 2008-03-17), and two fix-up commits 46220ca (remote.c: Fix overtight refspec validation, 2008-03-20) and 7d19da4 (refspec: allow colon-less wildcard "refs/category/*", 2008-03-25) squashed in. Signed-off-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- builtin-fetch.c | 3 +- builtin-send-pack.c | 2 +- remote.c | 176 ++++++++++++++++++++++++++++++++++++++++------------ remote.h | 3 +- t/t5511-refspec.sh | 72 +++++++++++++++++++++ 5 files changed, 214 insertions(+), 42 deletions(-) create mode 100755 t/t5511-refspec.sh diff --git a/builtin-fetch.c b/builtin-fetch.c index 9a6ddcec92..6fd006f48c 100644 --- a/builtin-fetch.c +++ b/builtin-fetch.c @@ -630,5 +630,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) signal(SIGINT, unlock_pack_on_signal); atexit(unlock_pack); - return do_fetch(transport, parse_ref_spec(ref_nr, refs), ref_nr); + return do_fetch(transport, + parse_fetch_refspec(ref_nr, refs), ref_nr); } diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 8afb1d0bca..98c54d9e28 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -536,7 +536,7 @@ static void verify_remote_names(int nr_heads, const char **heads) int i; for (i = 0; i < nr_heads; i++) { - const char *remote = strchr(heads[i], ':'); + const char *remote = strrchr(heads[i], ':'); remote = remote ? (remote + 1) : heads[i]; switch (check_ref_format(remote)) { diff --git a/remote.c b/remote.c index 6b56473f5b..95d65a07ef 100644 --- a/remote.c +++ b/remote.c @@ -305,42 +305,127 @@ static void read_config(void) git_config(handle_config); } -struct refspec *parse_ref_spec(int nr_refspec, const char **refspec) +static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch) { int i; + int st; struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec); + for (i = 0; i < nr_refspec; i++) { - const char *sp, *ep, *gp; - sp = refspec[i]; - if (*sp == '+') { + size_t llen, rlen; + int is_glob; + const char *lhs, *rhs; + + llen = rlen = is_glob = 0; + + lhs = refspec[i]; + if (*lhs == '+') { rs[i].force = 1; - sp++; + lhs++; + } + + rhs = strrchr(lhs, ':'); + if (rhs) { + rhs++; + rlen = strlen(rhs); + is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*")); + if (is_glob) + rlen -= 2; + rs[i].dst = xstrndup(rhs, rlen); } - gp = strchr(sp, '*'); - ep = strchr(sp, ':'); - if (gp && ep && gp > ep) - gp = NULL; - if (ep) { - if (ep[1]) { - const char *glob = strchr(ep + 1, '*'); - if (!glob) - gp = NULL; - if (gp) - rs[i].dst = xstrndup(ep + 1, - glob - ep - 1); - else - rs[i].dst = xstrdup(ep + 1); + + 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; + } else if (rhs && is_glob) { + goto invalid; + } + + rs[i].pattern = is_glob; + rs[i].src = xstrndup(lhs, llen); + + if (fetch) { + /* + * LHS + * - empty is allowed; it means HEAD. + * - otherwise it must be a valid looking ref. + */ + if (!*rs[i].src) + ; /* empty is ok */ + else { + st = check_ref_format(rs[i].src); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + goto invalid; + } + /* + * RHS + * - missing is ok, and is same as empty. + * - empty is ok; it means not to store. + * - otherwise it must be a valid looking ref. + */ + if (!rs[i].dst) { + ; /* ok */ + } else if (!*rs[i].dst) { + ; /* ok */ + } else { + st = check_ref_format(rs[i].dst); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + goto invalid; } } else { - ep = sp + strlen(sp); - } - if (gp) { - rs[i].pattern = 1; - ep = gp; + /* + * LHS + * - empty is allowed; it means delete. + * - when wildcarded, it must be a valid looking ref. + * - otherwise, it must be an extended SHA-1, but + * there is no existing way to validate this. + */ + if (!*rs[i].src) + ; /* empty is ok */ + else if (is_glob) { + st = check_ref_format(rs[i].src); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + goto invalid; + } + else + ; /* anything goes, for now */ + /* + * RHS + * - missing is allowed, but LHS then must be a + * valid looking ref. + * - empty is not allowed. + * - otherwise it must be a valid looking ref. + */ + if (!rs[i].dst) { + st = check_ref_format(rs[i].src); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + goto invalid; + } else if (!*rs[i].dst) { + goto invalid; + } else { + st = check_ref_format(rs[i].dst); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + goto invalid; + } } - rs[i].src = xstrndup(sp, ep - sp); } return rs; + + invalid: + die("Invalid refspec '%s'", refspec[i]); +} + +struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec) +{ + return parse_refspec_internal(nr_refspec, refspec, 1); +} + +struct refspec *parse_push_refspec(int nr_refspec, const char **refspec) +{ + return parse_refspec_internal(nr_refspec, refspec, 0); } static int valid_remote_nick(const char *name) @@ -371,8 +456,8 @@ struct remote *remote_get(const char *name) add_url(ret, name); if (!ret->url) return NULL; - ret->fetch = parse_ref_spec(ret->fetch_refspec_nr, ret->fetch_refspec); - ret->push = parse_ref_spec(ret->push_refspec_nr, ret->push_refspec); + ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec); + ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec); return ret; } @@ -385,11 +470,11 @@ int for_each_remote(each_remote_fn fn, void *priv) if (!r) continue; if (!r->fetch) - r->fetch = parse_ref_spec(r->fetch_refspec_nr, - r->fetch_refspec); + r->fetch = parse_fetch_refspec(r->fetch_refspec_nr, + r->fetch_refspec); if (!r->push) - r->push = parse_ref_spec(r->push_refspec_nr, - r->push_refspec); + r->push = parse_push_refspec(r->push_refspec_nr, + r->push_refspec); result = fn(r, priv); } return result; @@ -455,7 +540,8 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec) if (!fetch->dst) continue; if (fetch->pattern) { - if (!prefixcmp(needle, key)) { + if (!prefixcmp(needle, key) && + needle[strlen(key)] == '/') { *result = xmalloc(strlen(value) + strlen(needle) - strlen(key) + 1); @@ -695,7 +781,9 @@ static const struct refspec *check_pattern_match(const struct refspec *rs, { int i; for (i = 0; i < rs_nr; i++) { - if (rs[i].pattern && !prefixcmp(src->name, rs[i].src)) + if (rs[i].pattern && + !prefixcmp(src->name, rs[i].src) && + src->name[strlen(rs[i].src)] == '/') return rs + i; } return NULL; @@ -710,7 +798,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, int nr_refspec, const char **refspec, int flags) { struct refspec *rs = - parse_ref_spec(nr_refspec, (const char **) refspec); + parse_push_refspec(nr_refspec, (const char **) refspec); int send_all = flags & MATCH_REFS_ALL; int send_mirror = flags & MATCH_REFS_MIRROR; @@ -894,7 +982,7 @@ int get_fetch_map(const struct ref *remote_refs, struct ref ***tail, int missing_ok) { - struct ref *ref_map, *rm; + struct ref *ref_map, **rmp; if (refspec->pattern) { ref_map = get_expanded_map(remote_refs, refspec); @@ -911,10 +999,20 @@ int get_fetch_map(const struct ref *remote_refs, } } - for (rm = ref_map; rm; rm = rm->next) { - if (rm->peer_ref && check_ref_format(rm->peer_ref->name + 5)) - die("* refusing to create funny ref '%s' locally", - rm->peer_ref->name); + for (rmp = &ref_map; *rmp; ) { + if ((*rmp)->peer_ref) { + int st = check_ref_format((*rmp)->peer_ref->name + 5); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) { + struct ref *ignore = *rmp; + error("* Ignoring funny ref '%s' locally", + (*rmp)->peer_ref->name); + *rmp = (*rmp)->next; + free(ignore->peer_ref); + free(ignore); + continue; + } + } + rmp = &((*rmp)->next); } if (ref_map) diff --git a/remote.h b/remote.h index 86e036d610..6b8c32cc7b 100644 --- a/remote.h +++ b/remote.h @@ -63,7 +63,8 @@ void free_refs(struct ref *ref); */ void ref_remove_duplicates(struct ref *ref_map); -struct refspec *parse_ref_spec(int nr_refspec, const char **refspec); +struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec); +struct refspec *parse_push_refspec(int nr_refspec, const char **refspec); int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, int nr_refspec, const char **refspec, int all); diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh new file mode 100755 index 0000000000..670a8f1c99 --- /dev/null +++ b/t/t5511-refspec.sh @@ -0,0 +1,72 @@ +#!/bin/sh + +test_description='refspec parsing' + +. ./test-lib.sh + +test_refspec () { + + kind=$1 refspec=$2 expect=$3 + git config remote.frotz.url "." && + git config --remove-section remote.frotz && + git config remote.frotz.url "." && + git config "remote.frotz.$kind" "$refspec" && + if test "$expect" != invalid + then + title="$kind $refspec" + test='git ls-remote frotz' + else + title="$kind $refspec (invalid)" + test='test_must_fail git ls-remote frotz' + fi + test_expect_success "$title" "$test" +} + +test_refspec push '' invalid +test_refspec push ':' invalid + +test_refspec fetch '' +test_refspec fetch ':' + +test_refspec push 'refs/heads/*:refs/remotes/frotz/*' +test_refspec push 'refs/heads/*:refs/remotes/frotz' invalid +test_refspec push 'refs/heads:refs/remotes/frotz/*' invalid +test_refspec push 'refs/heads/master:refs/remotes/frotz/xyzzy' + + +# These have invalid LHS, but we do not have a formal "valid sha-1 +# expression syntax checker" so they are not checked with the current +# code. They will be caught downstream anyway, but we may want to +# have tighter check later... + +: test_refspec push 'refs/heads/master::refs/remotes/frotz/xyzzy' invalid +: test_refspec push 'refs/heads/maste :refs/remotes/frotz/xyzzy' invalid + +test_refspec fetch 'refs/heads/*:refs/remotes/frotz/*' +test_refspec fetch 'refs/heads/*:refs/remotes/frotz' invalid +test_refspec fetch 'refs/heads:refs/remotes/frotz/*' invalid +test_refspec fetch 'refs/heads/master:refs/remotes/frotz/xyzzy' +test_refspec fetch 'refs/heads/master::refs/remotes/frotz/xyzzy' invalid +test_refspec fetch 'refs/heads/maste :refs/remotes/frotz/xyzzy' invalid + +test_refspec push 'master~1:refs/remotes/frotz/backup' +test_refspec fetch 'master~1:refs/remotes/frotz/backup' invalid +test_refspec push 'HEAD~4:refs/remotes/frotz/new' +test_refspec fetch 'HEAD~4:refs/remotes/frotz/new' invalid + +test_refspec push 'HEAD' +test_refspec fetch 'HEAD' +test_refspec push 'refs/heads/ nitfol' invalid +test_refspec fetch 'refs/heads/ nitfol' invalid + +test_refspec push 'HEAD:' invalid +test_refspec fetch 'HEAD:' +test_refspec push 'refs/heads/ nitfol:' invalid +test_refspec fetch 'refs/heads/ nitfol:' invalid + +test_refspec push ':refs/remotes/frotz/deleteme' +test_refspec fetch ':refs/remotes/frotz/HEAD-to-me' +test_refspec push ':refs/remotes/frotz/delete me' invalid +test_refspec fetch ':refs/remotes/frotz/HEAD to me' invalid + +test_done -- 2.11.4.GIT