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>
Daniel Barkalow committed
Mar 17, 2008 at 22:05 UTC
c091b3d415f95d3e4e62acddb084e211af46acbf
5 files changed
+214
-42
builtin-fetch.c
+2
-1
@@ -630,5 +630,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
630
631
signal(SIGINT, unlock_pack_on_signal);
632
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);
635
}
builtin-send-pack.c
+1
-1
@@ -536,7 +536,7 @@ static void verify_remote_names(int nr_heads, const char **heads)
536
int i;
537
538
for (i = 0; i < nr_heads; i++) {
539
- const char *remote = strchr(heads[i], ':');
539
+ const char *remote = strrchr(heads[i], ':');
540
541
remote = remote ? (remote + 1) : heads[i];
542
switch (check_ref_format(remote)) {
remote.c
+137
-39
@@ -305,42 +305,127 @@ static void read_config(void)
305
git_config(handle_config);
306
}
307
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)
309
{
310
int i;
311
+ int st;
312
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
313
+
314
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 == '+') {
323
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);
335
}
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;
377
}
378
} 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
+ }
413
}
341
- rs[i].src = xstrndup(sp, ep - sp);
414
}
415
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);
429
}
430
431
static int valid_remote_nick(const char *name)
@@ -371,8 +456,8 @@ struct remote *remote_get(const char *name)
456
add_url(ret, name);
457
if (!ret->url)
458
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);
461
return ret;
462
}
463
@@ -385,11 +470,11 @@ int for_each_remote(each_remote_fn fn, void *priv)
470
if (!r)
471
continue;
472
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);
475
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);
478
result = fn(r, priv);
479
}
480
return result;
@@ -455,7 +540,8 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
540
if (!fetch->dst)
541
continue;
542
if (fetch->pattern) {
458
- if (!prefixcmp(needle, key)) {
543
+ if (!prefixcmp(needle, key) &&
544
+ needle[strlen(key)] == '/') {
545
*result = xmalloc(strlen(value) +
546
strlen(needle) -
547
strlen(key) + 1);
@@ -695,7 +781,9 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
781
{
782
int i;
783
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)] == '/')
787
return rs + i;
788
}
789
return NULL;
@@ -710,7 +798,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
798
int nr_refspec, const char **refspec, int flags)
799
{
800
struct refspec *rs =
713
- parse_ref_spec(nr_refspec, (const char **) refspec);
801
+ parse_push_refspec(nr_refspec, (const char **) refspec);
802
int send_all = flags & MATCH_REFS_ALL;
803
int send_mirror = flags & MATCH_REFS_MIRROR;
804
@@ -894,7 +982,7 @@ int get_fetch_map(const struct ref *remote_refs,
982
struct ref ***tail,
983
int missing_ok)
984
{
897
- struct ref *ref_map, *rm;
985
+ struct ref *ref_map, **rmp;
986
987
if (refspec->pattern) {
988
ref_map = get_expanded_map(remote_refs, refspec);
@@ -911,10 +999,20 @@ int get_fetch_map(const struct ref *remote_refs,
999
}
1000
}
1001
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);
1016
}
1017
1018
if (ref_map)
remote.h
+2
-1
@@ -63,7 +63,8 @@ void free_refs(struct ref *ref);
63
*/
64
void ref_remove_duplicates(struct ref *ref_map);
65
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);
68
69
int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
70
int nr_refspec, const char **refspec, int all);
t/t5511-refspec.sh
new
+72
@@ -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