Skip to content

Commit c091b3d

Browse files
iabervongitster
iabervon
authored and
gitster
committed
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 <barkalow@iabervon.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 71a5099 commit c091b3d

File tree

5 files changed

+214
-42
lines changed

5 files changed

+214
-42
lines changed

‎builtin-fetch.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,5 +630,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
630630

631631
signal(SIGINT, unlock_pack_on_signal);
632632
atexit(unlock_pack);
633-
return do_fetch(transport, parse_ref_spec(ref_nr, refs), ref_nr);
633+
return do_fetch(transport,
634+
parse_fetch_refspec(ref_nr, refs), ref_nr);
634635
}

‎builtin-send-pack.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ static void verify_remote_names(int nr_heads, const char **heads)
536536
int i;
537537

538538
for (i = 0; i < nr_heads; i++) {
539-
const char *remote = strchr(heads[i], ':');
539+
const char *remote = strrchr(heads[i], ':');
540540

541541
remote = remote ? (remote + 1) : heads[i];
542542
switch (check_ref_format(remote)) {

‎remote.c‎

Lines changed: 137 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -305,42 +305,127 @@ static void read_config(void)
305305
git_config(handle_config);
306306
}
307307

308-
struct refspec *parse_ref_spec(int nr_refspec, const char **refspec)
308+
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch)
309309
{
310310
int i;
311+
int st;
311312
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
313+
312314
for (i = 0; i < nr_refspec; i++) {
313-
const char *sp, *ep, *gp;
314-
sp = refspec[i];
315-
if (*sp == '+') {
315+
size_t llen, rlen;
316+
int is_glob;
317+
const char *lhs, *rhs;
318+
319+
llen = rlen = is_glob = 0;
320+
321+
lhs = refspec[i];
322+
if (*lhs == '+') {
316323
rs[i].force = 1;
317-
sp++;
324+
lhs++;
325+
}
326+
327+
rhs = strrchr(lhs, ':');
328+
if (rhs) {
329+
rhs++;
330+
rlen = strlen(rhs);
331+
is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*"));
332+
if (is_glob)
333+
rlen -= 2;
334+
rs[i].dst = xstrndup(rhs, rlen);
318335
}
319-
gp = strchr(sp, '*');
320-
ep = strchr(sp, ':');
321-
if (gp && ep && gp > ep)
322-
gp = NULL;
323-
if (ep) {
324-
if (ep[1]) {
325-
const char *glob = strchr(ep + 1, '*');
326-
if (!glob)
327-
gp = NULL;
328-
if (gp)
329-
rs[i].dst = xstrndup(ep + 1,
330-
glob - ep - 1);
331-
else
332-
rs[i].dst = xstrdup(ep + 1);
336+
337+
llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));
338+
if (2 <= llen && !memcmp(lhs + llen - 2, "/*", 2)) {
339+
if ((rhs && !is_glob) || (!rhs && fetch))
340+
goto invalid;
341+
is_glob = 1;
342+
llen -= 2;
343+
} else if (rhs && is_glob) {
344+
goto invalid;
345+
}
346+
347+
rs[i].pattern = is_glob;
348+
rs[i].src = xstrndup(lhs, llen);
349+
350+
if (fetch) {
351+
/*
352+
* LHS
353+
* - empty is allowed; it means HEAD.
354+
* - otherwise it must be a valid looking ref.
355+
*/
356+
if (!*rs[i].src)
357+
; /* empty is ok */
358+
else {
359+
st = check_ref_format(rs[i].src);
360+
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
361+
goto invalid;
362+
}
363+
/*
364+
* RHS
365+
* - missing is ok, and is same as empty.
366+
* - empty is ok; it means not to store.
367+
* - otherwise it must be a valid looking ref.
368+
*/
369+
if (!rs[i].dst) {
370+
; /* ok */
371+
} else if (!*rs[i].dst) {
372+
; /* ok */
373+
} else {
374+
st = check_ref_format(rs[i].dst);
375+
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
376+
goto invalid;
333377
}
334378
} else {
335-
ep = sp + strlen(sp);
336-
}
337-
if (gp) {
338-
rs[i].pattern = 1;
339-
ep = gp;
379+
/*
380+
* LHS
381+
* - empty is allowed; it means delete.
382+
* - when wildcarded, it must be a valid looking ref.
383+
* - otherwise, it must be an extended SHA-1, but
384+
* there is no existing way to validate this.
385+
*/
386+
if (!*rs[i].src)
387+
; /* empty is ok */
388+
else if (is_glob) {
389+
st = check_ref_format(rs[i].src);
390+
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
391+
goto invalid;
392+
}
393+
else
394+
; /* anything goes, for now */
395+
/*
396+
* RHS
397+
* - missing is allowed, but LHS then must be a
398+
* valid looking ref.
399+
* - empty is not allowed.
400+
* - otherwise it must be a valid looking ref.
401+
*/
402+
if (!rs[i].dst) {
403+
st = check_ref_format(rs[i].src);
404+
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
405+
goto invalid;
406+
} else if (!*rs[i].dst) {
407+
goto invalid;
408+
} else {
409+
st = check_ref_format(rs[i].dst);
410+
if (st && st != CHECK_REF_FORMAT_ONELEVEL)
411+
goto invalid;
412+
}
340413
}
341-
rs[i].src = xstrndup(sp, ep - sp);
342414
}
343415
return rs;
416+
417+
invalid:
418+
die("Invalid refspec '%s'", refspec[i]);
419+
}
420+
421+
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec)
422+
{
423+
return parse_refspec_internal(nr_refspec, refspec, 1);
424+
}
425+
426+
struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
427+
{
428+
return parse_refspec_internal(nr_refspec, refspec, 0);
344429
}
345430

