Re: [PATCH] http-push: don't always prompt for password

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2011, 16:48 UTC
Message-ID: <7vlirvdeb2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1320390188-24334-1-git-send-email-stefan.naewe@gmail.com>

Stefan Naewe <stefan.naewe@gmail.com> writes:

Show 8 quoted lines

> http-push prompts for a password when the URL is set as
> 'https://user@host/repo' even though there is one set
> in ~/.netrc. Pressing ENTER at the password prompt succeeds
> then, but is a annoying and makes it almost useless
> in a shell script, e.g.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> ---

Thanks.

With this the only callsite of init_curl_http_auth() becomes the one after we get the 401 response, and this caller makes sure that user_name is not NULL.

Do we still want "if (user_name)" inside init_curl_http_auth()?

I tried to rewrite the proposed commit log message to describe the real issue, and here is what I came up with:

Author: Stefan Naewe <stefan.naewe@gmail.com>
Date:   Fri Nov 4 08:03:08 2011 +0100

    http: don't always prompt for password
    
    When a username is already specified at the beginning of any HTTP
    transaction (e.g. "git push https://user@hosting.example.com/project.git"
    or "git ls-remote https://user@hosting.example.com/project.git"), the code
    interactively asks for a password before calling into the libcurl library.
    It is very likely that the reason why user included the username in the
    URL is because the user knows that it would require authentication to
    access the resource. Asking for the password upfront would save one
    roundtrip to get a 401 response, getting the password and then retrying
    the request. This is a reasonable optimization.
    
    HOWEVER.
    
    This is done even when $HOME/.netrc might have a corresponding entry to
    access the site, or the site does not require authentication to access the
    resource after all. But neither condition can be determined until we call
    into libcurl library (we do not read and parse $HOME/.netrc ourselves). In
    these cases, the user is forced to respond to the password prompt, only to
    give a password that is not used in the HTTP transaction. If the password
    is in $HOME/.netrc, an empty input would later let the libcurl layer to
    pick up the password from there, and if the resource does not require
    authentication, any input would be taken and then discarded without
    getting used. It is wasteful to ask this unused information to the end
    user.
    
    Reduce the confusion by not trying to optimize for this case and always
    incur roundtrip penalty. An alternative might be to document this and keep
    this round-trip optimization as-is.
    
    Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
    Helped-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

What is somewhat troubling is that after analyzing the root cause of the issue, I am wondering if a more correct fix is to remove the user@ part from the URL (in other words, document that a URL with an embedded username will ask for password upfront, and tell the users that if they have netrc entries or if they are accessing a resource that does not require authentication, they should omit the username from the URL).