Re: [[PATCH v2]] git-send-email: Added the ability to query the number of smtp password questions
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 12, 2013, 17:57 UTC
- Message-ID
- <xmqqwqkdh59o.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1384084613-12260-2-git-send-email-silvio@port1024.net>
silvio@port1024.net writes:
Show 7 quoted lines
> From: Silvio F <silvio.fricke@gmail.com> > > With this patch "git-send-mail" ask a configurable number of questions to > input the smtp password. Without this patch we have only one trial. > > Signed-off-by: Silvio F <silvio.fricke@gmail.com> > ---
I wonder if Git::credential (or even the underlying lower level credential_fill() in the helper API) should give hints to the caller if calling it again may yield a different result. An interactive prompt may allow the user to mistype the password and then a later call may return a correct one, but the .netrc helper will read from the file and will return a fixed result, so there is no use calling credential_fill() again. And in the latter case, you do not want to loop $askpasswordcount times.
I also have to wonder if this logic belongs to git-send-email. Specifically, I wonder if we can place the looping logic in Git::credential, so that other users of the library can take advantage of it?
Thanks.
[jc: cc'ed peff@ for thoughts on credential helper API]
Show 83 quoted lines
> Documentation/git-send-email.txt | 4 ++++
> git-send-email.perl | 32 +++++++++++++++++++++-----------
> 2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index f0e57a5..ac993d6 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -364,6 +364,10 @@ sendemail.confirm::
> one of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'
> in the previous section for the meaning of these values.
>
> +sendmail.askpasswordcount::
> + Number of times the smtp password can be entered before sending mail is
> + aborted. Default is 1.
> +
> EXAMPLE
> -------
> Use gmail as the smtp server
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 3782c3b..aeb2e6d 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -203,6 +203,7 @@ my ($validate, $confirm);
> my (@suppress_cc);
> my ($auto_8bit_encoding);
> my ($compose_encoding);
> +my ($askpasswordcount) = 1;
>
> my ($debug_net_smtp) = 0; # Net::SMTP, see send_message()
>
> @@ -237,6 +238,7 @@ my %config_settings = (
> "from" => \$sender,
> "assume8bitencoding" => \$auto_8bit_encoding,
> "composeencoding" => \$compose_encoding,
> + "askpasswordcount" => \$askpasswordcount
> );
>
> my %config_path_settings = (
> @@ -360,6 +362,10 @@ sub read_config {
> }
> }
>
> + if ($askpasswordcount < 1) {
> + $askpasswordcount = 1;
> + }
> +
> if (!defined $smtp_encryption) {
> my $enc = Git::config(@repo, "$prefix.smtpencryption");
> if (defined $enc) {
> @@ -1069,17 +1075,21 @@ sub smtp_auth_maybe {
> # TODO: Authentication may fail not because credentials were
> # invalid but due to other reasons, in which we should not
> # reject credentials.
> - $auth = Git::credential({
> - 'protocol' => 'smtp',
> - 'host' => smtp_host_string(),
> - 'username' => $smtp_authuser,
> - # if there's no password, "git credential fill" will
> - # give us one, otherwise it'll just pass this one.
> - 'password' => $smtp_authpass
> - }, sub {
> - my $cred = shift;
> - return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
> - });
> + for my $i (1 .. $askpasswordcount) {
> + $auth = Git::credential({
> + 'protocol' => 'smtp',
> + 'host' => smtp_host_string(),
> + 'username' => $smtp_authuser,
> + # if there's no password, "git credential fill" will
> + # give us one, otherwise it'll just pass this one.
> + 'password' => $smtp_authpass
> + }, sub {
> + my $cred = shift;
> + return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
> + });
> +
> + last if ($auth);
> + }
>
> return $auth;
> }