git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/4] upload-pack: only accept packfile-uris if we advertised it

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 28, 2024, 23:43 UTC
Message-ID
<xmqq5xy8jgtn.fsf@gitster.g>
In-Reply-To
<20240228225050.GA1159078@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> I suspect in the long term that we may have other ways to trigger this
> feature than the static blobpackfileuri config (e.g., a hook that knows
> about site-specific packfiles "somehow"). So we may need to update the
> test later for that, but presumably in the vanilla config we'll continue
> to skip advertising it.

Sounds quite sensible. Thanks for a series that is very pleasant to read.

Show 90 quoted lines
>
>  t/t5702-protocol-v2.sh | 18 ++++++++++++++++++
>  upload-pack.c          | 16 +++++++---------
>  2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> index 6ef4971845..902e42c1c0 100755
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -778,6 +778,24 @@ test_expect_success 'archive with custom path does not request v2' '
>  	! grep ^GIT_PROTOCOL env.trace
>  '
>  
> +test_expect_success 'reject client packfile-uris if not advertised' '
> +	{
> +		packetize command=fetch &&
> +		printf 0001 &&
> +		packetize packfile-uris https &&
> +		packetize done &&
> +		printf 0000
> +	} >input &&
> +	test_must_fail env GIT_PROTOCOL=version=2 \
> +		git upload-pack client <input &&
> +	test_must_fail env GIT_PROTOCOL=version=2 \
> +		git -c uploadpack.blobpackfileuri \
> +		upload-pack client <input &&
> +	GIT_PROTOCOL=version=2 \
> +		git -c uploadpack.blobpackfileuri=anything \
> +		upload-pack client <input
> +'
> +
>  # Test protocol v2 with 'http://' transport
>  #
>  . "$TEST_DIRECTORY"/lib-httpd.sh
> diff --git a/upload-pack.c b/upload-pack.c
> index 491ef51daa..66f4de9d87 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -113,6 +113,7 @@ struct upload_pack_data {
>  	unsigned done : 1;					/* v2 only */
>  	unsigned allow_ref_in_want : 1;				/* v2 only */
>  	unsigned allow_sideband_all : 1;			/* v2 only */
> +	unsigned allow_packfile_uris : 1;			/* v2 only */
>  	unsigned advertise_sid : 1;
>  	unsigned sent_capabilities : 1;
>  };
> @@ -1362,6 +1363,9 @@ static int upload_pack_config(const char *var, const char *value,
>  		data->allow_ref_in_want = git_config_bool(var, value);
>  	} else if (!strcmp("uploadpack.allowsidebandall", var)) {
>  		data->allow_sideband_all = git_config_bool(var, value);
> +	} else if (!strcmp("uploadpack.blobpackfileuri", var)) {
> +		if (value)
> +			data->allow_packfile_uris = 1;
>  	} else if (!strcmp("core.precomposeunicode", var)) {
>  		precomposed_unicode = git_config_bool(var, value);
>  	} else if (!strcmp("transfer.advertisesid", var)) {
> @@ -1647,7 +1651,8 @@ static void process_args(struct packet_reader *request,
>  			continue;
>  		}
>  
> -		if (skip_prefix(arg, "packfile-uris ", &p)) {
> +		if (data->allow_packfile_uris &&
> +		    skip_prefix(arg, "packfile-uris ", &p)) {
>  			string_list_split(&data->uri_protocols, p, ',', -1);
>  			continue;
>  		}
> @@ -1847,8 +1852,6 @@ int upload_pack_advertise(struct repository *r,
>  	get_upload_pack_config(r, &data);
>  
>  	if (value) {
> -		char *str = NULL;
> -
>  		strbuf_addstr(value, "shallow wait-for-done");
>  
>  		if (data.allow_filter)
> @@ -1860,13 +1863,8 @@ int upload_pack_advertise(struct repository *r,
>  		if (data.allow_sideband_all)
>  			strbuf_addstr(value, " sideband-all");
>  
> -		if (!repo_config_get_string(r,
> -					    "uploadpack.blobpackfileuri",
> -					    &str) &&
> -		    str) {
> +		if (data.allow_packfile_uris)
>  			strbuf_addstr(value, " packfile-uris");
> -			free(str);
> -		}
>  	}
>  
>  	upload_pack_data_clear(&data);
Previous: Jeff KingNext: Jeff King
Message 7 of 15 in “some v2 capability advertisement cleanups”
  1. 0/4 some v2 capability advertisement cleanupsJeff King, Feb 28, 2024
  2. 1/4 upload-pack: use repository struct to get configJeff King, Feb 28, 2024
  3. Patrick SteinhardtMar 4, 2024
  4. 2/4 upload-pack: centralize setup of sideband-all configJeff King, Feb 28, 2024
  5. 3/4 upload-pack: use existing config mechanism for advertisementJeff King, Feb 28, 2024
  6. 4/4 upload-pack: only accept packfile-uris if we advertised itJeff King, Feb 28, 2024
  7. Junio C HamanoFeb 28, 2024
  8. Jeff KingFeb 29, 2024
  9. Junio C HamanoFeb 29, 2024
  10. Jeff KingMar 1, 2024
  11. Patrick SteinhardtMar 4, 2024
  12. Junio C HamanoFeb 28, 2024
  13. Jeff KingFeb 29, 2024
  14. Patrick SteinhardtMar 4, 2024
  15. Jeff KingMar 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.