Repository navigation
Backport fix for now-flaky HTTP-auth tests - #1006
Closed
Johannes Schindelin (dscho) wants to merge 1 commit into
Closed
Johannes Schindelin (dscho) wants to merge 1 commit into
Johannes Schindelin (dscho) wants to merge 1 commit into
Conversation
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>
Matthew John Cheetham (mjcheetham)
approved these changes
Oct 15, 2026
Member
Author
|
Superseded by #1007 |
Johannes Schindelin (dscho)
added a commit
that referenced
this pull request
Oct 15, 2026
I [just ran](#1005 (comment)) into new test failures. Failures unrelated to the PR in question. And failures that have, of course, been fixed in upstream Git, but not in any tagged version. Let's backport them _real_ quick. This is a replacement for #1006 (where I mistakenly thought I could re-use the upstream branch from gitgitgadget/git without any adjustements).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I just ran into new test failures. Failures unrelated to the PR in question. And failures that have, of course, been fixed in upstream Git, but not in any tagged version.
Let's backport them real quick.