Skip to content

Commit b9c52ac

Browse files
gitstergitster
committed
Merge branch 'ka/want-ref-in-namespace' into seen
"git upload-pack" which runs on the other side of "git fetch" forgot to take the ref namespaces into account when handling want-ref requests. * ka/want-ref-in-namespace: docs: clarify the interaction of transfer.hideRefs and namespaces upload-pack.c: treat want-ref relative to namespace t5730: introduce fetch command helper
2 parents fdb2e66 + 24a6e5b commit b9c52ac

File tree

3 files changed

+224
-47
lines changed

3 files changed

+224
-47
lines changed

‎Documentation/config/transfer.txt

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,16 @@ If you have multiple hideRefs values, later entries override earlier ones
5252
(and entries in more-specific config files override less-specific ones).
5353
+
5454
If a namespace is in use, the namespace prefix is stripped from each
55-
reference before it is matched against `transfer.hiderefs` patterns.
56-
For example, if `refs/heads/master` is specified in `transfer.hideRefs` and
57-
the current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master`
58-
is omitted from the advertisements but `refs/heads/master` and
59-
`refs/namespaces/bar/refs/heads/master` are still advertised as so-called
60-
"have" lines. In order to match refs before stripping, add a `^` in front of
61-
the ref name. If you combine `!` and `^`, `!` must be specified first.
55+
reference before it is matched against `transfer.hiderefs` patterns. For
56+
example, if `refs/heads/master` is specified in `transfer.hideRefs` and the
57+
current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master` is
58+
omitted from the advertisements. If `uploadpack.allowRefInWant` is set,
59+
`upload-pack` will treat `want-ref refs/heads/master` in a protocol v2
60+
`fetch` command as if `refs/heads/master` was unknown. Note, however, that
61+
`receive-pack` will still advertise the object id `refs/heads/master` is
62+
pointing to, but will conceil the name of the ref. In order to match refs
63+
before stripping, add a `^` in front of the ref name. If you combine `!` and
64+
`^`, `!` must be specified first.
6265
+
6366
Even if you hide refs, a client may still be able to steal the target
6467
objects via the techniques described in the "SECURITY" section of the

‎t/t5703-upload-pack-ref-in-want.sh

Lines changed: 203 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,54 @@ write_command () {
4040
fi
4141
}
4242

