* [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
@ 2026-01-13 3:19 Aaron Plattner
2026-01-13 6:49 ` Patrick Steinhardt
2026-01-13 13:09 ` Junio C Hamano
0 siblings, 2 replies; 9+ messages in thread
From: Aaron Plattner @ 2026-01-13 3:19 UTC (permalink / raw)
To: git; +Cc: Aaron Plattner, Patrick Steinhardt, Rahul Rameshbabu,
Lucas De Marchi
If a large request requires post_rpc() to call probe_rpc(), the latter
does not use the authorization credentials used for other requests. If
this fails with an HTTP 401 error and http_auth.multistage isn't set,
then the whole request just fails.
For example, using git-credential-msal [1], the following attempt to clone a
large repository fails partway through because the initial request to download
the commit history and promisor packs succeeds, but the
subsequent request to download the blobs needed to construct the working
tree fails with a 401 error and the checkout fails.
(lines removed for brevity)
git clone --filter=blob:none https://secure-server.example/repo
11:03:26.855369 git.c:502 trace: built-in: git clone --filter=blob:none https://secure-server.example/repo
Cloning into 'sw'...
warning: templates not found in /home/aaron/share/git-core/templates
11:03:26.857169 run-command.c:673 trace: run_command: git remote-https origin https://secure-server.example/repo
11:03:27.012104 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
11:03:27.049243 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
11:03:27.049270 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
11:03:27.053786 run-command.c:673 trace: run_command: 'git credential-msal get'
11:03:27.952830 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
11:03:27.952849 http.c:849 => Send header: Authorization: Bearer <redacted>
11:03:27.995419 http.c:849 <= Recv header: HTTP/1.1 200 OK
11:03:28.230039 http.c:890 == Info: Reusing existing https: connection with host secure-server.example
11:03:28.230208 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
11:03:28.230216 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
11:03:28.230221 http.c:849 => Send header: Authorization: Bearer <redacted>
11:03:28.269085 http.c:849 <= Recv header: HTTP/1.1 200 OK
11:03:28.684163 http.c:890 == Info: Reusing existing https: connection with host secure-server.example
11:03:28.684379 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
11:03:28.684391 http.c:849 => Send header: Accept: application/x-git-upload-pack-result
11:03:28.684393 http.c:849 => Send header: Authorization: Bearer <redacted>
11:03:28.869546 run-command.c:673 trace: run_command: git index-pack --stdin --fix-thin '--keep=fetch-pack 43856 on dgx-spark' --promisor
11:06:39.861237 run-command.c:673 trace: run_command: git -c fetch.negotiationAlgorithm=noop fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no --filter=blob:none --stdin
11:06:39.865981 run-command.c:673 trace: run_command: git remote-https origin https://secure-server.example/repo
11:06:39.868039 run-command.c:673 trace: run_command: git-remote-https origin https://secure-server.example/repo
11:07:30.412575 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
11:07:30.456285 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
11:07:30.456318 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
11:07:30.456439 run-command.c:673 trace: run_command: 'git credential-cache get'
11:07:30.461266 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
11:07:30.461282 http.c:849 => Send header: Authorization: Bearer <redacted>
11:07:30.501628 http.c:849 <= Recv header: HTTP/1.1 200 OK
11:07:34.725262 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
11:07:34.725279 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
11:07:34.761407 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
11:07:34.761443 http.c:890 == Info: Bearer authentication problem, ignoring.
11:07:34.761453 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
11:07:34.761509 http.c:890 == Info: The requested URL returned error: 401
11:07:34.761530 http.c:890 == Info: closing connection #0
11:07:34.761913 run-command.c:673 trace: run_command: 'git credential-cache erase'
11:07:34.761927 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
11:07:34.768069 git.c:502 trace: built-in: git credential-cache erase
11:07:34.768690 run-command.c:673 trace: run_command: 'git credential-msal erase'
11:07:34.768713 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
11:07:34.772742 git.c:808 trace: exec: git-credential-msal erase
11:07:34.772783 run-command.c:673 trace: run_command: git-credential-msal erase
11:07:34.772819 run-command.c:765 trace: start_command: /usr/bin/git-credential-msal erase
error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
fatal: unable to write request to remote: Broken pipe
fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry with 'git restore --source=HEAD :/'
In this case, the HTTP_REAUTH retry logic is not used because the
credential helper didn't set the 'continue' flag, so
http_auth.multistage is false and handle_curl_result() fails with
HTTP_NOAUTH instead.
Fix the immediate problem by including the authorization headers in the
probe_rpc() request as well.
Add a test for this scenario:
1. Create a repository with two thousand refs.
2. Clone that into the web root used by t5563-simple-http-auth.sh.
3. Configure http.postBuffer to be very small in order to trigger the
probe_rpc() path that fails.
4. Clone using a valid Bearer token.
[1] https://github.com/Binary-Eater/git-credential-msal
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Tested-by: Lucas De Marchi <demarchi@kernel.org>
---
v2: Update commit description to clarify why HTTP_REAUTH doesn't help here, and
add a test to verify the fix.
remote-curl.c | 2 ++
t/t5563-simple-http-auth.sh | 46 +++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/remote-curl.c b/remote-curl.c
index 69f919454a..1d0ae72521 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -877,6 +877,8 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
headers = curl_slist_append(headers, rpc->hdr_content_type);
headers = curl_slist_append(headers, rpc->hdr_accept);
+ headers = http_append_auth_header(&http_auth, headers);
+
curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
curl_easy_setopt(slot->curl, CURLOPT_POST, 1L);
curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index c1febbae9d..adc210cdd3 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -63,6 +63,26 @@ test_expect_success 'setup repository' '
git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
'
+test_expect_success 'setup large repository for probe_rpc testing' '
+ rm -rf large.git &&
+ git init large.git &&
+ (
+ cd large.git &&
+ git config set maintenance.auto false &&
+ git commit --allow-empty --message "initial" &&
+ # Create many refs to trigger probe_rpc, which is called when
+ # the request body is larger than http.postBuffer.
+ #
+ # In the test later, http.postBuffer is set to 70000. Each
+ # "want" line is ~45 bytes, so we need at least 70000/45 = ~1600
+ # refs
+ printf "create refs/heads/branch-%d @\n" $(test_seq 2000) |
+ git update-ref --stdin
+ ) &&
+ git clone --bare large.git "$HTTPD_DOCUMENT_ROOT_PATH/large.git" &&
+ rm -rf large.git
+'
+
test_expect_success 'access using basic auth' '
test_when_finished "per_test_cleanup" &&
@@ -605,6 +625,32 @@ test_expect_success 'access using bearer auth with invalid credentials' '
EOF
'
+test_expect_success 'clone with bearer auth and probe_rpc' '
+ test_when_finished "per_test_cleanup" &&
+
+ set_credential_reply get <<-EOF &&
+ capability[]=authtype
+ authtype=Bearer
+ credential=YS1naXQtdG9rZW4=
+ EOF
+
+ # Bearer token
+ cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+ id=1 creds=Bearer YS1naXQtdG9rZW4=
+ EOF
+
+ cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+ id=1 status=200
+ id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com"
+ EOF
+
+ # Set a small buffer to force probe_rpc to be called
+ # Must be > LARGE_PACKET_MAX (65520)
+ test_config_global http.postBuffer 70000 &&
+ test_config_global credential.helper test-helper &&
+ git clone "$HTTPD_URL/custom_auth/large.git" partial-auth-clone 2>clone-error
+'
+
test_expect_success 'access using three-legged auth' '
test_when_finished "per_test_cleanup" &&
--
2.52.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-13 3:19 [PATCH v2] remote-curl: Use auth for probe_rpc() requests too Aaron Plattner
@ 2026-01-13 6:49 ` Patrick Steinhardt
2026-01-13 13:09 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Patrick Steinhardt @ 2026-01-13 6:49 UTC (permalink / raw)
To: Aaron Plattner; +Cc: git, Rahul Rameshbabu, Lucas De Marchi
On Mon, Jan 12, 2026 at 07:19:28PM -0800, Aaron Plattner wrote:
> If a large request requires post_rpc() to call probe_rpc(), the latter
> does not use the authorization credentials used for other requests. If
> this fails with an HTTP 401 error and http_auth.multistage isn't set,
> then the whole request just fails.
>
> For example, using git-credential-msal [1], the following attempt to clone a
> large repository fails partway through because the initial request to download
> the commit history and promisor packs succeeds, but the
> subsequent request to download the blobs needed to construct the working
> tree fails with a 401 error and the checkout fails.
>
> (lines removed for brevity)
>
> git clone --filter=blob:none https://secure-server.example/repo
> 11:03:26.855369 git.c:502 trace: built-in: git clone --filter=blob:none https://secure-server.example/repo
> Cloning into 'sw'...
> warning: templates not found in /home/aaron/share/git-core/templates
> 11:03:26.857169 run-command.c:673 trace: run_command: git remote-https origin https://secure-server.example/repo
> 11:03:27.012104 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
> 11:03:27.049243 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
> 11:03:27.049270 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
> 11:03:27.053786 run-command.c:673 trace: run_command: 'git credential-msal get'
> 11:03:27.952830 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
> 11:03:27.952849 http.c:849 => Send header: Authorization: Bearer <redacted>
> 11:03:27.995419 http.c:849 <= Recv header: HTTP/1.1 200 OK
> 11:03:28.230039 http.c:890 == Info: Reusing existing https: connection with host secure-server.example
> 11:03:28.230208 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
> 11:03:28.230216 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
> 11:03:28.230221 http.c:849 => Send header: Authorization: Bearer <redacted>
> 11:03:28.269085 http.c:849 <= Recv header: HTTP/1.1 200 OK
> 11:03:28.684163 http.c:890 == Info: Reusing existing https: connection with host secure-server.example
> 11:03:28.684379 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
> 11:03:28.684391 http.c:849 => Send header: Accept: application/x-git-upload-pack-result
> 11:03:28.684393 http.c:849 => Send header: Authorization: Bearer <redacted>
> 11:03:28.869546 run-command.c:673 trace: run_command: git index-pack --stdin --fix-thin '--keep=fetch-pack 43856 on dgx-spark' --promisor
> 11:06:39.861237 run-command.c:673 trace: run_command: git -c fetch.negotiationAlgorithm=noop fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no --filter=blob:none --stdin
> 11:06:39.865981 run-command.c:673 trace: run_command: git remote-https origin https://secure-server.example/repo
> 11:06:39.868039 run-command.c:673 trace: run_command: git-remote-https origin https://secure-server.example/repo
> 11:07:30.412575 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
> 11:07:30.456285 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
> 11:07:30.456318 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
> 11:07:30.456439 run-command.c:673 trace: run_command: 'git credential-cache get'
> 11:07:30.461266 http.c:849 => Send header: GET repo/info/refs?service=git-upload-pack HTTP/1.1
> 11:07:30.461282 http.c:849 => Send header: Authorization: Bearer <redacted>
> 11:07:30.501628 http.c:849 <= Recv header: HTTP/1.1 200 OK
> 11:07:34.725262 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
> 11:07:34.725279 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
> 11:07:34.761407 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
> 11:07:34.761443 http.c:890 == Info: Bearer authentication problem, ignoring.
> 11:07:34.761453 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
> 11:07:34.761509 http.c:890 == Info: The requested URL returned error: 401
> 11:07:34.761530 http.c:890 == Info: closing connection #0
> 11:07:34.761913 run-command.c:673 trace: run_command: 'git credential-cache erase'
> 11:07:34.761927 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
> 11:07:34.768069 git.c:502 trace: built-in: git credential-cache erase
> 11:07:34.768690 run-command.c:673 trace: run_command: 'git credential-msal erase'
> 11:07:34.768713 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
> 11:07:34.772742 git.c:808 trace: exec: git-credential-msal erase
> 11:07:34.772783 run-command.c:673 trace: run_command: git-credential-msal erase
> 11:07:34.772819 run-command.c:765 trace: start_command: /usr/bin/git-credential-msal erase
> error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
> fatal: unable to write request to remote: Broken pipe
> fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
> warning: Clone succeeded, but checkout failed.
> You can inspect what was checked out with 'git status'
> and retry with 'git restore --source=HEAD :/'
>
> In this case, the HTTP_REAUTH retry logic is not used because the
> credential helper didn't set the 'continue' flag, so
> http_auth.multistage is false and handle_curl_result() fails with
> HTTP_NOAUTH instead.
Nice, this is a helpful little detail.
> diff --git a/remote-curl.c b/remote-curl.c
> index 69f919454a..1d0ae72521 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -877,6 +877,8 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
> headers = curl_slist_append(headers, rpc->hdr_content_type);
> headers = curl_slist_append(headers, rpc->hdr_accept);
>
> + headers = http_append_auth_header(&http_auth, headers);
> +
> curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
> curl_easy_setopt(slot->curl, CURLOPT_POST, 1L);
> curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> index c1febbae9d..adc210cdd3 100755
> --- a/t/t5563-simple-http-auth.sh
> +++ b/t/t5563-simple-http-auth.sh
> @@ -63,6 +63,26 @@ test_expect_success 'setup repository' '
> git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
> '
>
> +test_expect_success 'setup large repository for probe_rpc testing' '
> + rm -rf large.git &&
No previous test creates this directory, so it shouldn't ever exist.
> + git init large.git &&
> + (
> + cd large.git &&
> + git config set maintenance.auto false &&
> + git commit --allow-empty --message "initial" &&
> + # Create many refs to trigger probe_rpc, which is called when
> + # the request body is larger than http.postBuffer.
> + #
> + # In the test later, http.postBuffer is set to 70000. Each
> + # "want" line is ~45 bytes, so we need at least 70000/45 = ~1600
> + # refs
> + printf "create refs/heads/branch-%d @\n" $(test_seq 2000) |
> + git update-ref --stdin
> + ) &&
> + git clone --bare large.git "$HTTPD_DOCUMENT_ROOT_PATH/large.git" &&
> + rm -rf large.git
You can use "test_when_finished rm -rf large.git" before creating the
repo to ensure it's always getting pruned at the end of the test.
> +'
> +
> test_expect_success 'access using basic auth' '
> test_when_finished "per_test_cleanup" &&
>
I think we can simply inline this setup into the below test. We only use
it in that specific test anyway.
> @@ -605,6 +625,32 @@ test_expect_success 'access using bearer auth with invalid credentials' '
> EOF
> '
>
> +test_expect_success 'clone with bearer auth and probe_rpc' '
> + test_when_finished "per_test_cleanup" &&
> +
> + set_credential_reply get <<-EOF &&
> + capability[]=authtype
> + authtype=Bearer
> + credential=YS1naXQtdG9rZW4=
> + EOF
> +
> + # Bearer token
> + cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
> + id=1 creds=Bearer YS1naXQtdG9rZW4=
> + EOF
> +
> + cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
> + id=1 status=200
> + id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com"
> + EOF
> +
> + # Set a small buffer to force probe_rpc to be called
> + # Must be > LARGE_PACKET_MAX (65520)
> + test_config_global http.postBuffer 70000 &&
> + test_config_global credential.helper test-helper &&
> + git clone "$HTTPD_URL/custom_auth/large.git" partial-auth-clone 2>clone-error
> +'
> +
Yup, I've verified that this test fails when the bug fix is reverted.
Good.
You might want to reroll to address my two nits around the test, but
even without that reroll I'm already happy with this v2. Thanks!
Patrick
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-13 3:19 [PATCH v2] remote-curl: Use auth for probe_rpc() requests too Aaron Plattner
2026-01-13 6:49 ` Patrick Steinhardt
@ 2026-01-13 13:09 ` Junio C Hamano
2026-01-14 1:06 ` Aaron Plattner
1 sibling, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2026-01-13 13:09 UTC (permalink / raw)
To: Aaron Plattner; +Cc: git, Patrick Steinhardt, Rahul Rameshbabu, Lucas De Marchi
Aaron Plattner <aplattner@nvidia.com> writes:
> Subject: Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
Micronit. "Use" -> "use" (see "git shortlog --no-merges -200
master" and notice that the usual "the first word of the sentence is
upcased" rule does not typically apply to the word after "<area>: "
prefix).
> If a large request requires post_rpc() to call probe_rpc(), the latter
> does not use the authorization credentials used for other requests. If
> this fails with an HTTP 401 error and http_auth.multistage isn't set,
> then the whole request just fails.
> For example, using git-credential-msal [1], the following attempt to clone a
> large repository fails partway through because the initial request to download
> the commit history and promisor packs succeeds, but the
> subsequent request to download the blobs needed to construct the working
> tree fails with a 401 error and the checkout fails.
>
> (lines removed for brevity)
>
> git clone --filter=blob:none https://secure-server.example/repo
> ...
> 11:07:34.725262 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
> 11:07:34.725279 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
> 11:07:34.761407 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
> 11:07:34.761443 http.c:890 == Info: Bearer authentication problem, ignoring.
> 11:07:34.761453 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
> 11:07:34.761509 http.c:890 == Info: The requested URL returned error: 401
> 11:07:34.761530 http.c:890 == Info: closing connection #0
Excellent illustration here.
> 11:07:34.761913 run-command.c:673 trace: run_command: 'git credential-cache erase'
> 11:07:34.761927 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
> 11:07:34.768069 git.c:502 trace: built-in: git credential-cache erase
> 11:07:34.768690 run-command.c:673 trace: run_command: 'git credential-msal erase'
> 11:07:34.768713 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
> 11:07:34.772742 git.c:808 trace: exec: git-credential-msal erase
> 11:07:34.772783 run-command.c:673 trace: run_command: git-credential-msal erase
> 11:07:34.772819 run-command.c:765 trace: start_command: /usr/bin/git-credential-msal erase
> error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
> fatal: unable to write request to remote: Broken pipe
> fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
> warning: Clone succeeded, but checkout failed.
> You can inspect what was checked out with 'git status'
> and retry with 'git restore --source=HEAD :/'
>
> In this case, the HTTP_REAUTH retry logic is not used because the
> credential helper didn't set the 'continue' flag, so
> http_auth.multistage is false and handle_curl_result() fails with
> HTTP_NOAUTH instead.
>
> Fix the immediate problem by including the authorization headers in the
> probe_rpc() request as well.
Great.
> diff --git a/remote-curl.c b/remote-curl.c
> index 69f919454a..1d0ae72521 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -877,6 +877,8 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
> headers = curl_slist_append(headers, rpc->hdr_content_type);
> headers = curl_slist_append(headers, rpc->hdr_accept);
>
> + headers = http_append_auth_header(&http_auth, headers);
> +
Is http_auth headers so different from the content-type and accept
in the larger picture to warrant the blank line before this new
call? If not, you probably would want to have these three
assignments to "headers" that accumulates the header lines together
in a single block of three lines without any blank line in between.
> curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
> curl_easy_setopt(slot->curl, CURLOPT_POST, 1L);
> curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> index c1febbae9d..adc210cdd3 100755
> --- a/t/t5563-simple-http-auth.sh
> +++ b/t/t5563-simple-http-auth.sh
> @@ -63,6 +63,26 @@ test_expect_success 'setup repository' '
> git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
> '
>
> +test_expect_success 'setup large repository for probe_rpc testing' '
> + rm -rf large.git &&
> + git init large.git &&
> + (
> + cd large.git &&
> + git config set maintenance.auto false &&
> + git commit --allow-empty --message "initial" &&
> + # Create many refs to trigger probe_rpc, which is called when
> + # the request body is larger than http.postBuffer.
> + #
> + # In the test later, http.postBuffer is set to 70000. Each
> + # "want" line is ~45 bytes, so we need at least 70000/45 = ~1600
> + # refs
> + printf "create refs/heads/branch-%d @\n" $(test_seq 2000) |
> + git update-ref --stdin
> + ) &&
Hopefully, $(test_seq 2000) would not bust $(sysconf ARG_MAX), which
could be as low as 4KB, on any system we care about. If not, of
course we could
test_seq 2000 |
xargs printf "create ...\n" |
git update-ref --stdin
which probably is not all that more expensive than what you wrote above.
Other than that, looking great. Thanks for working on this.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-13 13:09 ` Junio C Hamano
@ 2026-01-14 1:06 ` Aaron Plattner
2026-01-14 2:20 ` Jeff King
0 siblings, 1 reply; 9+ messages in thread
From: Aaron Plattner @ 2026-01-14 1:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Patrick Steinhardt, Rahul Rameshbabu, Lucas De Marchi
On 1/13/26 5:09 AM, Junio C Hamano wrote:
> Aaron Plattner <aplattner@nvidia.com> writes:
>
>> Subject: Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
>
> Micronit. "Use" -> "use" (see "git shortlog --no-merges -200
> master" and notice that the usual "the first word of the sentence is
> upcased" rule does not typically apply to the word after "<area>: "
> prefix).
Thanks, sorry for missing that.
>> If a large request requires post_rpc() to call probe_rpc(), the latter
>> does not use the authorization credentials used for other requests. If
>> this fails with an HTTP 401 error and http_auth.multistage isn't set,
>> then the whole request just fails.
>
>> For example, using git-credential-msal [1], the following attempt to clone a
>> large repository fails partway through because the initial request to download
>> the commit history and promisor packs succeeds, but the
>> subsequent request to download the blobs needed to construct the working
>> tree fails with a 401 error and the checkout fails.
>>
>> (lines removed for brevity)
>>
>> git clone --filter=blob:none https://secure-server.example/repo
>> ...
>> 11:07:34.725262 http.c:849 => Send header: POST repo/git-upload-pack HTTP/1.1
>> 11:07:34.725279 http.c:849 => Send header: Content-Type: application/x-git-upload-pack-request
>> 11:07:34.761407 http.c:849 <= Recv header: HTTP/1.1 401 Unauthorized
>> 11:07:34.761443 http.c:890 == Info: Bearer authentication problem, ignoring.
>> 11:07:34.761453 http.c:849 <= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
>> 11:07:34.761509 http.c:890 == Info: The requested URL returned error: 401
>> 11:07:34.761530 http.c:890 == Info: closing connection #0
>
> Excellent illustration here.
>
>> 11:07:34.761913 run-command.c:673 trace: run_command: 'git credential-cache erase'
>> 11:07:34.761927 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-cache erase' 'git credential-cache erase'
>> 11:07:34.768069 git.c:502 trace: built-in: git credential-cache erase
>> 11:07:34.768690 run-command.c:673 trace: run_command: 'git credential-msal erase'
>> 11:07:34.768713 run-command.c:765 trace: start_command: /bin/sh -c 'git credential-msal erase' 'git credential-msal erase'
>> 11:07:34.772742 git.c:808 trace: exec: git-credential-msal erase
>> 11:07:34.772783 run-command.c:673 trace: run_command: git-credential-msal erase
>> 11:07:34.772819 run-command.c:765 trace: start_command: /usr/bin/git-credential-msal erase
>> error: RPC failed; HTTP 401 curl 22 The requested URL returned error: 401
>> fatal: unable to write request to remote: Broken pipe
>> fatal: could not fetch c4fff0229c9be06ecf576356a4d39a8a755b8d81 from promisor remote
>> warning: Clone succeeded, but checkout failed.
>> You can inspect what was checked out with 'git status'
>> and retry with 'git restore --source=HEAD :/'
>>
>> In this case, the HTTP_REAUTH retry logic is not used because the
>> credential helper didn't set the 'continue' flag, so
>> http_auth.multistage is false and handle_curl_result() fails with
>> HTTP_NOAUTH instead.
>>
>> Fix the immediate problem by including the authorization headers in the
>> probe_rpc() request as well.
>
> Great.
>
>> diff --git a/remote-curl.c b/remote-curl.c
>> index 69f919454a..1d0ae72521 100644
>> --- a/remote-curl.c
>> +++ b/remote-curl.c
>> @@ -877,6 +877,8 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
>> headers = curl_slist_append(headers, rpc->hdr_content_type);
>> headers = curl_slist_append(headers, rpc->hdr_accept);
>>
>> + headers = http_append_auth_header(&http_auth, headers);
>> +
>
> Is http_auth headers so different from the content-type and accept
> in the larger picture to warrant the blank line before this new
> call? If not, you probably would want to have these three
> assignments to "headers" that accumulates the header lines together
> in a single block of three lines without any blank line in between.
No, that's fair.
>> curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
>> curl_easy_setopt(slot->curl, CURLOPT_POST, 1L);
>> curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
>> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
>> index c1febbae9d..adc210cdd3 100755
>> --- a/t/t5563-simple-http-auth.sh
>> +++ b/t/t5563-simple-http-auth.sh
>> @@ -63,6 +63,26 @@ test_expect_success 'setup repository' '
>> git push --mirror "$HTTPD_DOCUMENT_ROOT_PATH/repo.git"
>> '
>>
>> +test_expect_success 'setup large repository for probe_rpc testing' '
>> + rm -rf large.git &&
>> + git init large.git &&
>> + (
>> + cd large.git &&
>> + git config set maintenance.auto false &&
>> + git commit --allow-empty --message "initial" &&
>> + # Create many refs to trigger probe_rpc, which is called when
>> + # the request body is larger than http.postBuffer.
>> + #
>> + # In the test later, http.postBuffer is set to 70000. Each
>> + # "want" line is ~45 bytes, so we need at least 70000/45 = ~1600
>> + # refs
>> + printf "create refs/heads/branch-%d @\n" $(test_seq 2000) |
>> + git update-ref --stdin
>> + ) &&
>
> Hopefully, $(test_seq 2000) would not bust $(sysconf ARG_MAX), which
> could be as low as 4KB, on any system we care about. If not, of
> course we could
>
> test_seq 2000 |
> xargs printf "create ...\n" |
> git update-ref --stdin
>
> which probably is not all that more expensive than what you wrote above.
That's a good call. I tried this
test_seq 2000 |
xargs printf "create refs/heads/branch-%d @\n" |
git update-ref --stdin
and verified it produces the same results, as does the same plus passing
"-n 10" to xargs.
> Other than that, looking great. Thanks for working on this.
Thanks Junio, I'll send a v4 with these updates.
-- Aaron
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-14 1:06 ` Aaron Plattner
@ 2026-01-14 2:20 ` Jeff King
2026-01-14 14:11 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Jeff King @ 2026-01-14 2:20 UTC (permalink / raw)
To: Aaron Plattner
Cc: Junio C Hamano, git, Patrick Steinhardt, Rahul Rameshbabu,
Lucas De Marchi
On Tue, Jan 13, 2026 at 05:06:20PM -0800, Aaron Plattner wrote:
> > Hopefully, $(test_seq 2000) would not bust $(sysconf ARG_MAX), which
> > could be as low as 4KB, on any system we care about. If not, of
> > course we could
> >
> > test_seq 2000 |
> > xargs printf "create ...\n" |
> > git update-ref --stdin
> >
> > which probably is not all that more expensive than what you wrote above.
>
> That's a good call. I tried this
>
> test_seq 2000 |
> xargs printf "create refs/heads/branch-%d @\n" |
> git update-ref --stdin
>
> and verified it produces the same results, as does the same plus passing "-n
> 10" to xargs.
test_seq can take a format parameter these days, so just:
test_seq -f "create refs/heads/branch-%d @" |
git update-ref --stdin
is enough, and saves some processes.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-14 2:20 ` Jeff King
@ 2026-01-14 14:11 ` Junio C Hamano
2026-01-14 16:33 ` Aaron Plattner
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2026-01-14 14:11 UTC (permalink / raw)
To: Jeff King
Cc: Aaron Plattner, git, Patrick Steinhardt, Rahul Rameshbabu,
Lucas De Marchi
Jeff King <peff@peff.net> writes:
> test_seq can take a format parameter these days, so just:
>
> test_seq -f "create refs/heads/branch-%d @" |
> git update-ref --stdin
>
> is enough, and saves some processes.
Ahh, I forgot about that one. Good suggestion.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-14 14:11 ` Junio C Hamano
@ 2026-01-14 16:33 ` Aaron Plattner
2026-01-14 17:30 ` Jeff King
2026-01-14 17:41 ` Junio C Hamano
0 siblings, 2 replies; 9+ messages in thread
From: Aaron Plattner @ 2026-01-14 16:33 UTC (permalink / raw)
To: Junio C Hamano, Jeff King
Cc: git, Patrick Steinhardt, Rahul Rameshbabu, Lucas De Marchi
On 1/14/26 6:11 AM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> test_seq can take a format parameter these days, so just:
>>
>> test_seq -f "create refs/heads/branch-%d @" |
>> git update-ref --stdin
>>
>> is enough, and saves some processes.
>
> Ahh, I forgot about that one. Good suggestion.
>
> Thanks.
Ooh, neat. I guess I copied the wrong example. I verified that this
works too, so I can send a v5 for that.
Is it worth changing up the other cases of this pattern, mostly in
pack-refs-tests.sh? E.g.,
# Create 15 loose references.
printf "create refs/heads/loose-%d HEAD\n" $(test_seq 15) >stdin &&
git update-ref --stdin <stdin &&
[...]
# Create 99 packed refs. This should cause the heuristic
# to require more than the minimum amount of loose refs.
test_seq 99 |
while read i
do
printf "create refs/heads/packed-%d HEAD\n" $i || return 1
done >stdin &&
git update-ref --stdin <stdin &&
I can put together a patch for those.
-- Aaron
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-14 16:33 ` Aaron Plattner
@ 2026-01-14 17:30 ` Jeff King
2026-01-14 17:41 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Jeff King @ 2026-01-14 17:30 UTC (permalink / raw)
To: Aaron Plattner
Cc: Junio C Hamano, git, Patrick Steinhardt, Rahul Rameshbabu,
Lucas De Marchi
On Wed, Jan 14, 2026 at 08:33:43AM -0800, Aaron Plattner wrote:
> Ooh, neat. I guess I copied the wrong example. I verified that this works
> too, so I can send a v5 for that.
>
> Is it worth changing up the other cases of this pattern, mostly in
> pack-refs-tests.sh? E.g.,
>
> # Create 15 loose references.
> printf "create refs/heads/loose-%d HEAD\n" $(test_seq 15) >stdin &&
> git update-ref --stdin <stdin &&
>
> [...]
>
> # Create 99 packed refs. This should cause the heuristic
> # to require more than the minimum amount of loose refs.
> test_seq 99 |
> while read i
> do
> printf "create refs/heads/packed-%d HEAD\n" $i || return 1
> done >stdin &&
> git update-ref --stdin <stdin &&
>
> I can put together a patch for those.
Yeah, I think they are worth updating. I looked for spots to convert
when I added the feature in b32c7ec02f (test-lib: teach test_seq the -f
option, 2025-06-23). But I missed those ones.
I think I grepped for "for i in $(test_seq ...)", but the use of the
while loop and the $()-substitution meant I didn't see them. Examining
every test_seq call didn't seem worthwhile, as there are hundreds. ;)
Grepping for "test_seq .* |" does yield a few more (e.g., p5551), but
most are false positives. Grepping for "printf.*$(test_seq" gets some
more.
I don't know that we need to exhaustively find all cases. ;) In the
first case above, it does save us a subshell. In the second case I think
it saves us a subshell _and_ the result is much easier to read (because
it avoids the loop and return). So IMHO it's a nice improvement, but
there's diminishing returns to investigating every test_seq call.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
2026-01-14 16:33 ` Aaron Plattner
2026-01-14 17:30 ` Jeff King
@ 2026-01-14 17:41 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2026-01-14 17:41 UTC (permalink / raw)
To: Aaron Plattner
Cc: Jeff King, git, Patrick Steinhardt, Rahul Rameshbabu,
Lucas De Marchi
Aaron Plattner <aplattner@nvidia.com> writes:
> Is it worth changing up the other cases of this pattern, mostly in
> pack-refs-tests.sh? E.g.,
>
> # Create 15 loose references.
> printf "create refs/heads/loose-%d HEAD\n" $(test_seq 15) >stdin &&
> git update-ref --stdin <stdin &&
>
> [...]
>
> # Create 99 packed refs. This should cause the heuristic
> # to require more than the minimum amount of loose refs.
> test_seq 99 |
> while read i
> do
> printf "create refs/heads/packed-%d HEAD\n" $i || return 1
> done >stdin &&
> git update-ref --stdin <stdin &&
>
> I can put together a patch for those.
I am fairly sure that these existing ones were written way before
the feature in test_seq to use the format parameter got popular. A
separate patch to clean them up would be a good addition but of
course should be outside the current topic ;-)
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-01-14 17:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-13 3:19 [PATCH v2] remote-curl: Use auth for probe_rpc() requests too Aaron Plattner
2026-01-13 6:49 ` Patrick Steinhardt
2026-01-13 13:09 ` Junio C Hamano
2026-01-14 1:06 ` Aaron Plattner
2026-01-14 2:20 ` Jeff King
2026-01-14 14:11 ` Junio C Hamano
2026-01-14 16:33 ` Aaron Plattner
2026-01-14 17:30 ` Jeff King
2026-01-14 17:41 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox