fetch-pack: accept absolute-path packfile URIs

Packfile URIs require a scheme and authority even when the pack and the
repository are served by the same HTTP(S) server.

Allow the server to advertise an absolute-path reference, as defined by
RFC 3986, section 4.2 [1]. Resolve it using the remote's scheme and
authority, following section 5.2.2 [2]. For example, /foo.pack from
https://example.com/repo.git resolves to https://example.com/foo.pack.
Packfile URIs do not support local paths, so there is no ambiguity with
a local file named /foo.pack.

Add a separate packfile-uris-absolute-path fetch capability. The client
declares support only for HTTP(S) remotes whose scheme is allowed by
fetch.uriprotocols. This flag does not change the server's URI selection.
When uploadpack.blobPackfileUri contains an absolute-path reference,
reject packfile-URI requests without the flag before starting
pack-objects. Leave fetches that do not request packfile URIs unchanged.

Handle only references beginning with a single slash. Redact their paths
in packet traces, as we do for absolute URIs.

[1] https://www.rfc-editor.org/rfc/rfc3986.html#section-4.2
[2] https://www.rfc-editor.org/rfc/rfc3986.html#section-5.2.2

Signed-off-by: Friel <friel@openai.com>

Author: friel-openai

Documentation/gitprotocol-v2.adoc

@@ -384,6 +384,18 @@ can be included in the client's request.
 	should wait for the client to say "done" before sending the
 	packfile.
 
+If the 'packfile-uris-absolute-path' feature is advertised, the following
+argument can be included in the client's request:
+
+    packfile-uris-absolute-path
+	Indicates that the client can resolve absolute-path references in
+	the 'packfile-uris' section. The client MUST only send this argument
+	when the remote uses an HTTP or HTTPS scheme accepted in its
+	'packfile-uris' request. This declares client support; it does not
+	request that the server change which URIs it sends. A server that
+	requires this support MAY reject a packfile-URI request that omits
+	this argument.
+
 The response of `fetch` is broken into a number of sections separated by
 delimiter packets (0001), with each section beginning with its section
 header. Most sections are sent only when the packfile is sent.
@@ -493,6 +505,16 @@ header. Most sections are sent only when the packfile is sent.
 	* For each URI the server sends, it sends a hash of the pack's
 	  contents (as output by git index-pack) followed by the URI.
 
