git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Naewe <stefan.naewe@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net
Subject: Re: [PATCH] http-push: don't always prompt for password
Date: Fri, 04 Nov 2011 09:48:17 -0700	[thread overview]
Message-ID: <7vlirvdeb2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1320390188-24334-1-git-send-email-stefan.naewe@gmail.com> (Stefan Naewe's message of "Fri, 4 Nov 2011 08:03:08 +0100")

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

> 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).

  reply	other threads:[~2011-11-04 16:48 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-31  5:00 [ANNOUNCE] Git 1.7.8.rc0 Junio C Hamano
2011-10-31 14:17 ` Stefan Näwe
2011-10-31 17:19   ` Junio C Hamano
2011-11-01  9:53   ` Stefan Näwe
2011-11-01 18:12     ` Junio C Hamano
2011-11-01 18:19       ` Jeff King
2011-11-01 20:06       ` Stefan Naewe
2011-11-01 20:18         ` Stefan Naewe
2011-11-02 10:27           ` Michael J Gruber
2011-11-02 18:03           ` Jeff King
2011-11-02 18:10             ` Jeff King
2011-11-02 19:13               ` Junio C Hamano
2011-11-02 20:09                 ` Jeff King
2011-11-03 23:02                   ` Junio C Hamano
2011-11-01 21:53         ` Stefan Naewe
2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
2011-11-02 14:08         ` Michael J Gruber
2011-11-02 17:13         ` [RFC/PATCH] http-push: don't always prompt for password Junio C Hamano
2011-11-02 17:23           ` Jeff King
2011-11-02 17:40             ` Junio C Hamano
2011-11-04  7:03               ` [PATCH] " Stefan Naewe
2011-11-04 16:48                 ` Junio C Hamano [this message]
2011-11-04 17:43                   ` Jeff King
2011-11-04 19:06                     ` Junio C Hamano
2011-11-04 18:34                   ` Stefan Naewe
2011-11-05  6:45                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vlirvdeb2.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=stefan.naewe@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).