From: Michael Witten <mfwitten@mit.edu>
Date: Fri, 01 Feb 2008 18:38:47 GMT
Subject: Re: [PATCH] git-send-email: ssh/login style password requests
Message-ID: <FE992810-2B2E-4C23-A7D3-AD4C39D925ED@MIT.EDU>
In-Reply-To: <7vodb19edp.fsf@gitster.siamese.dyndns.org>


On 1 Feb 2008, at 5:09 AM, Junio C Hamano wrote:

> Michael Witten <mfwitten@MIT.EDU> writes:
>
>> +--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.

How are these descriptions:

	http://web.mit.edu/mfwitten/git-send-email.html

I've attempted to keep them short, but complete. Also, the
resulting man page versions are formatted similarly (though
at the expense of some ASCIIdoc readability).

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

Actually, a lack of *.smtp<user|pass> results in variables without
definition rather than with empty-string values; the original code,
too, makes use of this fact. Consequently, an empty string for
*.smtppass does not result in prompting, as tested.

>> +		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

Good question! Though I'm not sure you can handle this any other
way than to intercept the signal.

> (2) $term->readline() interface might already have a method to turn  
> echo off.

I have very limited knowledge of these things, especially of Perl's  
interfaces,
but this is what `perldoc -q password` has to say about it:

> 	There's an example of this in "crypt" in perlfunc).
> 	First, you put the terminal into "no echo" mode,
> 	then just read the password normally.  You may do
> 	this with an old-style ioctl() function, POSIX
> 	terminal control (see POSIX or its documentation
> 	the Camel Book), or a call to the stty program,
> 	with varying degrees of portability.
> 	
> 	You can also do this for most systems using the
> 	Term::ReadKey module from CPAN, which is easier
> 	to use and in theory more portable.
> 	
> 	   use Term::ReadKey;
> 	
> 	   ReadMode('noecho');
> 	   $password = ReadLine(0);

My code is from the "crypt" version. The other versions require
extra module dependencies, and the portability doesn't extend
immediately to Windows with any of these methods.

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

Great!

Sincerely,
Michael Witten