+	* If the client sent `packfile-uris-absolute-path`, the server may
+	  send references beginning with a single `/`. These references
+	  inherit the remote URL's scheme and authority, replacing its
+	  path, query, and fragment. For example, `/foo.pack` from
+	  `https://example.com/repo.git` resolves to
+	  `https://example.com/foo.pack`. The server MUST NOT send these
+	  references unless the client sent `packfile-uris-absolute-path`.
+	  References beginning with `//` and relative paths without a
+	  leading `/` are not supported.
+
 	* The hashes are 40 hex characters long. When Git upgrades to a new
 	  hash algorithm, this might need to be updated. (It should match
 	  whatever index-pack outputs after "pack\t" or "keep\t".

builtin/fetch-pack.c

@@ -187,6 +187,7 @@ int cmd_fetch_pack(int argc,
 		dest = argv[i++];
 	else
 		usage(fetch_pack_usage);
+	args.url = dest;
 
 	/*
 	 * Copy refs from cmdline to growable list, then append any

builtin/pack-objects.c

@@ -1814,6 +1814,10 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
 		const char *p;
 
 		if (ex) {
+			if (ex->uri[0] == '/' && ex->uri[1] != '/') {
+				oidset_insert(&excluded_by_config, oid);
+				return 0;
+			}
 			for (i = 0; i < uri_protocols.nr; i++) {
 				if (skip_prefix(ex->uri,
 						uri_protocols.items[i].string,

fetch-pack.c

@@ -37,6 +37,7 @@
 #include "mergesort.h"
 #include "prio-queue.h"
 #include "promisor-remote.h"
+#include "urlmatch.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -1375,6 +1376,29 @@ static int add_haves(struct fetch_negotiator *negotiator,
 	return haves_added;
 }
 
+static char *get_packfile_uri_base(const char *url)
+{
+	struct url_info info;
+	char *base;
+
+	if (!url || !uri_protocols.nr)
+		return NULL;
+	base = url_normalize(url, &info);
+	if (!base)
+		return NULL;
+
+	/* An absolute path must not bypass the configured URI protocols. */
+	if ((starts_with(base, "http:") &&
+	     unsorted_string_list_has_string(&uri_protocols, "http")) ||
+	    (starts_with(base, "https:") &&
+	     unsorted_string_list_has_string(&uri_protocols, "https"))) {
+		base[info.path_off] = '\0';
+		return base;
+	}
+	free(base);
+	return NULL;
+}
+
 static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 			      struct fetch_pack_args *args,
 			      const struct ref *wants, struct oidset *common,
@@ -1423,6 +1447,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 			}
 		}
 		if (to_send.len) {
+			if (server_supports_feature("fetch", "packfile-uris-absolute-path", 0)) {
+				char *uri_base = get_packfile_uri_base(args->url);
+
+				if (uri_base)
+					packet_buf_write(&req_buf, "packfile-uris-absolute-path");
+				free(uri_base);
+			}
 			packet_buf_write(&req_buf, "packfile-uris %s",
 					 to_send.buf);
 			strbuf_release(&to_send);
@@ -1639,18 +1670,32 @@ static void receive_wanted_refs(struct packet_reader *reader,
 }
 
 static void receive_packfile_uris(struct packet_reader *reader,
-				  struct string_list *uris)
+				  struct string_list *uris,
+				  const char *url)
 {
+	char *uri_base = get_packfile_uri_base(url);
+
 	process_section_header(reader, "packfile-uris", 0);
 	while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
-		if (reader->pktlen < the_hash_algo->hexsz ||
-		    reader->line[the_hash_algo->hexsz] != ' ')
+		struct object_id oid;
+		const char *end;
+
+		if (parse_oid_hex(reader->line, &oid, &end) || *end != ' ')
 			die("expected '<hash> <uri>', got: %s", reader->line);
 
-		string_list_append(uris, reader->line);
+		if (end[1] == '/') {
+			if (end[2] == '/' || !uri_base)
+				die("unexpected relative packfile URI");
+			string_list_append_nodup(uris,
+						 xstrfmt("%s %s%s", oid_to_hex(&oid),
+							 uri_base, end + 1));
+		} else {
+			string_list_append(uris, reader->line);
+		}
 	}
 	if (reader->status != PACKET_READ_DELIM)
 		die("expected DELIM");
