Skip to content

Backport fix for now-flaky HTTP-auth tests - #6489

Merged
dscho merged 3 commits into
mainfrom
g4w/jk/http-curl-strip-creds
Oct 16, 2026
Merged

dscho merged 3 commits into
mainfrom
g4w/jk/http-curl-strip-creds

Conversation

@dscho

@dscho dscho commented Oct 15, 2026

Copy link
Copy Markdown
Member

I ran into these new failures (caused by an upgrade to libcurl v8.23.0 in CI builds) in Microsoft Git and fixed them via microsoft#1007; This is a companion to fast-track the same fix into Git for Windows.

peff and others added 3 commits September 28, 2026 10:49
When we detect that curl performed a redirect of a URL we requested, we
update our base URL to match the new location and flush the http_auth
credentials. This goes back to c93c92f (http: update base URLs when
we see redirects, 2013-09-28).

We detect the redirect by comparing the requested URL to the response
from CURLINFO_EFFECTIVE_URL, using a simple string comparison. This has
worked fine for years, but a change in the upcoming curl 8.23.0 adds a
complication. If our URL directly contains credentials (like
"https://user:pass@example.com/foo.git"), then as of 7a6bd027d0
(getinfo: make sure CURLINFO_EFFECTIVE_URL does not contain creds,
2026-09-21), curl will strip the credentials from what it returns (so
just "https://example.com/foo.git" in this case).

This breaks our direct string comparison, and we believe that we've been
redirected. We flush our http_auth credentials, and now subsequent
requests will use the reduced URL, causing us to re-request credentials
from the user. Notably this causes t5550.15 (among others) to complain;
it tries a clone with credentials in the URL, and fails if the user is
prompted at all.

We can handle this new behavior by doing a more careful comparison: if
the direct string comparison fails, we'll strip out the credentials
ourselves and compare. This is a little extra work, but in practice it
should only happen once per process.

I've used curl's curl_url() interface to do the stripping here, mostly
because its behavior should match the stripping it does internally. And
also, though we have code to parse a URL, we don't have any to
reconstruct it, making a single string comparison hard.

One alternative would be to parse with url_parse() or similar, and
compare the individual fields (skipping username/password). I think that
would probably also work in practice, but it seemed to me that the
simplest change would be sticking with string comparisons.

The curl_url() interface appeared in 7.62.0. We document that 7.61.0 is
still supported, so I've made it conditional here. Only new versions
strip the result from CURLINFO_EFFECTIVE_URL, so it's OK for very old
versions to skip the extra comparison. Likewise if we encounter any
errors, we just quietly skip the comparison. That's fine if you don't
have creds in your URLs, and if you do, you'll get end up in the
existing error path (a redirect warning, and eventually an auth
failure).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `jk/http-curl-strip-creds` topic introduces callers to yet-uncovered
cURL library functions, which we hereby cover in the lazyload-curl shim.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The HTTP transport has been updated to handle changes in recent curl
versions that strip credentials from the effective URL, preventing
unnecessary credential prompts during redirects.

* jk/http-curl-strip-creds:
  http: handle curl stripping creds from effective url

Backported to Git for Windows, to address the newly-flaky t5540 and
t5550 tests (because CI updated libcurl to v8.23.0).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho

dscho commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

The win+VS build (push) CI check is failing because the PR branch does not include 38068e6; However, the (pull_request) build runs on the merge commit, which does include it, and succeeds. So I'll just merge this here PR now.

@dscho
dscho merged commit 5cf10d7 into main Oct 16, 2026
200 of 203 checks passed
@dscho
dscho deleted the g4w/jk/http-curl-strip-creds branch October 16, 2026 10:04
gitforwindowshelper Bot pushed a commit that referenced this pull request Oct 16, 2026
I ran into these new failures (caused by an upgrade to libcurl v8.23.0
in CI builds) in Microsoft Git and fixed them via
microsoft#1007; This is a companion to
fast-track the same fix into Git for Windows.
gitforwindowshelper Bot pushed a commit that referenced this pull request Oct 16, 2026
I ran into these new failures (caused by an upgrade to libcurl v8.23.0
in CI builds) in Microsoft Git and fixed them via
microsoft#1007; This is a companion to
fast-track the same fix into Git for Windows.
gitforwindowshelper Bot pushed a commit that referenced this pull request Oct 16, 2026
I ran into these new failures (caused by an upgrade to libcurl v8.23.0
in CI builds) in Microsoft Git and fixed them via
microsoft#1007; This is a companion to
fast-track the same fix into Git for Windows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants