All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 3/4] upload-pack: use existing config mechanism for advertisement
Date: Wed, 28 Feb 2024 17:48:18 -0500	[thread overview]
Message-ID: <20240228224818.GA1158952@coredump.intra.peff.net> (raw)
In-Reply-To: <20240228224625.GA1158651@coredump.intra.peff.net>

When serving a v2 capabilities request, we call upload_pack_advertise()
to tell us the set of features we can advertise to the client. That
involves looking at various config options, all of which need to be kept
in sync with the rules we use in upload_pack_config to set flags like
allow_filter, allow_sideband_all, and so on. If these two pieces of code
get out of sync then we may refuse to respect a capability we
advertised, or vice versa accept one that we should not.

Instead, let's call the same config helper that we'll use for processing
the actual client request, and then just pick the values out of the
resulting struct. This is only a little bit shorter than the current
code, but we don't repeat any policy logic (e.g., we don't have to worry
about the magic sideband-all environment variable here anymore).

And this reveals a gap in the existing code: there is no struct flag for
the packfile-uris capability (we accept it even if it is not advertised,
which we should not). We'll leave the advertisement code for now and
deal with it in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 upload-pack.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index 6bda20754d..491ef51daa 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1841,41 +1841,35 @@ int upload_pack_v2(struct repository *r, struct packet_reader *request)
 int upload_pack_advertise(struct repository *r,
 			  struct strbuf *value)
 {
+	struct upload_pack_data data;
+
+	upload_pack_data_init(&data);
+	get_upload_pack_config(r, &data);
+
 	if (value) {
-		int allow_filter_value;
-		int allow_ref_in_want;
-		int allow_sideband_all_value;
 		char *str = NULL;
 
 		strbuf_addstr(value, "shallow wait-for-done");
 
-		if (!repo_config_get_bool(r,
-					 "uploadpack.allowfilter",
-					 &allow_filter_value) &&
-		    allow_filter_value)
+		if (data.allow_filter)
 			strbuf_addstr(value, " filter");
 
-		if (!repo_config_get_bool(r,
-					 "uploadpack.allowrefinwant",
-					 &allow_ref_in_want) &&
-		    allow_ref_in_want)
+		if (data.allow_ref_in_want)
 			strbuf_addstr(value, " ref-in-want");
 
-		if (git_env_bool("GIT_TEST_SIDEBAND_ALL", 0) ||
-		    (!repo_config_get_bool(r,
-					   "uploadpack.allowsidebandall",
-					   &allow_sideband_all_value) &&
-		     allow_sideband_all_value))
+		if (data.allow_sideband_all)
 			strbuf_addstr(value, " sideband-all");
 
 		if (!repo_config_get_string(r,
 					    "uploadpack.blobpackfileuri",
 					    &str) &&
 		    str) {
 			strbuf_addstr(value, " packfile-uris");
 			free(str);
 		}
 	}
 
+	upload_pack_data_clear(&data);
+
 	return 1;
 }
-- 
2.44.0.rc2.424.gbdbf4d014b


  parent reply	other threads:[~2024-02-28 22:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28 22:46 [PATCH 0/4] some v2 capability advertisement cleanups Jeff King
2024-02-28 22:46 ` [PATCH 1/4] upload-pack: use repository struct to get config Jeff King
2024-03-04  7:45   ` Patrick Steinhardt
2024-02-28 22:47 ` [PATCH 2/4] upload-pack: centralize setup of sideband-all config Jeff King
2024-02-28 22:48 ` Jeff King [this message]
2024-02-28 22:50 ` [PATCH 4/4] upload-pack: only accept packfile-uris if we advertised it Jeff King
2024-02-28 23:43   ` Junio C Hamano
2024-02-29  5:42   ` Jeff King
2024-02-29 16:34     ` Junio C Hamano
2024-03-01  7:10       ` Jeff King
2024-03-04  7:45   ` Patrick Steinhardt
2024-02-28 23:51 ` [PATCH 0/4] some v2 capability advertisement cleanups Junio C Hamano
2024-02-29  0:44   ` Jeff King
2024-03-04  7:44 ` Patrick Steinhardt
2024-03-04 10:02   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240228224818.GA1158952@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.