diff mbox series

[2/6] builtin/receive-pack: fix exclude patterns when announcing refs

Message ID 0317a5a7edeab29b7cad31d11140bd99f459144f.1725881266.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series refs/reftable: wire up exclude patterns | expand

Commit Message

Patrick Steinhardt Sept. 9, 2024, 11:31 a.m. UTC
In `write_head_info()` we announce references to the remote client. We
need to honor "transfer.hideRefs" here so that we do not announce any
references that the client shouldn't be able to learn about. This is
done via two separate mechanisms:

  - We hand over exclude patterns to the reference backend. We can only
    honor "plain" exclude patterns here that do not have prefixes with
    special meaning such as "^" or "!". Filtering down the references is
    handled by `hidden_refs_to_excludes()`.

  - In `show_ref_cb()` we perform a second check against hidden refs.
    For one this is done such that we can handle those special prefixes.
    And second, handling exclude patterns in ref backends is optional,
    so we also have to handle "normal" patterns.

The special-meaning "^" prefix alters whether a hidden ref applies to
the namespace-stripped reference name or the full name. So while we
would usually call `refs_for_each_namespaced_ref()` to only get those
references in the current namespace, we can't because we'd get the
already-rewritten reference names. Instead, we are forced to use
`refs_for_each_fullref_in()` and then manually strip away the namespace
prefix such that we have access to both names.

But this also means that we do not get namespace handling for exclude
patterns, which `refs_for_each_namespaced_ref()` brings for free. This
results in a bug because we potentially end up hiding away references
based on their namespaced name and not on the stripped name as we really
should be doing.

Fix this by manually rewriting the exclude patterns to their namespaced
variants.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/receive-pack.c           | 18 ++++++++++++++++--
 t/t5509-fetch-push-namespaces.sh |  8 ++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

Comments

karthik nayak Sept. 13, 2024, 11:50 a.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

[snip]

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 3f35140e489..478c62ca836 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -339,12 +339,26 @@ static void show_one_alternate_ref(const struct object_id *oid,
>  static void write_head_info(void)
>  {
>  	static struct oidset seen = OIDSET_INIT;
> +	struct strvec excludes_vector = STRVEC_INIT;
> +	const char **exclude_patterns;
> +
> +	/*
> +	 * We need access to the reference names both with and without their
> +	 * namespace and thus cannot use `refs_for_each_namespaced_ref()`. We
> +	 * thus have to adapt exclude patterns to carry the namespace prefix
> +	 * ourselves.
> +	 */
> +	exclude_patterns = get_namespaced_exclude_patterns(
> +		hidden_refs_to_excludes(&hidden_refs),
> +		get_git_namespace(), &excludes_vector);
>

Nit: So we do use it here, might be worth pointing out in the previous commit.

[snip]
diff mbox series

Patch

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 3f35140e489..478c62ca836 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -339,12 +339,26 @@  static void show_one_alternate_ref(const struct object_id *oid,
 static void write_head_info(void)
 {
 	static struct oidset seen = OIDSET_INIT;
+	struct strvec excludes_vector = STRVEC_INIT;
+	const char **exclude_patterns;
+
+	/*
+	 * We need access to the reference names both with and without their
+	 * namespace and thus cannot use `refs_for_each_namespaced_ref()`. We
+	 * thus have to adapt exclude patterns to carry the namespace prefix
+	 * ourselves.
+	 */
+	exclude_patterns = get_namespaced_exclude_patterns(
+		hidden_refs_to_excludes(&hidden_refs),
+		get_git_namespace(), &excludes_vector);
 
 	refs_for_each_fullref_in(get_main_ref_store(the_repository), "",
-				 hidden_refs_to_excludes(&hidden_refs),
-				 show_ref_cb, &seen);
+				 exclude_patterns, show_ref_cb, &seen);
 	for_each_alternate_ref(show_one_alternate_ref, &seen);
+
 	oidset_clear(&seen);
+	strvec_clear(&excludes_vector);
+
 	if (!sent_capabilities)
 		show_ref("capabilities^{}", null_oid());
 
diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
index 98e8352b6cc..f029ae0d286 100755
--- a/t/t5509-fetch-push-namespaces.sh
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -124,6 +124,14 @@  test_expect_success 'try to update a ref that is not hidden' '
 	git -C original push pushee-namespaced main
 '
 
+test_expect_success 'git-receive-pack(1) with transfer.hideRefs does not match unstripped refs during advertisement' '
+	git -C pushee update-ref refs/namespaces/namespace/refs/heads/foo/1 refs/namespaces/namespace/refs/heads/main &&
+	git -C pushee pack-refs --all &&
+	test_config -C pushee transfer.hideRefs refs/namespaces/namespace/refs/heads/foo &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C original push pushee-namespaced main &&
+	test_grep refs/heads/foo/1 trace
+'
+
 test_expect_success 'try to update a hidden full ref' '
 	test_config -C pushee transfer.hideRefs "^refs/namespaces/namespace/refs/heads/main" &&
 	test_must_fail git -C original push pushee-namespaced main