diff --git a/builtin-fetch.c b/builtin-fetch.c
index b2b9935..a11548c 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -652,5 +652,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 930e0fb..bb9c33a 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -537,7 +537,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 9700a33..4117bfc 100644
--- a/remote.c
+++ b/remote.c
@@ -393,58 +393,98 @@ static void read_config(void)
alias_all_urls();
}
-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++;
}
- gp = strstr(sp, "/*");
- ep = strchr(sp, ':');
- if (gp && ep && gp > ep)
- gp = NULL;
- if (ep) {
- if (ep[1]) {
- const char *glob = strstr(ep + 1, "/*");
- if (glob && glob[2])
- glob = NULL;
- if (!glob)
- gp = NULL;
- if (gp)
- rs[i].dst = xstrndup(ep + 1,
- glob - ep - 1);
- else
- rs[i].dst = xstrdup(ep + 1);
- }
- } else {
- ep = sp + strlen(sp);
+
+ rhs = strrchr(lhs, ':');
+ if (rhs) {
+ rhs++;
+ rlen = strlen(rhs);
+ is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*"));
+ rs[i].dst = xstrndup(rhs, rlen - is_glob * 2);
}
- if (gp && gp + 2 != ep)
- gp = NULL;
- if (gp) {
- rs[i].pattern = 1;
- ep = gp;
+
+ llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
+ if (is_glob != (2 <= llen && !memcmp(lhs + llen - 2, "/*", 2)))
+ goto invalid;
+
+ if (is_glob) {
+ llen -= 2;
+ rlen -= 2;
}
- rs[i].src = xstrndup(sp, ep - sp);
+ rs[i].pattern = is_glob;
+ rs[i].src = xstrndup(lhs, llen);
- if (*rs[i].src) {
+ /*
+ * Do we want to validate LHS?
+ *
+ * - Fetch refspec must have LHS that names a valid
+ * looking ref (unless it is empty, which defaults to
+ * HEAD).
+ *
+ * - Push refspec "LHS" without ":RHS" is a synonym for
+ * "LHS:LHS", and LHS must be a valid looking ref
+ * (cannot be empty).
+ *
+ * - Push refspec "LHS:RHS" that is a glob must be
+ * a valid looking ref (unless it is empty to remove)
+ *
+ * Hence we check non-empty LHS for fetch, and
+ * colonless or glob LHS for push here.
+ *
+ * Note that push refspec "LHS:RHS" that is not a glob
+ * can have any valid extended SHA-1 expression (or
+ * "empty" for removal) in LHS, and we cannot check
+ * for error until it actually gets used.
+ */
+ if (fetch ? (*rs[i].src) : (!rhs || is_glob)) {
st = check_ref_format(rs[i].src);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- die("Invalid refspec '%s'", refspec[i]);
+ goto invalid;
}
- if (rs[i].dst && *rs[i].dst) {
+
+ /*
+ * RHS: both push destination and fetch tracking ref
+ * must name a valid looking ref, but in fetch, colon
+ * followed by emptiness is the same as not having the
+ * colon nor RHS.
+ */
+ if (rs[i].dst && (!fetch || *rs[i].dst)) {
st = check_ref_format(rs[i].dst);
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- die("Invalid refspec '%s'", refspec[i]);
+ goto invalid;
}
}
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)@@ -475,8 +515,8 @@ struct remote *remote_get(const char *name)
add_url_alias(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;
}
@@ -489,11 +529,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;
@@ -824,7 +864,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;
diff --git a/remote.h b/remote.h
index f1dedf1..7e9ae79 100644
--- a/remote.h
+++ b/remote.h
@@ -67,7 +67,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 0000000..7360311
--- /dev/null
+++ b/t/t5511-refspec.sh
@@ -0,0 +1,67 @@
+#!/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 '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--
1.5.5.rc0.139.g9315d