Subject: [PATCH v2 00/10] Prepare for advertised remotes auto-configure via URL allowlist

From: Christian Couder <[email-masked]>
Date: Tue, 7 Apr 2026 13:52:33 +0200

Recently, I sent a 16 patch long series that makes it possible for
promisor remotes advertised through the "promisor-remote" protocol
capability to be auto-configured on the client side via a URL
allowlist configured using a new `promisor.acceptFromServerUrl`
configuration variable:

https://lore.kernel.org/git/[email-masked]/

I got the suggestion to split the 16 patches series into two smaller
series, starting with a preparatory series. So here is this
preparatory series. It's a mix of mostly small fixes, refactorings and
cleanups.

High level description of the patches
=====================================

 - Patches 1-4/10 are fixes:

   - Patch 1/10 is the most significant. The others are relatively
     small.
     
   - Patch 4/10 is the only new in this series (while all the others
     were in the previous series). It prepares for Patch 5/10.

 - Patches 5-10/10 are refactorings and cleanups:

   - Patches 5-7/10 are relatively small independents cleanups or
     refactorings.

   - Patches 8-9/10 are related refactorings simplifying the data
     structures used in filter_promisor_remote() and
     promisor_remote_reply().

   - Patch 10/10 is cleaning up the 'file://' URIs with absolute paths
     in the test script.

Changes since v1
================

Thanks to Patrick and Junio for reviewing the previous versions of
this series.

Only patch 10/10 ("t5710: use proper file:// URIs for absolute paths")
changed since v1:

 - PWD_URL and ENCODED_PWD_URL have been renamed TRASH_DIRECTORY_URL
   and ENCODED_TRASH_DIRECTORY_URL respectively.

 - Instead of percent-encoding only space characters, we now encode
   '%', ' ', '=', ',' and ';'.

 - The test script is skipped altogether if there are other characters
   than those we encode or expect in the current directory path.

 - The tests introduced by commit 1/10 now also benefit from the
   TRASH_DIRECTORY_URL and ENCODED_TRASH_DIRECTORY_URL variables.

 - Commit message and code comments are improved a bit.

CI tests
========

They all pass, see:

https://github.com/chriscool/git/actions/runs/24076952684

Range-diff since v1
===================

 1:  906c5f6fbb =  1:  6687a0fe3f promisor-remote: try accepted remotes before others in get_direct()
 2:  830eb10097 =  2:  23869a4509 promisor-remote: pass config entry to all_fields_match() directly
 3:  b18cdf2fcb =  3:  16d5e0b7ef promisor-remote: clarify that a remote is ignored
 4:  b2125ec770 =  4:  f55f6e8c14 promisor-remote: reject empty name or URL in advertised remote
 5:  e69abb1256 =  5:  52cd36ab64 promisor-remote: refactor should_accept_remote() control flow
 6:  772ea53e9e =  6:  286ae3b4e3 promisor-remote: refactor has_control_char()
 7:  79cf945c24 =  7:  4cb239cd24 promisor-remote: refactor accept_from_server()
 8:  b953e2491e =  8:  7bfe26d5d4 promisor-remote: keep accepted promisor_info structs alive
 9:  3fce719fc0 =  9:  9ffa80444f promisor-remote: remove the 'accepted' strvec
10:  e54af9e176 ! 10:  b77b449c76 t5710: use proper file:// URIs for absolute paths
    @@ Commit message
         these URLs through `url_normalize()` or similar functions.
     
         To future-proof the tests and ensure cross-platform URI compliance,
    -    let's introduce a $PWD_URL helper that explicitly guarantees a leading
    -    slash for the path component, ensuring valid 3-slash `file:///` URIs on
    -    all operating systems.
    +    let's introduce a $TRASH_DIRECTORY_URL helper variable that explicitly
    +    guarantees a leading slash for the path component, ensuring valid
    +    3-slash `file:///` URIs on all operating systems.
     
    -    While at it, let's also introduce $ENCODED_PWD_URL to handle spaces in
    -    directory paths (which is needed for URL glob pattern matching).
    +    While at it, let's also introduce $ENCODED_TRASH_DIRECTORY_URL to
    +    handle some common special characters in directory paths.
     
    -    Then let's replace all instances of `file://$(pwd)` with $PWD_URL across
    -    the test script, and let's simplify the `ENCODED_URL` constructions in
    -    the `sendFields` and `checkFields` tests to use $ENCODED_PWD_URL
    -    directly.
    +    To be extra safe, let's skip all the tests if there are uncommon
    +    special characters in the directory path.
    +
    +    Then let's replace all instances of `file://$(pwd)` with
    +    $TRASH_DIRECTORY_URL across the test script, and let's simplify the
    +    `sendFields` and `checkFields` tests to use
    +    $ENCODED_TRASH_DIRECTORY_URL directly.
     
         Signed-off-by: Christian Couder <[email-masked]>
     
    @@ t/t5710-promisor-remote-capability.sh: copy_to_lop () {
      	cp "$path" "$path2"
      }
      
    -+# On Windows, 'pwd' returns a path like 'D:/foo/bar'. Prepend '/' to turn
    ++# On Windows, `pwd` returns a path like 'D:/foo/bar'. Prepend '/' to turn
     +# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.
     +# On Unix, the path already starts with '/', so this is a no-op.
     +pwd_path=$(pwd)
     +case "$pwd_path" in
     +[a-zA-Z]:*) pwd_path="/$pwd_path" ;;
     +esac
    -+PWD_URL="file://$pwd_path"
    -+# Same as PWD_URL but with spaces percent-encoded, for use in URL patterns.
    -+ENCODED_PWD_URL="file://$(echo "$pwd_path" | sed "s/ /%20/g")"
    ++
    ++# Allowed characters: alphanumeric, standard path/URI (_ . ~ / : -),
    ++# and those percent-encoded below (% space = , ;)
    ++rest=$(printf "%s" "$pwd_path" | tr -d 'a-zA-Z0-9_.~/:% =,;-')
    ++if test -n "$rest"
    ++then
    ++	skip_all="PWD contains unsupported special characters"
    ++	test_done
    ++fi
    ++
    ++TRASH_DIRECTORY_URL="file://$pwd_path"
    ++
    ++encoded_path=$(printf "%s" "$pwd_path" |
    ++	       sed -e 's/%/%25/g' -e 's/ /%20/g' -e 's/=/%3D/g' \
    ++		   -e 's/;/%3B/g' -e 's/,/%2C/g')
    ++
    ++ENCODED_TRASH_DIRECTORY_URL="file://$encoded_path"
     +
      test_expect_success "setup for testing promisor remote advertisement" '
      	# Create another bare repo called "lop" (for Large Object Promisor)
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "setup for testing pr
      
      	# Configure lop as promisor remote for server
     -	git -C server remote add lop "file://$(pwd)/lop" &&
    -+	git -C server remote add lop "$PWD_URL/lop" &&
    ++	git -C server remote add lop "$TRASH_DIRECTORY_URL/lop" &&
      	git -C server config remote.lop.promisor true &&
      
      	git -C lop config uploadpack.allowFilter true &&
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=All \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=All \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=None \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "init + fetch with pr
      	git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" &&
     -	git -C client config remote.lop.url "file://$(pwd)/lop" &&
     -	git -C client config remote.server.url "file://$(pwd)/server" &&
    -+	git -C client config remote.lop.url "$PWD_URL/lop" &&
    -+	git -C client config remote.server.url "$PWD_URL/server" &&
    ++	git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
    ++	git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" &&
      	git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" &&
      	git -C client config promisor.acceptfromserver All &&
      	GIT_NO_LAZY_FETCH=0 git -C client fetch --filter="blob:limit=5k" server &&
    +@@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with two promisors but only one advertised" '
    + 	GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
    + 		-c remote.unused_lop.promisor=true \
    + 		-c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \
    +-		-c remote.unused_lop.url="file://$(pwd)/unused_lop" \
    ++		-c remote.unused_lop.url="$TRASH_DIRECTORY_URL/unused_lop" \
    + 		-c remote.lop.promisor=true \
    + 		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
    +-		-c remote.lop.url="file://$(pwd)/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
    + 		-c promisor.acceptfromserver=All \
    + 		--no-local --filter="blob:limit=5k" server client &&
    + 
    +@@ t/t5710-promisor-remote-capability.sh: test_expect_success "init + fetch two promisors but only one advertised" '
    + 	git -C client init &&
    + 	git -C client config remote.unused_lop.promisor true &&
    + 	git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" &&
    +-	git -C client config remote.unused_lop.url "file://$(pwd)/unused_lop" &&
    ++	git -C client config remote.unused_lop.url "$TRASH_DIRECTORY_URL/unused_lop" &&
    + 	git -C client config remote.lop.promisor true &&
    + 	git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" &&
    +-	git -C client config remote.lop.url "file://$(pwd)/lop" &&
    +-	git -C client config remote.server.url "file://$(pwd)/server" &&
    ++	git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
    ++	git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" &&
    + 	git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" &&
    + 	git -C client config promisor.acceptfromserver All &&
    + 
     @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.acceptfromserver set to 'KnownName'" '
      	# Clone from server to create a client
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=KnownName \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownNam
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.serverTwo.promisor=true \
      		-c remote.serverTwo.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.serverTwo.url="file://$(pwd)/lop" \
    -+		-c remote.serverTwo.url="$PWD_URL/lop" \
    ++		-c remote.serverTwo.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=KnownName \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=KnownUrl \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownUrl
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/serverTwo" \
    -+		-c remote.lop.url="$PWD_URL/serverTwo" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/serverTwo" \
      		-c promisor.acceptfromserver=KnownUrl \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownUrl
      	test_when_finished "rm -rf client" &&
      
     -	test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" &&
    -+	test_when_finished "git -C server config set remote.lop.url \"$PWD_URL/lop\"" &&
    ++	test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" &&
      	git -C server config unset remote.lop.url &&
      
      	# Clone from server to create a client
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownUrl
      	test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=KnownUrl \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownUrl
      	test_when_finished "rm -rf client" &&
      
     -	test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" &&
    -+	test_when_finished "git -C server config set remote.lop.url \"$PWD_URL/lop\"" &&
    ++	test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" &&
      	git -C server config set remote.lop.url "" &&
      
      	# Clone from server to create a client
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with 'KnownUrl
      	test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=KnownUrl \
      		--no-local --filter="blob:limit=5k" server client &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      		-c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=All \
      		--no-local --filter="blob:limit=5k" server client &&
      
      	# Check that fields are properly transmitted
     -	ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") &&
     -	PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" &&
    -+	PR1="name=lop,url=$ENCODED_PWD_URL/lop,partialCloneFilter=blob:none" &&
    ++	PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" &&
      	PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" &&
      	test_grep "clone< promisor-remote=$PR1;$PR2" trace &&
      	test_grep "clone> promisor-remote=lop;otherLop" trace &&
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      		-c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c remote.lop.partialCloneFilter="blob:none" \
      		-c promisor.acceptfromserver=All \
      		-c promisor.checkFields=partialcloneFilter \
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	# Check that fields are properly transmitted
     -	ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") &&
     -	PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" &&
    -+	PR1="name=lop,url=$ENCODED_PWD_URL/lop,partialCloneFilter=blob:none" &&
    ++	PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" &&
      	PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" &&
      	test_grep "clone< promisor-remote=$PR1;$PR2" trace &&
      	test_grep "clone> promisor-remote=lop" trace &&
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      		-c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c remote.lop.token="fooYYY" \
      		-c remote.lop.partialCloneFilter="blob:none" \
      		-c promisor.acceptfromserver=All \
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone and fetch with
      	GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
      		-c remote.lop.promisor=true \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=All \
      		--no-local --filter=auto server client 2>err &&
      
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
      	GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
      		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
     -		-c remote.lop.url="file://$(pwd)/lop" \
    -+		-c remote.lop.url="$PWD_URL/lop" \
    ++		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
      		-c promisor.acceptfromserver=All \
      		--no-local --filter="blob:limit=5k" server client &&

Christian Couder (10):
  promisor-remote: try accepted remotes before others in get_direct()
  promisor-remote: pass config entry to all_fields_match() directly
  promisor-remote: clarify that a remote is ignored
  promisor-remote: reject empty name or URL in advertised remote
  promisor-remote: refactor should_accept_remote() control flow
  promisor-remote: refactor has_control_char()
  promisor-remote: refactor accept_from_server()
  promisor-remote: keep accepted promisor_info structs alive
  promisor-remote: remove the 'accepted' strvec
  t5710: use proper file:// URIs for absolute paths

 Documentation/gitprotocol-v2.adoc     |   4 +
 promisor-remote.c                     | 207 +++++++++++++++-----------
 t/t5710-promisor-remote-capability.sh | 138 ++++++++++++++---
 3 files changed, 238 insertions(+), 111 deletions(-)

-- 
2.54.0.rc0.114.g05d466edb8