[v2] ls-refs: filter refs using namespace-stripped name

Commit Message

Jonathan Tan Jan. 17, 2019, 11:33 p.m. UTC

If a user fetches refs/heads/master from a repo with namespace "ns", the
remote is expected to (1) not send the real refs/heads/master, and (2)
send refs/namespaces/ns/refs/heads/master with the name
refs/heads/master. (1) indeed happens now, but not (2) - Git only sends
refs that have the user-given prefix, but it checks them against the
full name of the ref (the one starting with refs/namespaces), and not
the namespace-stripped one.

This is demonstrated by the patch in the test. Currently, it results in
"fatal: couldn't find remote ref refs/heads/master" despite both
unnamespaced and namespaced master being present. With the code change,
it produces the expected result.

Check the ref prefixes against the namespace-stripped name.

This bug was discovered through applying patches [1] that override
protocol.version to 2 in repositories when running tests, allowing us to
notice differences in behavior across different protocol versions.

[1] https://public-inbox.org/git/[e-mail]/

Signed-off-by: Jonathan Tan <[e-mail]>
---
Changes from v1:
 - updated commit message to explain from the end user viewpoint
 - wrote the test using end-user-facing Git commands

Thanks, Junio, for your review.

> OK, so "ls-refs: filter based on non-namespace name" (in the title) is a
> means to the objective 'ls-refs: make sure it honors namespaces"
> which is a bugfix?

Yes and no. I've updated the commit message to explain that it honors
namespaces in that it doesn't send the wrong refs, but it doesn't honor
them in that it doesn't send the right refs.

> The new test peeks at the protocol level, but wouldn't we be able to
> see the breakage by running ls-remote or something and observing its
> result as well, or is the bug only observable with test-tool and not
> triggerable by end-user facing git commands?

ls-remote wouldn't work as its filtering is incompatible with ref-prefix
(see 631f0f8c4b ("ls-remote: do not send ref prefixes for patterns",
2018-10-31)), but it can be observed with fetch. I've replaced the test
with one that uses fetch instead.
---
 ls-refs.c              |  2 +-
 t/t5702-protocol-v2.sh | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

Patch

diff --git a/ls-refs.c b/ls-refs.c
index a06f12eca8..7782bb054b 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -40,7 +40,7 @@  static int send_ref(const char *refname, const struct object_id *oid,
 	const char *refname_nons = strip_namespace(refname);
 	struct strbuf refline = STRBUF_INIT;
 
-	if (!ref_match(&data->prefixes, refname))
+	if (!ref_match(&data->prefixes, refname_nons))
 		return 0;
 
 	strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons);
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 0f2b09ebb8..3d802aa587 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -514,6 +514,27 @@  test_expect_success 'fetch with http:// using protocol v2' '
 	grep "git< version 2" log
 '
 
+test_expect_success 'fetch from namespaced repo respects namespaces' '
+	test_when_finished "rm -f log" &&
+
+	git init "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" &&
+	test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" one &&
+	test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" two &&
+	git -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" \
+		update-ref refs/namespaces/ns/refs/heads/master one &&
+
+	GIT_TRACE_PACKET="$(pwd)/log" git -C http_child -c protocol.version=2 \
+		fetch "$HTTPD_URL/smart_namespace/nsrepo" \
+		refs/heads/master:refs/heads/theirs &&
+
+	# Server responded using protocol v2
+	grep "fetch< version 2" log &&
+
+	git -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" rev-parse one >expect &&
+	git -C http_child rev-parse theirs >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'push with http:// and a config of v2 does not request v2' '
 	test_when_finished "rm -f log" &&
 	# Till v2 for push is designed, make sure that if a client has