Re: [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0)
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Nov 2, 2011, 14:08 UTC
- Message-ID
- <4EB14EC8.2070400@drmicha.warpmail.net>
- In-Reply-To
- <4EB104EA.2040001@atlas-elektronik.com>
Stefan Näwe venit, vidit, dixit 02.11.2011 09:52:
Show 70 quoted lines
> 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,
> StefanThanks!
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