Skip to content

Commit

Permalink
Merge branch 'ka/want-ref-in-namespace'
Browse files Browse the repository at this point in the history 
"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
  • Loading branch information
@gitster
gitster committed Sep 10, 2021
2 parents 173368d + 53a66ec commit 1ab13eb
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 48 deletions.
14 changes: 9 additions & 5 deletions Documentation/config/transfer.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,17 @@ If you have multiple hideRefs values, later entries override earlier ones
(and entries in more-specific config files override less-specific ones).
+
If a namespace is in use, the namespace prefix is stripped from each
reference before it is matched against `transfer.hiderefs` patterns.
reference before it is matched against `transfer.hiderefs` patterns. In
order to match refs before stripping, add a `^` in front of the ref name. If
you combine `!` and `^`, `!` must be specified first.
+
For example, if `refs/heads/master` is specified in `transfer.hideRefs` and
the current namespace is `foo`, then `refs/namespaces/foo/refs/heads/master`
is omitted from the advertisements but `refs/heads/master` and
`refs/namespaces/bar/refs/heads/master` are still advertised as so-called
"have" lines. In order to match refs before stripping, add a `^` in front of
the ref name. If you combine `!` and `^`, `!` must be specified first.
is omitted from the advertisements. If `uploadpack.allowRefInWant` is set,
`upload-pack` will treat `want-ref refs/heads/master` in a protocol v2
`fetch` command as if `refs/namespaces/foo/refs/heads/master` did not exist.
`receive-pack`, on the other hand, will still advertise the object id the
ref is pointing to without mentioning its name (a so-called ".have" line).
+
Even if you hide refs, a client may still be able to steal the target
objects via the techniques described in the "SECURITY" section of the
Expand Down
208 changes: 172 additions & 36 deletions t/t5703-upload-pack-ref-in-want.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,30 @@ write_command () {
fi
}

# Write a complete fetch command to stdout, suitable for use with `test-tool
# pkt-line`. "want-ref", "want", and "have" lines are read from stdin.
#
# Examples:
#
# write_fetch_command <<-EOF
# want-ref refs/heads/main
# have $(git rev-parse a)
# EOF
#
# write_fetch_command <<-EOF
# want $(git rev-parse b)
# have $(git rev-parse a)
# EOF
#
write_fetch_command () {
write_command fetch &&
echo "0001" &&
echo "no-progress" &&
cat &&
echo "done" &&
echo "0000"
}

# c(o/foo) d(o/bar)
# \ /
# b e(baz) f(main)
Expand Down Expand Up @@ -77,15 +101,11 @@ test_expect_success 'config controls ref-in-want advertisement' '
'

test_expect_success 'invalid want-ref line' '
test-tool pkt-line pack >in <<-EOF &&
$(write_command fetch)
0001
no-progress
write_fetch_command >pkt <<-EOF &&
want-ref refs/heads/non-existent
done
0000
EOF
test-tool pkt-line pack <pkt >in &&
test_must_fail test-tool serve-v2 --stateless-rpc 2>out <in &&
grep "unknown ref" out
'
Expand All @@ -97,16 +117,11 @@ test_expect_success 'basic want-ref' '
EOF
git rev-parse f >expected_commits &&
oid=$(git rev-parse a) &&
test-tool pkt-line pack >in <<-EOF &&
$(write_command fetch)
0001
no-progress
write_fetch_command >pkt <<-EOF &&
want-ref refs/heads/main
have $oid
done
0000
have $(git rev-parse a)
EOF
test-tool pkt-line pack <pkt >in &&
test-tool serve-v2 --stateless-rpc >out <in &&
check_output
Expand All @@ -121,17 +136,12 @@ test_expect_success 'multiple want-ref lines' '
EOF
git rev-parse c d >expected_commits &&
oid=$(git rev-parse b) &&
test-tool pkt-line pack >in <<-EOF &&
$(write_command fetch)
0001
no-progress
write_fetch_command >pkt <<-EOF &&
want-ref refs/heads/o/foo
want-ref refs/heads/o/bar
have $oid
done
0000
have $(git rev-parse b)
EOF
test-tool pkt-line pack <pkt >in &&
test-tool serve-v2 --stateless-rpc >out <in &&
check_output
Expand All @@ -144,16 +154,12 @@ test_expect_success 'mix want and want-ref' '
EOF
git rev-parse e f >expected_commits &&
test-tool pkt-line pack >in <<-EOF &&
$(write_command fetch)
0001
no-progress
write_fetch_command >pkt <<-EOF &&
want-ref refs/heads/main
want $(git rev-parse e)
have $(git rev-parse a)
done
0000
EOF
test-tool pkt-line pack <pkt >in &&
test-tool serve-v2 --stateless-rpc >out <in &&
check_output
Expand All @@ -166,16 +172,11 @@ test_expect_success 'want-ref with ref we already have commit for' '
EOF
>expected_commits &&
oid=$(git rev-parse c) &&
test-tool pkt-line pack >in <<-EOF &&
$(write_command fetch)
0001
no-progress
write_fetch_command >pkt <<-EOF &&
want-ref refs/heads/o/foo
have $oid
done
0000
have $(git rev-parse c)
EOF
test-tool pkt-line pack <pkt >in &&
test-tool serve-v2 --stateless-rpc >out <in &&
check_output
Expand Down Expand Up @@ -298,6 +299,141 @@ test_expect_success 'fetching with wildcard that matches multiple refs' '
grep "want-ref refs/heads/o/bar" log
'

REPO="$(pwd)/repo-ns"

test_expect_success 'setup namespaced repo' '
(
git init -b main "$REPO" &&
cd "$REPO" &&
test_commit a &&
test_commit b &&
git checkout a &&
test_commit c &&
git checkout a &&
test_commit d &&
git update-ref refs/heads/ns-no b &&
git update-ref refs/namespaces/ns/refs/heads/ns-yes c &&
git update-ref refs/namespaces/ns/refs/heads/hidden d
) &&
git -C "$REPO" config uploadpack.allowRefInWant true
'

test_expect_success 'with namespace: want-ref is considered relative to namespace' '
wanted_ref=refs/heads/ns-yes &&
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
cat >expected_refs <<-EOF &&
$oid $wanted_ref
EOF
cat >expected_commits <<-EOF &&
$oid
$(git -C "$REPO" rev-parse a)
EOF
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
check_output
'

test_expect_success 'with namespace: want-ref outside namespace is unknown' '
wanted_ref=refs/heads/ns-no &&
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
test_must_fail env GIT_NAMESPACE=ns \
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
grep "unknown ref" out
'

# Cross-check refs/heads/ns-no indeed exists
test_expect_success 'without namespace: want-ref outside namespace succeeds' '
wanted_ref=refs/heads/ns-no &&
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
cat >expected_refs <<-EOF &&
$oid $wanted_ref
EOF
cat >expected_commits <<-EOF &&
$oid
$(git -C "$REPO" rev-parse a)
EOF
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
check_output
'

test_expect_success 'with namespace: hideRefs is matched, relative to namespace' '
wanted_ref=refs/heads/hidden &&
git -C "$REPO" config transfer.hideRefs $wanted_ref &&
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
test_must_fail env GIT_NAMESPACE=ns \
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
grep "unknown ref" out
'

# Cross-check refs/heads/hidden indeed exists
test_expect_success 'with namespace: want-ref succeeds if hideRefs is removed' '
wanted_ref=refs/heads/hidden &&
git -C "$REPO" config --unset transfer.hideRefs $wanted_ref &&
oid=$(git -C "$REPO" rev-parse "refs/namespaces/ns/$wanted_ref") &&
cat >expected_refs <<-EOF &&
$oid $wanted_ref
EOF
cat >expected_commits <<-EOF &&
$oid
$(git -C "$REPO" rev-parse a)
EOF
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
GIT_NAMESPACE=ns test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
check_output
'

test_expect_success 'without namespace: relative hideRefs does not match' '
wanted_ref=refs/namespaces/ns/refs/heads/hidden &&
git -C "$REPO" config transfer.hideRefs refs/heads/hidden &&
oid=$(git -C "$REPO" rev-parse $wanted_ref) &&
cat >expected_refs <<-EOF &&
$oid $wanted_ref
EOF
cat >expected_commits <<-EOF &&
$oid
$(git -C "$REPO" rev-parse a)
EOF
write_fetch_command >pkt <<-EOF &&
want-ref $wanted_ref
EOF
test-tool pkt-line pack <pkt >in &&
test-tool -C "$REPO" serve-v2 --stateless-rpc >out <in &&
check_output
'


. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd

Expand Down
18 changes: 11 additions & 7 deletions upload-pack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1417,21 +1417,25 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
struct string_list *wanted_refs,
struct object_array *want_obj)
{
const char *arg;
if (skip_prefix(line, "want-ref ", &arg)) {
const char *refname_nons;
if (skip_prefix(line, "want-ref ", &refname_nons)) {
struct object_id oid;
struct string_list_item *item;
struct object *o;
struct strbuf refname = STRBUF_INIT;

if (read_ref(arg, &oid)) {
packet_writer_error(writer, "unknown ref %s", arg);
die("unknown ref %s", arg);
strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
if (ref_is_hidden(refname_nons, refname.buf) ||
read_ref(refname.buf, &oid)) {
packet_writer_error(writer, "unknown ref %s", refname_nons);
die("unknown ref %s", refname_nons);
}
strbuf_release(&refname);

item = string_list_append(wanted_refs, arg);
item = string_list_append(wanted_refs, refname_nons);
item->util = oiddup(&oid);

o = parse_object_or_die(&oid, arg);
o = parse_object_or_die(&oid, refname_nons);
if (!(o->flags & WANTED)) {
o->flags |= WANTED;
add_object_array(o, NULL, want_obj);
Expand Down

0 comments on commit 1ab13eb

Please sign in to comment.