Re: [PATCH] git-send-email: ssh/login style password requests

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 1, 2008, 10:09 UTC
Message-ID: <7vodb19edp.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1201841954-17949-1-git-send-email-mfwitten@mit.edu>

Michael Witten <mfwitten@MIT.EDU> writes:

Show 10 quoted lines

> +--smtp-pass::
> +	Password for SMTP-AUTH. The default value can be specified
> +	with the configuration variable 'sendemail.smtppass' or
> +	sendemail.<identity>.smtppass (see sendemail.identity).
> +	The argument is optional: If no argument is specified, then
> +	the password is assumed to be the empty string.
> +	If a username has been set, but no password has been set,
> +	the user is prompted for a password with masked input for
> +	privacy; passwords need not be recorded in configuration
> +	files.

I am a bit puzzled about the above description, though. It is not clear if there is any difference between "the empty string" password, and "no password has been set". It makes me wonder what the possible cases are.

 (1) no *.smtppass configuration present; no --smtp-pass parameter
     is given on the command line.

 (2) *.smtppass configuration present but is an empty string; no
     --smtp-pass parameter is given on the command line.

 (3) *.smtppass configuration present with a non-empty string; no
     --smtp-pass parameter is given on the command line.

 (4) parameter is given with an empty string (i.e. "--smtp-pass=")
     on the command line.

 (5) parameter is given with a non-empty string (i.e. "--smtp-pass=foo")
     on the command line.

My _guess_ is that command line always trump configuration so in cases (4) and (5) it does not matter what you have (or do not have) in *.smtppass configuration. In other cases, lack of *.smtppass and having *.smtppass with an empty string as its value are equivalent. And in any case, an empty string (or lack of specification) results in prompting.

But you shouldn't make _me_ guess. You certainly would be forcing many other people to guess with this.

Show 15 quoted lines

> +		if (defined $smtp_authuser) {
> +
> +			if (!defined $smtp_authpass) {
> +				
> +				system "stty -echo";
> +
> +				do {
> +					$_ = $term->readline("Password: ");
> +				} while (!defined $_);
> +
> +				system "stty echo";
> +
> +				$smtp_authpass = $_ if ($_);
> +			}
> +

I like what the patch tries to do, but the system() there feels a tad ugly. In addition it makes me wonder (1) what happens if you ^C out from this while loop, and (2) $term->readline() interface might already have a method to turn echo off.

We luckily have people handy at Perl on the list, so I'll wait for our resident Perl experts to suggest a better alterantive.

We are not in a hurry, so I expect a polished resubmit after 1.5.4. Thanks.