346431
static int valid_remote_nick(const char *name)
@@ -371,8 +456,8 @@ struct remote *remote_get(const char *name)
371456
add_url(ret, name);
372457
if (!ret->url)
373458
return NULL;
374-
ret->fetch = parse_ref_spec(ret->fetch_refspec_nr, ret->fetch_refspec);
375-
ret->push = parse_ref_spec(ret->push_refspec_nr, ret->push_refspec);
459+
ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec);
460+
ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec);
376461
return ret;
377462
}
378463

@@ -385,11 +470,11 @@ int for_each_remote(each_remote_fn fn, void *priv)
385470
if (!r)
386471
continue;
387472
if (!r->fetch)
388-
r->fetch = parse_ref_spec(r->fetch_refspec_nr,
389-
r->fetch_refspec);
473+
r->fetch = parse_fetch_refspec(r->fetch_refspec_nr,
474+
r->fetch_refspec);
390475
if (!r->push)
391-
r->push = parse_ref_spec(r->push_refspec_nr,
392-
r->push_refspec);
476+
r->push = parse_push_refspec(r->push_refspec_nr,
477+
r->push_refspec);
393478
result = fn(r, priv);
394479
}
395480
return result;
@@ -455,7 +540,8 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
455540
if (!fetch->dst)
456541
continue;
457542
if (fetch->pattern) {
458-
if (!prefixcmp(needle, key)) {
543+
if (!prefixcmp(needle, key) &&
544+
needle[strlen(key)] == '/') {
459545
*result = xmalloc(strlen(value) +
460546
strlen(needle) -
461547
strlen(key) + 1);
@@ -695,7 +781,9 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
695781
{
696782
int i;
697783
for (i = 0; i < rs_nr; i++) {
698-
if (rs[i].pattern && !prefixcmp(src->name, rs[i].src))
784+
if (rs[i].pattern &&
785+
!prefixcmp(src->name, rs[i].src) &&
786+
src->name[strlen(rs[i].src)] == '/')
699787
return rs + i;
700788
}
701789
return NULL;
@@ -710,7 +798,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
710798
int nr_refspec, const char **refspec, int flags)
711799
{
712800
struct refspec *rs =
713-
parse_ref_spec(nr_refspec, (const char **) refspec);
801+
parse_push_refspec(nr_refspec, (const char **) refspec);
714802
int send_all = flags & MATCH_REFS_ALL;
715803
int send_mirror = flags & MATCH_REFS_MIRROR;
716804

@@ -894,7 +982,7 @@ int get_fetch_map(const struct ref *remote_refs,
894982
struct ref ***tail,
895983
int missing_ok)
896984
{
897-
struct ref *ref_map, *rm;
985+
struct ref *ref_map, **rmp;
898986

899987
if (refspec->pattern) {
900988
ref_map = get_expanded_map(remote_refs, refspec);
@@ -911,10 +999,20 @@ int get_fetch_map(const struct ref *remote_refs,
911999
}
9121000
}
9131001

914-
for (rm = ref_map; rm; rm = rm->next) {
915-
if (rm->peer_ref && check_ref_format(rm->peer_ref->name + 5))
916-
die("* refusing to create funny ref '%s' locally",
917-
rm->peer_ref->name);
1002+
for (rmp = &ref_map; *rmp; ) {
1003+
if ((*rmp)->peer_ref) {
1004+
int st = check_ref_format((*rmp)->peer_ref->name + 5);
1005+
if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
1006+
struct ref *ignore = *rmp;
1007+
error("* Ignoring funny ref '%s' locally",
1008+
(*rmp)->peer_ref->name);
1009+
*rmp = (*rmp)->next;
1010+
free(ignore->peer_ref);
1011+
free(ignore);
1012+
continue;
1013+
}
1014+
}
1015+
rmp = &((*rmp)->next);
9181016
}
9191017

9201018
if (ref_map)

‎remote.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ void free_refs(struct ref *ref);
6363
*/
6464
void ref_remove_duplicates(struct ref *ref_map);
6565

66-
struct refspec *parse_ref_spec(int nr_refspec, const char **refspec);
66+
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
67+
struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);
6768

6869
int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
6970
int nr_refspec, const char **refspec, int all);

‎t/t5511-refspec.sh‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#!/bin/sh
2+
3+
test_description='refspec parsing'
4+
5+
. ./test-lib.sh
6+
7+
test_refspec () {
8+
9+
kind=$1 refspec=$2 expect=$3
10+
git config remote.frotz.url "." &&
11+
git config --remove-section remote.frotz &&
12+
git config remote.frotz.url "." &&
13+
git config "remote.frotz.$kind" "$refspec" &&
14+
if test "$expect" != invalid
15+
then
16+
title="$kind $refspec"
17+
test='git ls-remote frotz'
18+
else
19+
title="$kind $refspec (invalid)"
20+
test='test_must_fail git ls-remote frotz'
21+
fi
22+
test_expect_success "$title" "$test"
23+
}
24+
25+
test_refspec push '' invalid
26+
test_refspec push ':' invalid
27+
28+
test_refspec fetch ''
29+
test_refspec fetch ':'
30+
31+
test_refspec push 'refs/heads/*:refs/remotes/frotz/*'
32+
test_refspec push 'refs/heads/*:refs/remotes/frotz' invalid
33+
test_refspec push 'refs/heads:refs/remotes/frotz/*' invalid
34+
test_refspec push 'refs/heads/master:refs/remotes/frotz/xyzzy'
35+
36+
37+
# These have invalid LHS, but we do not have a formal "valid sha-1
38+
# expression syntax checker" so they are not checked with the current
39+
# code. They will be caught downstream anyway, but we may want to
40+
# have tighter check later...
41+
42+
: test_refspec push 'refs/heads/master::refs/remotes/frotz/xyzzy' invalid
43+
: test_refspec push 'refs/heads/maste :refs/remotes/frotz/xyzzy' invalid
44+
45+
test_refspec fetch 'refs/heads/*:refs/remotes/frotz/*'
46+
test_refspec fetch 'refs/heads/*:refs/remotes/frotz' invalid
47+
test_refspec fetch 'refs/heads:refs/remotes/frotz/*' invalid
48+
test_refspec fetch 'refs/heads/master:refs/remotes/frotz/xyzzy'
49+
test_refspec fetch 'refs/heads/master::refs/remotes/frotz/xyzzy' invalid
50+
test_refspec fetch 'refs/heads/maste :refs/remotes/frotz/xyzzy' invalid
51+
52+
test_refspec push 'master~1:refs/remotes/frotz/backup'
53+
test_refspec fetch 'master~1:refs/remotes/frotz/backup' invalid
54+
test_refspec push 'HEAD~4:refs/remotes/frotz/new'
55+
test_refspec fetch 'HEAD~4:refs/remotes/frotz/new' invalid
56+
57+
test_refspec push 'HEAD'
58+
test_refspec fetch 'HEAD'
59+
test_refspec push 'refs/heads/ nitfol' invalid
60+
test_refspec fetch 'refs/heads/ nitfol' invalid
61+
62+
test_refspec push 'HEAD:' invalid
63+
test_refspec fetch 'HEAD:'
64+
test_refspec push 'refs/heads/ nitfol:' invalid
65+
test_refspec fetch 'refs/heads/ nitfol:' invalid
66+
67+
test_refspec push ':refs/remotes/frotz/deleteme'
68+
test_refspec fetch ':refs/remotes/frotz/HEAD-to-me'
69+
test_refspec push ':refs/remotes/frotz/delete me' invalid
70+
test_refspec fetch ':refs/remotes/frotz/HEAD to me' invalid
71+
72+
test_done

0 commit comments

Comments
 (0)