Re: [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0)

[Michael J Gruber <git@drmicha.warpmail.net>]

Stefan Näwe venit, vidit, dixit 02.11.2011 09:52:
> Am 01.11.2011 19:12, schrieb Junio C Hamano:
>>
>> There are only handful of commits that even remotely touch http related
>> codepath between v1.7.7 and v1.7.8-rc0:
>>
>>   * deba493 http_init: accept separate URL parameter
>>
>>   This could change the URL string given to http_auth_init().
>>
>>   * 070b4dd http: use hostname in credential description
>>
>>   This only changes the prompt string; as far as I understand it, the
>>   condition the password is prompted in the callsites of git_getpass()
>>   has not changed.
>>
>>   * 6cdf022 remote-curl: Fix warning after HTTP failure
>>   * be22d92 http: avoid empty error messages for some curl errors
>>   * 8abc508 http: remove extra newline in error message
>>   * 8d677ed http: retry authentication failures for all http requests
>>   * 28d0c10 remote-curl: don't retry auth failures with dumb protocol
>>
>>   These shouldn't affect anything wrt prompting, unless you are somehow
>>   internally reauthenticating.
>>
>> Could you try reverting deba493 and retest, and then if the behaviour is
>> the same "need ENTER", further revert 070b4dd and retest?
> 
> I did a little more testing.
> This WIP makes it work for me (i.e. "need ENTER" is gone, works with
> and without .netrc, with 'https://host/repo.git' and 
> 'https://user@host...' URL). Needs testing, of course.
> 
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> diff --git a/http.c b/http.c
> index a4bc770..008ad72 100644
> --- a/http.c
> +++ b/http.c
> @@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
>         curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>  #endif
> 
> -       init_curl_http_auth(result);
> -
>         if (ssl_cert != NULL)
>                 curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
>         if (has_cert_password())
> @@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
>                 else if (missing_target(&results))
>                         ret = HTTP_MISSING_TARGET;
>                 else if (results.http_code == 401) {
> -                       if (user_name) {
> +                       if (user_name && user_pass) {
>                                 ret = HTTP_NOAUTH;
>                         } else {
>                                 /*
> @@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
>                                  * but that is non-portable.  Using git_getpass() can at least be stubbed
>                                  * on other platforms with a different implementation if/when necessary.
>                                  */
> -                               user_name = xstrdup(git_getpass_with_description("Username", description));
> +                               if (!user_name)
> +                                       user_name = xstrdup(git_getpass_with_description("Username", description));
>                                 init_curl_http_auth(slot->curl);
>                                 ret = HTTP_REAUTH;
>                         }
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> 
> 
> Regards,
>   Stefan
Thanks!

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>

More specifically, I ran our test suite (next plus Stefan's patch), and
tested

https://user@host with .netrc and with askpass
https://host with .netrc

The latter fails with askpass because we ask
Password for 'host'
and not
Password for 'user@host'
but that is true both with and without the patch. (I thought we had
changed that, but I guess it's cooking.)

Michael