43+
# Write a complete fetch command to stdout, suitable for use with `test-tool
44+
# pkt-line`. "want-ref", "want", and "have" values can be given in this order,
45+
# with sections separated by "--".
46+
#
47+
# Examples:
48+
#
49+
# write_fetch_command refs/heads/main
50+
#
51+
# write_fetch_command \
52+
# refs/heads/main \
53+
# -- \
54+
# -- \
55+
# $(git rev-parse x)
56+
#
57+
# write_fetch_command \
58+
# --
59+
# $(git rev-parse a) \
60+
# --
61+
# $(git rev-parse b)
62+
write_fetch_command () {
63+
write_command fetch &&
64+
echo "0001" &&
65+
echo "no-progress" || return
66+
while :
67+
do
68+
case $# in 0) break ;; esac &&
69+
case "$1" in --) shift; break ;; esac &&
70+
echo "want-ref $1" &&
71+
shift || return
72+
done &&
73+
while :
74+
do
75+
case $# in 0) break ;; esac &&
76+
case "$1" in --) shift; break ;; esac &&
77+
echo "want $1" &&
78+
shift || return
79+
done &&
80+
while :
81+
do
82+
case $# in 0) break ;; esac &&
83+
case "$1" in --) shift; break ;; esac &&
84+
echo "have $1" &&
85+
shift || return
86+
done &&
87+
echo "done" &&
88+
echo "0000"
89+
}
90+
4391
# c(o/foo) d(o/bar)
4492
# \ /
4593
# b e(baz) f(main)
@@ -97,15 +145,13 @@ test_expect_success 'basic want-ref' '
97145
EOF
98146
git rev-parse f >expected_commits &&
99147
100-
oid=$(git rev-parse a) &&
101148
test-tool pkt-line pack >in <<-EOF &&
102-
$(write_command fetch)
103-
0001
104-
no-progress
105-
want-ref refs/heads/main
106-
have $oid
107-
done
108-
0000
149+
$(write_fetch_command \
150+
refs/heads/main \
151+
-- \
152+
-- \
153+
$(git rev-parse a) \
154+
)
109155
EOF
110156
111157
test-tool serve-v2 --stateless-rpc >out <in &&
@@ -121,16 +167,14 @@ test_expect_success 'multiple want-ref lines' '
121167
EOF
122168
git rev-parse c d >expected_commits &&
123169
124-
oid=$(git rev-parse b) &&
125170
test-tool pkt-line pack >in <<-EOF &&
126-
$(write_command fetch)
127-
0001
128-
no-progress
129-
want-ref refs/heads/o/foo
130-
want-ref refs/heads/o/bar
131-
have $oid
132-
done
133-
0000
171+
$(write_fetch_command \
172+
refs/heads/o/foo \
173+
refs/heads/o/bar \
174+
-- \
175+
-- \
176+
$(git rev-parse b) \
177+
)
134178
EOF
135179
136180
test-tool serve-v2 --stateless-rpc >out <in &&
@@ -145,14 +189,13 @@ test_expect_success 'mix want and want-ref' '
145189
git rev-parse e f >expected_commits &&
146190
147191
test-tool pkt-line pack >in <<-EOF &&
148-
$(write_command fetch)
149-
0001
150-
no-progress
151-
want-ref refs/heads/main
152-
want $(git rev-parse e)
153-
have $(git rev-parse a)
154-
done
155-
0000
192+
$(write_fetch_command \
193+
refs/heads/main \
194+
-- \
195+
$(git rev-parse e) \
196+
-- \
197+
$(git rev-parse a) \
198+
)
156199
EOF
157200
158201
test-tool serve-v2 --stateless-rpc >out <in &&
@@ -166,15 +209,13 @@ test_expect_success 'want-ref with ref we already have commit for' '
166209
EOF
167210
>expected_commits &&
168211
169-
oid=$(git rev-parse c) &&
170212
test-tool pkt-line pack >in <<-EOF &&
171-
$(write_command fetch)
172-
0001
173-
no-progress
174-
want-ref refs/heads/o/foo
175-
have $oid
176-
done
177-
0000
213+
$(write_fetch_command \
214+
refs/heads/o/foo \
215+
-- \
216+
-- \
217+
$(git rev-parse c) \
218+
)
178219
EOF
179220
180221
test-tool serve-v2 --stateless-rpc >out <in &&
@@ -298,6 +339,135 @@ test_expect_success 'fetching with wildcard that matches multiple refs' '
298339
grep "want-ref refs/heads/o/bar" log
299340
'
300341

