Re: [PATCH] remote-curl: Use auth for probe_rpc() requests too
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 12, 2026, 08:21 UTC
- Message-ID
- <aWSvJbYBWpJc-Vcc@pks.im>
- In-Reply-To
- <c09387eb-0847-4130-85d1-9da8a3f64164@nvidia.com>
On Fri, Jan 09, 2026 at 10:39:10AM -0800, Aaron Plattner wrote:
Show 45 quoted lines
> On 1/9/26 9:57 AM, Aaron Plattner wrote: > > On 1/9/26 6:51 AM, Patrick Steinhardt wrote: > [...] > > > > 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