Skip to content

Commit 1ab13eb

Browse files
gitstergitster
committed
Merge branch 'ka/want-ref-in-namespace'
"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 173368d + 53a66ec commit 1ab13eb

File tree

3 files changed

+192
-48
lines changed

3 files changed

+192
-48
lines changed

‎Documentation/config/transfer.txt‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,17 @@ 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.
55+
reference before it is matched against `transfer.hiderefs` patterns. In
56+
order to match refs before stripping, add a `^` in front of the ref name. If
57+
you combine `!` and `^`, `!` must be specified first.
58+
+
5659
For example, if `refs/heads/master` is specified in `transfer.hideRefs` and
5760
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.
61+
is omitted from the advertisements. If `uploadpack.allowRefInWant` is set,
62+
`upload-pack` will treat `want-ref refs/heads/master` in a protocol v2
63+
`fetch` command as if `refs/namespaces/foo/refs/heads/master` did not exist.
64+
`receive-pack`, on the other hand, will still advertise the object id the
65+
ref is pointing to without mentioning its name (a so-called ".have" line).
6266
+
6367
Even if you hide refs, a client may still be able to steal the target
6468
objects via the techniques described in the "SECURITY" section of the

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

Lines changed: 172 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,30 @@ 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" lines are read from stdin.
45+
#
46+
# Examples:
47+
#
48+
# write_fetch_command <<-EOF
49+
# want-ref refs/heads/main
50+
# have $(git rev-parse a)
51+
# EOF
52+
#
53+
# write_fetch_command <<-EOF
54+
# want $(git rev-parse b)
55+
# have $(git rev-parse a)
56+
# EOF
57+
#
58+
write_fetch_command () {
59+
write_command fetch &&
60+
echo "0001" &&
61+
echo "no-progress" &&
62+
cat &&
63+
echo "done" &&
64+
echo "0000"
65+
}
66+
4367
# c(o/foo) d(o/bar)
4468
# \ /
4569
# b e(baz) f(main)
@@ -77,15 +101,11 @@ test_expect_success 'config controls ref-in-want advertisement' '
77101
'
78102

79103
test_expect_success 'invalid want-ref line' '
80-
test-tool pkt-line pack >in <<-EOF &&
81-
$(write_command fetch)
82-
0001
83-
no-progress
104+
write_fetch_command >pkt <<-EOF &&
84105
want-ref refs/heads/non-existent
85-
done
86-
0000
87106
EOF
88107
108+
test-tool pkt-line pack <pkt >in &&
89109
test_must_fail test-tool serve-v2 --stateless-rpc 2>out <in &&
90110
grep "unknown ref" out
91111
'
@@ -97,16 +117,11 @@ test_expect_success 'basic want-ref' '
97117
EOF
98118
git rev-parse f >expected_commits &&
99119
100-
oid=$(git rev-parse a) &&
101-
test-tool pkt-line pack >in <<-EOF &&
102-
$(write_command fetch)
103-
0001
104-
no-progress
120+
write_fetch_command >pkt <<-EOF &&
105121
want-ref refs/heads/main
106-
have $oid
107-
done
108-
0000
122+
have $(git rev-parse a)
109123
EOF
124+
test-tool pkt-line pack <pkt >in &&
110125
111126
test-tool serve-v2 --stateless-rpc >out <in &&
112127
check_output
@@ -121,17 +136,12 @@ test_expect_success 'multiple want-ref lines' '
121136
EOF
122137
git rev-parse c d >expected_commits &&
123138
124-
oid=$(git rev-parse b) &&
125-
test-tool pkt-line pack >in <<-EOF &&
126-
$(write_command fetch)
127-
0001
128-
no-progress
139+
write_fetch_command >pkt <<-EOF &&
129140
want-ref refs/heads/o/foo
130141
want-ref refs/heads/o/bar
131-
have $oid
132-
done
133-
0000
142+
have $(git rev-parse b)
134143
EOF
144+
test-tool pkt-line pack <pkt >in &&
135145
136146
test-tool serve-v2 --stateless-rpc >out <in &&
137147
check_output
@@ -144,16 +154,12 @@ test_expect_success 'mix want and want-ref' '
144154
EOF
145155
git rev-parse e f >expected_commits &&
146156
147-
test-tool pkt-line pack >in <<-EOF &&
148-
$(write_command fetch)
149-
0001
150-
no-progress
157+
write_fetch_command >pkt <<-EOF &&
151158
want-ref refs/heads/main
152159
want $(git rev-parse e)
153160
have $(git rev-parse a)
154-
done
155-
0000
156161
EOF
162+
test-tool pkt-line pack <pkt >in &&
157163
158164
test-tool serve-v2 --stateless-rpc >out <in &&
159165
check_output
@@ -166,16 +172,11 @@ test_expect_success 'want-ref with ref we already have commit for' '
166172
EOF
167173
>expected_commits &&
168174
169-
oid=$(git rev-parse c) &&
170-
test-tool pkt-line pack >in <<-EOF &&
171-
$(write_command fetch)
172-
0001
173-
no-progress
175+
write_fetch_command >pkt <<-EOF &&
174176
want-ref refs/heads/o/foo
175-
have $oid
176-
done
177-
0000
177+
have $(git rev-parse c)
178178
EOF
179+
test-tool pkt-line pack <pkt >in &&
179180
180181
test-tool serve-v2 --stateless-rpc >out <in &&
181182
check_output
@@ -298,6 +299,141 @@ test_expect_success 'fetching with wildcard that matches multiple refs' '
298299
grep "want-ref refs/heads/o/bar" log
299300
'
300301