342+
REPO="$(pwd)/repo-ns"
343+
344+
test_expect_success 'setup namespaced repo' '
345+
(
346+
git init -b main "$REPO" &&
347+
cd "$REPO" &&
348+
test_commit a &&
349+
test_commit b &&
350+
git checkout a &&
351+
test_commit c &&
352+
git checkout a &&
353+
test_commit d &&
354+
git update-ref refs/heads/ns-no b &&
355+
git update-ref refs/namespaces/ns/refs/heads/ns-yes c &&
356+
git update-ref refs/namespaces/ns/refs/heads/hidden d
357+
) &&
358+
git -C "$REPO" config uploadpack.allowRefInWant true
359+
'
360+
361+
test_expect_success 'with namespace: want-ref is considered relative to namespace' '
362+
wanted_ref=refs/heads/ns-yes &&
363+
364+
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
365+
cat >expected_refs <<-EOF &&
366+
$oid $wanted_ref
367+
EOF
368+
cat >expected_commits <<-EOF &&
369+
$oid
370+
$(git -C "$REPO" rev-parse a)
371+
EOF
372+
373+
test-tool pkt-line pack >in <<-EOF &&
374+
$(write_fetch_command $wanted_ref)
375+
EOF
376+
377+
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
378+
check_output
379+
'
380+
381+
test_expect_success 'with namespace: want-ref outside namespace is unknown' '
382+
wanted_ref=refs/heads/ns-no &&
383+
384+
test-tool pkt-line pack >in <<-EOF &&
385+
$(write_fetch_command $wanted_ref)
386+
EOF
387+
388+
test_must_fail env GIT_NAMESPACE=ns \
389+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
390+
grep "unknown ref" out
391+
'
392+
393+
# Cross-check refs/heads/ns-no indeed exists
394+
test_expect_success 'without namespace: want-ref outside namespace succeeds' '
395+
wanted_ref=refs/heads/ns-no &&
396+
397+
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
398+
cat >expected_refs <<-EOF &&
399+
$oid $wanted_ref
400+
EOF
401+
cat >expected_commits <<-EOF &&
402+
$oid
403+
$(git -C "$REPO" rev-parse a)
404+
EOF
405+
406+
test-tool pkt-line pack >in <<-EOF &&
407+
$(write_fetch_command $wanted_ref)
408+
EOF
409+
410+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
411+
check_output
412+
'
413+
414+
test_expect_success 'with namespace: hideRefs is matched, relative to namespace' '
415+
wanted_ref=refs/heads/hidden &&
416+
git -C "$REPO" config transfer.hideRefs $wanted_ref &&
417+
418+
test-tool pkt-line pack >in <<-EOF &&
419+
$(write_fetch_command $wanted_ref)
420+
EOF
421+
422+
test_must_fail env GIT_NAMESPACE=ns \
423+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
424+
grep "unknown ref" out
425+
'
426+
427+
# Cross-check refs/heads/hidden indeed exists
428+
test_expect_success 'with namespace: want-ref succeeds if hideRefs is removed' '
429+
wanted_ref=refs/heads/hidden &&
430+
git -C "$REPO" config --unset transfer.hideRefs $wanted_ref &&
431+
432+
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
433+
cat >expected_refs <<-EOF &&
434+
$oid $wanted_ref
435+
EOF
436+
cat >expected_commits <<-EOF &&
437+
$oid
438+
$(git -C "$REPO" rev-parse a)
439+
EOF
440+
441+
test-tool pkt-line pack >in <<-EOF &&
442+
$(write_fetch_command $wanted_ref)
443+
EOF
444+
445+
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
446+
check_output
447+
'
448+
449+
test_expect_success 'without namespace: relative hideRefs does not match' '
450+
wanted_ref=refs/namespaces/ns/refs/heads/hidden &&
451+
git -C "$REPO" config transfer.hideRefs refs/heads/hidden &&
452+
453+
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
454+
cat >expected_refs <<-EOF &&
455+
$oid $wanted_ref
456+
EOF
457+
cat >expected_commits <<-EOF &&
458+
$oid
459+
$(git -C "$REPO" rev-parse a)
460+
EOF
461+
462+
test-tool pkt-line pack >in <<-EOF &&
463+
$(write_fetch_command $wanted_ref)
464+
EOF
465+
466+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
467+
check_output
468+
'
469+
470+
301471
. "$TEST_DIRECTORY"/lib-httpd.sh
302472
start_httpd
303473

‎upload-pack.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,21 +1421,25 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
14211421
struct string_list *wanted_refs,
14221422
struct object_array *want_obj)
14231423
{
1424-
const char *arg;
1425-
if (skip_prefix(line, "want-ref ", &arg)) {
1424+
const char *refname_nons;
1425+
if (skip_prefix(line, "want-ref ", &refname_nons)) {
14261426
struct object_id oid;
14271427
struct string_list_item *item;
14281428
struct object *o;
1429+
struct strbuf refname = STRBUF_INIT;
14291430

1430-
if (read_ref(arg, &oid)) {
1431-
packet_writer_error(writer, "unknown ref %s", arg);
1432-
die("unknown ref %s", arg);
1431+
strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
1432+
if (ref_is_hidden(refname_nons, refname.buf) ||
1433+
read_ref(refname.buf, &oid)) {
1434+
packet_writer_error(writer, "unknown ref %s", refname_nons);
1435+
die("unknown ref %s", refname_nons);
14331436
}
1437+
strbuf_release(&refname);
14341438

1435-
item = string_list_append(wanted_refs, arg);
1439+
item = string_list_append(wanted_refs, refname_nons);
14361440
item->util = oiddup(&oid);
14371441

1438-
o = parse_object_or_die(&oid, arg);
1442+
o = parse_object_or_die(&oid, refname_nons);
14391443
if (!(o->flags & WANTED)) {
14401444
o->flags |= WANTED;
14411445
add_object_array(o, NULL, want_obj);

0 commit comments

Comments
 (0)