+	free(uri_base);
 }
 
 enum fetch_state {
@@ -1826,7 +1871,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 			if (git_env_bool("GIT_TRACE_REDACT", 1))
 				reader.options |= PACKET_READ_REDACT_URI_PATH;
 			if (process_section_header(&reader, "packfile-uris", 1))
-				receive_packfile_uris(&reader, &packfile_uris);
+				receive_packfile_uris(&reader, &packfile_uris,
+						      args->url);
 			/* We don't expect more URIs. Reset to avoid expensive URI check. */
 			reader.options &= ~PACKET_READ_REDACT_URI_PATH;
 

fetch-pack.h

@@ -10,6 +10,8 @@ struct oid_array;
 
 struct fetch_pack_args {
 	const char *uploadpack;
+	/* Remote URL used to resolve absolute-path packfile URIs. */
+	const char *url;
 	int unpacklimit;
 	int depth;
 	const char *deepen_since;

pkt-line.c

@@ -398,7 +398,11 @@ static const char *find_packfile_uri_path(const char *buffer)
 	if (!(len == 40 || len == 64) || buffer[len] != ' ')
 		return NULL; /* required "<hash>SP" not seen */
 
-	path = strstr(buffer + len + 1, URI_MARK);
+	buffer += len + 1;
+	if (buffer[0] == '/' && buffer[1] != '/')
+		return buffer + 1;
+
+	path = strstr(buffer, URI_MARK);
 	if (!path)
 		return NULL;
 

t/t5702-protocol-v2.sh

@@ -1217,10 +1217,110 @@ configure_exclusion () {
 	git -C "$1" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
 	git -C "$1" config --add \
 		"uploadpack.blobpackfileuri" \
-		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+		"$(cat objh) $(cat packh) ${3-$HTTPD_URL}/dumb/mypack-$(cat packh).pack" &&
 	cat objh
 }
 
+test_expect_success 'setup absolute-path packfile URIs' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/absolute-uri" &&
+	git init "$P" &&
+	git -C "$P" config uploadpack.allowsidebandall true &&
+	>absolute-pack-hashes &&
+	for name in one two
+	do
+		test_commit -C "$P" "$name" &&
+		configure_exclusion "$P" "$name.t" "" >/dev/null &&
+		cat packh >>absolute-pack-hashes || return 1
+	done
+'
+
+test_expect_success 'clone with absolute-path packfile URIs' '
+	test_when_finished "rm -rf absolute-child log" &&
+	GIT_TRACE_PACKET="$TRASH_DIRECTORY/log" GIT_TRACE_REDACT=0 \
+	GIT_TEST_SIDEBAND_ALL=1 \
+	git -c protocol.version=2 -c fetch.uriprotocols=http,https \
+		clone "$HTTPD_URL/smart/absolute-uri" absolute-child &&
+	while read hash
+	do
+		test_path_is_file \
+			"absolute-child/.git/objects/pack/pack-$hash.pack" &&
+		test_grep -F "clone< \\1$hash /dumb/mypack-$hash.pack" log ||
+		return 1
+	done <absolute-pack-hashes &&
+	test_grep "clone< fetch=.*packfile-uris-absolute-path" log &&
+	test_grep "clone> packfile-uris http,https$" log &&
+	test_grep "clone> packfile-uris-absolute-path$" log &&
+	git -C absolute-child fsck
+'
+
+test_expect_success 'absolute-path packfile URI fetch redacts the path' '
+	test_when_finished "rm -rf absolute-child log" &&
+	git init absolute-child &&
+	GIT_TRACE_PACKET="$TRASH_DIRECTORY/log" GIT_TEST_SIDEBAND_ALL=1 \
+	git -C absolute-child -c protocol.version=2 \
+		-c fetch.uriprotocols=http,https \
+		fetch "$HTTPD_URL/smart/absolute-uri" &&
+	while read hash
+	do
+		test_grep -F "fetch< \\1$hash /<redacted>" log || return 1
+	done <absolute-pack-hashes &&
+	test_grep ! /dumb/mypack- log
+'
+
+test_expect_success 'absolute-path packfile URIs require an allowed HTTP scheme' '
+	test_when_finished "rm -rf absolute-child log err" &&
+	case "$HTTPD_PROTO" in
+	http) other_protocol=https ;;
+	https) other_protocol=http ;;
+	esac &&
+	for url in "$HTTPD_URL/smart/absolute-uri" \
+		"file://$HTTPD_DOCUMENT_ROOT_PATH/absolute-uri"
+	do
+		test_must_fail env GIT_TRACE_PACKET="$TRASH_DIRECTORY/log" \
+			GIT_TEST_SIDEBAND_ALL=1 \
+		git -c protocol.version=2 -c fetch.uriprotocols=$other_protocol \
+			clone "$url" absolute-child 2>err &&
+		test_grep "packfile-uris $other_protocol$" log &&
+		test_grep ! "clone> packfile-uris-absolute-path" log &&
+		test_grep "client does not support absolute-path packfile URIs" err &&
+		rm -rf absolute-child log || return 1
+	done
+'
+
+test_expect_success 'absolute-path packfile URIs reject an unsupported client before packing' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/absolute-uri" &&
+	test_when_finished "rm -f absolute-pack-objects-ran" &&
+	write_script "$TRASH_DIRECTORY/absolute-pack-objects-hook" <<-EOF &&
+	>"$TRASH_DIRECTORY/absolute-pack-objects-ran"
+	exec "\$@"
+	EOF
+	test_config_global uploadpack.packObjectsHook \
+		"$TRASH_DIRECTORY/absolute-pack-objects-hook" &&
+	test-tool pkt-line pack >in <<-EOF &&
+	command=fetch
+	object-format=$(test_oid algo)
+	0001
+	want $(git -C "$P" rev-parse HEAD)
+	sideband-all
+	packfile-uris http,https
+	done
+	0000
+	EOF
+	test_must_fail env GIT_PROTOCOL=version=2 git -C "$P" \
+		upload-pack --stateless-rpc . <in >out 2>err &&
+	test_grep "client does not support absolute-path packfile URIs" err &&
+	test_path_is_missing absolute-pack-objects-ran
+'
+
+test_expect_success 'absolute-path URI configuration does not affect ordinary fetches' '
+	test_when_finished "rm -rf absolute-child log" &&
+	GIT_TRACE_PACKET="$TRASH_DIRECTORY/log" GIT_TEST_SIDEBAND_ALL=1 \
+	git -c protocol.version=2 -c fetch.uriprotocols= \
+		clone "$HTTPD_URL/smart/absolute-uri" absolute-child &&
+	test_grep ! "clone> packfile-uris" log &&
+	git -C absolute-child fsck
+'
+
 test_expect_success 'part of packfile response provided as URI' '
 	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
 	rm -rf "$P" http_child log &&
@@ -1241,6 +1341,8 @@ test_expect_success 'part of packfile response provided as URI' '
 	git -c protocol.version=2 \
 		-c fetch.uriprotocols=http,https \
 		clone "$HTTPD_URL/smart/http_parent" http_child &&
+	test_grep ! "clone< fetch=.*packfile-uris-absolute-path" log &&
+	test_grep ! "clone> packfile-uris-absolute-path" log &&
 
 	# Ensure that my-blob and other-blob are in separate packfiles.
 	for idx in http_child/.git/objects/pack/*.idx

transport.c

@@ -487,6 +487,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 
 	memset(&args, 0, sizeof(args));
 	args.uploadpack = data->options.uploadpack;
+	args.url = transport->url;
 	args.keep_pack = data->options.keep;
 	args.lock_pack = 1;
 	args.use_thin_pack = data->options.thin;

upload-pack.c

@@ -119,6 +119,9 @@ struct upload_pack_data {
 	unsigned allow_ref_in_want : 1;				/* v2 only */
 	unsigned allow_sideband_all : 1;			/* v2 only */
 	unsigned seen_haves : 1;				/* v2 only */
+	/* At least one uploadpack.blobPackfileUri uses an absolute-path reference. */
+	unsigned require_absolute_path_uris:1; /* v2 only */
+	unsigned client_supports_absolute_path_uris:1; /* v2 only */
 	unsigned allow_packfile_uris : 1;			/* v2 only */
 	unsigned advertise_sid : 1;
 	unsigned sent_capabilities : 1;
@@ -1364,8 +1367,16 @@ static int upload_pack_config(const char *var, const char *value,
 	} else if (!strcmp("uploadpack.allowsidebandall", var)) {
 		data->allow_sideband_all = git_config_bool(var, value);
 	} else if (!strcmp("uploadpack.blobpackfileuri", var)) {
-		if (value)
+		if (value) {
+			struct object_id oid;
+			const char *uri;
+
 			data->allow_packfile_uris = 1;
+			if (!parse_oid_hex(value, &oid, &uri) && *uri == ' ' &&
+			    !parse_oid_hex(uri + 1, &oid, &uri) && *uri == ' ' &&
+			    uri[1] == '/' && uri[2] != '/')
+				data->require_absolute_path_uris = 1;
+		}
 	} else if (!strcmp("core.precomposeunicode", var)) {
 		cfg->precomposed_unicode = git_config_bool(var, value);
 	} else if (!strcmp("transfer.advertisesid", var)) {
@@ -1660,6 +1671,12 @@ static void process_args(struct packet_reader *request,
 			continue;
 		}
 
+		if (data->require_absolute_path_uris &&
+		    !strcmp(arg, "packfile-uris-absolute-path")) {
+			data->client_supports_absolute_path_uris = 1;
+			continue;
+		}
+
 		if (data->allow_packfile_uris &&
 		    skip_prefix(arg, "packfile-uris ", &p)) {
 			if (data->uri_protocols.nr)
@@ -1679,6 +1696,11 @@ static void process_args(struct packet_reader *request,
 	if (request->status != PACKET_READ_FLUSH)
 		die(_("expected flush after fetch arguments"));
 
+	if (data->uri_protocols.nr && data->require_absolute_path_uris &&
+	    !data->client_supports_absolute_path_uris)
+		send_err_and_die(data,
+				 "client does not support absolute-path packfile URIs");
+
 	if (trace2_is_enabled())
 		trace2_fetch_info(data);
 }
@@ -1852,6 +1874,9 @@ int upload_pack_advertise(struct repository *r,
 		if (data.allow_sideband_all)
 			strbuf_addstr(value, " sideband-all");
 
+		if (data.require_absolute_path_uris)
+			strbuf_addstr(value, " packfile-uris-absolute-path");
+
 		if (data.allow_packfile_uris)
 			strbuf_addstr(value, " packfile-uris");
 	}