On Wed, Feb 22, 2017 at 01:16:33PM -0800, Junio C Hamano wrote: > David Turner <David.Turner@xxxxxxxxxxxx> writes: > > > Always, no. For failed authentication (or authorization), apparently, yes. > > I tested this by setting the variable to false and then true, and trying to > > Push to a github repository which I didn't have write access to, with > > both an empty username (https://@:github.com/...) and no username > > (http://github.com/...). I ran this under GIT_CURL_VERBOSE=1 and > > I saw two 401 responses in the "http.emptyauth=true" case and one > > in the false case. I also tried with a repo that I did have access to (first > > configuring the necessary tokens for HTTPS push access), and saw two > > 401 responses in *both* cases. > > Thanks; that matches my observation. I do not think we care about > an extra roundtrip for the failure case, but as long as we do not > increase the number of roundtrip in the normal case, we can declare > that this is an improvement. I am not quite sure where that extra > 401 comes from in the normal case, and that might be an indication > that we already are doing something wrong, though. This patch drops the useless probe request: diff --git a/http.c b/http.c index 943e630ea..7b4c2db86 100644 --- a/http.c +++ b/http.c @@ -1663,6 +1663,9 @@ static int http_request(const char *url, curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL, options->effective_url); + if (results.auth_avail == CURLAUTH_BASIC) + http_auth_methods = CURLAUTH_BASIC; + curl_slist_free_all(headers); strbuf_release(&buf); but setting http.emptyauth adds back in the useless request. I think that could be fixed by skipping the empty-auth thing when http_auth_methods does not have CURLAUTH_NEGOTIATE in it (or perhaps other methods need it to, so maybe skip it if _just_ BASIC is set). I suspect the patch above could probably be generalized as: /* cut out methods we know the server doesn't support */ http_auth_methods &= results.auth_avail; and let curl figure it out from there. -Peff