Thread (6 messages) 6 messages, 3 authors, 2026-01-12

Re: [PATCH] remote-curl: Use auth for probe_rpc() requests too

From: Patrick Steinhardt <hidden>
Date: 2026-01-12 08:22:02

On Fri, Jan 09, 2026 at 10:39:10AM -0800, Aaron Plattner wrote:
On 1/9/26 9:57 AM, Aaron Plattner wrote:
quoted
On 1/9/26 6:51 AM, Patrick Steinhardt wrote:
[...]
quoted
quoted
quoted
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);
The change looks simple enough, and matches what we do in `post_rpc()`
itself.

It would be great to have a test case for this. It might be possible to
use t5563-simple-http-auth as an example, where we already know to set
up an HTTP server with authentication.
I'll look into that. It wasn't obvious to me how to make it hit this RPC
case specifically but I'll see if I can figure out a way.
I asked AI to try generating a test case for me and it discovered that the
problem doesn't reproduce with Basic auth because git sets CURLOPT_USERNAME
and CURLOPT_PASSWORD and curl implicitly includes those in subsequent
requests without git having to add them explicitly. If we used
CURLOPT_XOAUTH2_BEARER like imap-send.c does, then curl would presumably do
the same thing behind the scenes.

That said, I'm not sure using that makes sense since the credential helper
just tells git to use Bearer auth and what the token is, but not whether
it's OAuth2 or some other kind of token. I don't know if that matters.
Rahul, do you have any opinions there since you're familiar with this stuff
than I am?

Anyway, the test it came up with creates a repository with 2000 branches to
get the reply to hit the large_request=1 case and then uses a simple
credential helper with a dummy Bearer token to trigger the problem. If you
think the current fix and that test scenario sound reasonable, I'll clean it
up and send out a v2.
Creating 2000 branches can be done efficiently via a single
git-update-ref(1) call, so this wouldn't cause the test to become
prohibitively expensive. And if that manages to reproduce the problem it
sounds like a reasonable way forward.

Thanks!

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help