302+
REPO="$(pwd)/repo-ns"
303+
304+
test_expect_success 'setup namespaced repo' '
305+
(
306+
git init -b main "$REPO" &&
307+
cd "$REPO" &&
308+
test_commit a &&
309+
test_commit b &&
310+
git checkout a &&
311+
test_commit c &&
312+
git checkout a &&
313+
test_commit d &&
314+
git update-ref refs/heads/ns-no b &&
315+
git update-ref refs/namespaces/ns/refs/heads/ns-yes c &&
316+
git update-ref refs/namespaces/ns/refs/heads/hidden d
317+
) &&
318+
git -C "$REPO" config uploadpack.allowRefInWant true
319+
'
320+
321+
test_expect_success 'with namespace: want-ref is considered relative to namespace' '
322+
wanted_ref=refs/heads/ns-yes &&
323+
324+
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
325+
cat >expected_refs <<-EOF &&
326+
$oid $wanted_ref
327+
EOF
328+
cat >expected_commits <<-EOF &&
329+
$oid
330+
$(git -C "$REPO" rev-parse a)
331+
EOF
332+
333+
write_fetch_command >pkt <<-EOF &&
334+
want-ref $wanted_ref
335+
EOF
336+
test-tool pkt-line pack <pkt >in &&
337+
338+
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
339+
check_output
340+
'
341+
342+
test_expect_success 'with namespace: want-ref outside namespace is unknown' '
343+
wanted_ref=refs/heads/ns-no &&
344+
345+
write_fetch_command >pkt <<-EOF &&
346+
want-ref $wanted_ref
347+
EOF
348+
test-tool pkt-line pack <pkt >in &&
349+
350+
test_must_fail env GIT_NAMESPACE=ns \
351+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
352+
grep "unknown ref" out
353+
'
354+
355+
# Cross-check refs/heads/ns-no indeed exists
356+
test_expect_success 'without namespace: want-ref outside namespace succeeds' '
357+
wanted_ref=refs/heads/ns-no &&
358+
359+
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
360+
cat >expected_refs <<-EOF &&
361+
$oid $wanted_ref
362+
EOF
363+
cat >expected_commits <<-EOF &&
364+
$oid
365+
$(git -C "$REPO" rev-parse a)
366+
EOF
367+
368+
write_fetch_command >pkt <<-EOF &&
369+
want-ref $wanted_ref
370+
EOF
371+
test-tool pkt-line pack <pkt >in &&
372+
373+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
374+
check_output
375+
'
376+
377+
test_expect_success 'with namespace: hideRefs is matched, relative to namespace' '
378+
wanted_ref=refs/heads/hidden &&
379+
git -C "$REPO" config transfer.hideRefs $wanted_ref &&
380+
381+
write_fetch_command >pkt <<-EOF &&
382+
want-ref $wanted_ref
383+
EOF
384+
test-tool pkt-line pack <pkt >in &&
385+
386+
test_must_fail env GIT_NAMESPACE=ns \
387+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
388+
grep "unknown ref" out
389+
'
390+
391+
# Cross-check refs/heads/hidden indeed exists
392+
test_expect_success 'with namespace: want-ref succeeds if hideRefs is removed' '
393+
wanted_ref=refs/heads/hidden &&
394+
git -C "$REPO" config --unset transfer.hideRefs $wanted_ref &&
395+
396+
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
397+
cat >expected_refs <<-EOF &&
398+
$oid $wanted_ref
399+
EOF
400+
cat >expected_commits <<-EOF &&
401+
$oid
402+
$(git -C "$REPO" rev-parse a)
403+
EOF
404+
405+
write_fetch_command >pkt <<-EOF &&
406+
want-ref $wanted_ref
407+
EOF
408+
test-tool pkt-line pack <pkt >in &&
409+
410+
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
411+
check_output
412+
'
413+
414+
test_expect_success 'without namespace: relative hideRefs does not match' '
415+
wanted_ref=refs/namespaces/ns/refs/heads/hidden &&
416+
git -C "$REPO" config transfer.hideRefs refs/heads/hidden &&
417+
418+
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
419+
cat >expected_refs <<-EOF &&
420+
$oid $wanted_ref
421+
EOF
422+
cat >expected_commits <<-EOF &&
423+
$oid
424+
$(git -C "$REPO" rev-parse a)
425+
EOF
426+
427+
write_fetch_command >pkt <<-EOF &&
428+
want-ref $wanted_ref
429+
EOF
430+
test-tool pkt-line pack <pkt >in &&
431+
432+
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
433+
check_output
434+
'
435+
436+
301437
. "$TEST_DIRECTORY"/lib-httpd.sh
302438
start_httpd
303439

‎upload-pack.c‎

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

1426-
if (read_ref(arg, &oid)) {
1427-
packet_writer_error(writer, "unknown ref %s", arg);
1428-
die("unknown ref %s", arg);
1427+
strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
1428+
if (ref_is_hidden(refname_nons, refname.buf) ||
1429+
read_ref(refname.buf, &oid)) {
1430+
packet_writer_error(writer, "unknown ref %s", refname_nons);
1431+
die("unknown ref %s", refname_nons);
14291432
}
1433+
strbuf_release(&refname);
14301434

1431-
item = string_list_append(wanted_refs, arg);
1435+
item = string_list_append(wanted_refs, refname_nons);
14321436
item->util = oiddup(&oid);
14331437

1434-
o = parse_object_or_die(&oid, arg);
1438+
o = parse_object_or_die(&oid, refname_nons);
14351439
if (!(o->flags & WANTED)) {
14361440
o->flags |= WANTED;
14371441
add_object_array(o, NULL, want_obj);

0 commit comments

Comments
 (0)