Re: [PATCH 1/2][Perlers?] git-send-email: ssh/login style password requests

Previous thread: Re: [PATCH] Documentation/git-reset.txt: Use HEAD~N syntax everywhere (unify examples) by Jari Aalto on Saturday, February 2, 2008 - 1:15 pm. (2 messages)

Next thread: Re: [PATCH 2/2][Perlers?] git-send-email: SIG{TERM,INT} handlers by Junio C Hamano on Saturday, February 2, 2008 - 2:31 pm. (1 message)
From: Junio C Hamano
Date: Saturday, February 2, 2008 - 2:31 pm

Another example which appears in PerlFAQ #8 uses ReadKey with
its ReadLine, like this:

    use Term::ReadKey;
    ReadMode('noecho');
    $password = ReadLine(0);

which is different from Term::ReadLine's "ReadLine".  An earlier
example you cited from perlfunc.pod's crypt() entry does:

    system "stty -echo";
    print "Password: ";
    chomp($word = <STDIN>);
    print "\n";
    system "stty echo";

In either case, I was worried about the interaction between the
Term::ReadLine backend implementation and "stty".

Actually, I just tried this myself:

    #!/usr/bin/perl -w

    use Term::ReadLine;
    my $term = new Term::ReadLine 'foobar';

    my ($user, $password);
    while (!defined $user) {
            $user = $term->readline("User: ");
    }
    system 'stty -echo';
    while (!defined $password) {
            $password = $term->readline("Password: ");
    }
    system 'stty echo';
    print "You said <$user><$password>\n";
    print "ReadLine backend used was ", $term->ReadLine, "\n";

In my case, the backend was "Term::ReadLine::Perl".  A few
problems:

 * After typing "junio <Enter>" to "User:", an extra newline is
   left before "Password:" prompt;

 * "Password:" prompt still echoed password "abc".  There was no
   extra newline before "You said <junio><abc>".

 * In either case, typing <Enter> returns an empty string from
   $term->readline() so the "while (!defined)" loop does not buy
   us anything.


 
-

From: Michael Witten
Date: Sunday, February 3, 2008 - 10:59 am

The crux of my conclusion comes after the ---------------------------- .



Indeed. I tested your code and my git-send-email code with all three
backend implementations (Term::ReadLine::Stub, Term::ReadLine::Gnu, and
Term::ReadLine::Perl). The problem with echoing seems to be a fault of

Frankly, I wrote that readline code according to the other uses of  
readline,
as I am not well versed in these things.

Because all other uses of readline are wrapped in such while loops, I  
assume
that they are meant to cover systems with non-blocking (unbuffered)  
IO, rather
than to continue to prompt the user due to empty strings.

Should empty-string passwords not be allowed?


This got me thinking: At first I wanted to use readline for the  
password prompt,
because I figured it would allow the user better editing facilities,  
especially
with regard to the arrow keys. However, it occurred to me that perhaps  
this should
not be the case; perhaps arrow keys are meant to be useable in  
passwords, etc.

Well, this turns out to be the case! (which may not be a surprise to  
most people,
but it was to me). Passwords can actually contain some of the keycodes  
that read-
line converts into editing commands (now you know which characters you  
don't have
to test when cracking my password). Therefore, we shouldn't even  
bother using the
readline backend for password prompting; neither passwd nor ssh use  
readline, for
example.

I support the 'crypt' method, because the Term::ReadKey approach  
requires another
module dependency.

Sincerely,
Michael Witten
-

From: Michael Witten
Date: Sunday, February 3, 2008 - 1:59 pm

Yiarg!

It would appear that Term::ReadLine::Gnu's implementation (of at least
readline) delays signal handling, so that ^c (C-d) doesn't do anything  
while
the prompt is up; I suppose this is an artifact of of this:

	http://perldoc.perl.org/perlipc.html#Deferred-Signals-(Safe-Signals)

For instance:

	What subject should the initial email start with?<^c><ENTER>



Perhaps these loops are meant to handle ^d (C-d; VEOF). This is  
because issuing
EOF causes input operators/functions to return undefined; in fact,  
using just the
'crypt' method, ^d is unhandled, causing a runtime error. Is there a  
way to setup
a signal handler for this control character rather than using checks?

Michael Witten
-

From: Michael Witten
Date: Sunday, February 3, 2008 - 5:53 pm

Whilst convenient, it is most unwise to record passwords
in any place but one's brain. Moreover, it is especially
foolish to store them in configuration files, even with
access permissions set accordingly.

git-send-email has been amended, so that if it detects
an smtp username without a password, it promptly prompts
for the password and masks the input for privacy.

Furthermore, the argument to --smtp-pass has been rendered
optional.

The documentation has been updated to reflect these changes.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---

	The backend ambiguity of Term:ReadLine is avoided, and EOF
	is handled for the Password Prompt.

 Documentation/git-send-email.txt |   39 +++++++++++++++++++++++++++++++++----
 git-send-email.perl              |   25 ++++++++++++++++++++---
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0554f2b..4f4caa4 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -96,11 +96,40 @@ The --cc option must be repeated for each user you want on the cc list.
 	servers typically listen to smtp port 25 and ssmtp port
 	465).
 
---smtp-user, --smtp-pass::
-	Username and password for SMTP-AUTH. Defaults are the values of
-	the configuration values 'sendemail.smtpuser' and
-	'sendemail.smtppass', but see also 'sendemail.identity'.
-	If not set, authentication is not attempted.
+--smtp-user::
+	Username for SMTP-AUTH. In place of this option, the following
+	configuration variables can be specified:
++
+--
+		* sendemail.smtpuser
+		* sendemail.<identity>.smtpuser (see sendemail.identity).
+--
++
+However, --smtp-user always overrides these variables.
++
+If a username is not specified (with --smtp-user or a
+configuration variable), then authentication is not attempted.
+
+--smtp-pass::
+	Password for SMTP-AUTH. The argument is optional: If no
+	argument is specified, then the empty string is used as
+	the ...
From: Michael Witten
Date: Sunday, February 3, 2008 - 5:53 pm

A single signal handler is used for both SIGTERM
and SIGINT in order to clean up after an uncouth
termination of git-send-email.

In particular, the handler resets the text color
(this cleanup was already present), turns on tty
echoing (in case termination occurrs during a
masked Password prompt), and informs the user of
of any temporary files created by --compose.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
 git-send-email.perl |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fec55ea..14268fc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -24,8 +24,6 @@ use Data::Dumper;
 use Term::ANSIColor;
 use Git;
 
-$SIG{INT} = sub { print color("reset"), "\n"; exit };
-
 package FakeTerm;
 sub new {
 	my ($class, $reason) = @_;
@@ -201,6 +199,29 @@ my %config_settings = (
     "aliasesfile" => \@alias_files,
 );
 
+# Handle Uncouth Termination
+sub signal_handler {
+
+	# Make text normal
+	print color("reset"), "\n";
+
+	# SMTP password masked
+	system "stty echo";
+
+	# tmp files from --compose
+	if (-e $compose_filename) {
+		print "'$compose_filename' contains an intermediate version of the email you were composing.\n";
+	}
+	if (-e ($compose_filename . ".final")) {
+		print "'$compose_filename.final' contains the composed email.\n"
+	}
+
+	exit;
+};
+
+$SIG{TERM} = \&signal_handler;
+$SIG{INT}  = \&signal_handler;
+
 # Begin by accumulating all the variables (defined above), that we will end up
 # needing, first, from the command line:
 
-- 
1.5.4.9.gcc769-dirty

-

From: Michael Witten
Date: Sunday, February 3, 2008 - 5:53 pm

Before, when the user sent the EOF control character,
the prompts would be repeated on the same line as the
previous prompt.

Now, repeat prompts display on separate lines.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
 git-send-email.perl |   46 +++++++++++++++++++++++++++++-----------------
 1 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 14268fc..6724810 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -376,9 +376,12 @@ if (@files) {
 my $prompting = 0;
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter;
-	do {
+
+	while (1) {
 		$_ = $term->readline("Who should the emails appear to be from? [$sender] ");
-	} while (!defined $_);
+		last if defined $_;
+		print "\n";
+	}
 
 	$sender = $_ if ($_);
 	print "Emails will be sent from: ", $sender, "\n";
@@ -386,10 +389,14 @@ if (!defined $sender) {
 }
 
 if (!@to) {
-	do {
-		$_ = $term->readline("Who should the emails be sent to? ",
-				"");
-	} while (!defined $_);
+
+
+	while (1) {
+		$_ = $term->readline("Who should the emails be sent to? ", "");
+		last if defined $_;
+		print "\n";
+	}
+
 	my $to = $_;
 	push @to, split /,/, $to;
 	$prompting++;
@@ -411,20 +418,23 @@ sub expand_aliases {
 @bcclist = expand_aliases(@bcclist);
 
 if (!defined $initial_subject && $compose) {
-	do {
-		$_ = $term->readline("What subject should the initial email start with? ",
-			$initial_subject);
-	} while (!defined $_);
+	while (1) {
+		$_ = $term->readline("What subject should the initial email start with? ", $initial_subject);
+		last if defined $_;
+		print "\n";
+	}
+
 	$initial_subject = $_;
 	$prompting++;
 }
 
 if ($thread && !defined $initial_reply_to && $prompting) {
-	do {
-		$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ",
-			$initial_reply_to);
-	} while (!defined $_);
-
+	while (1) {
+		$_= $term->readline("Message-ID to be used as In-Reply-To for the ...
Previous thread: Re: [PATCH] Documentation/git-reset.txt: Use HEAD~N syntax everywhere (unify examples) by Jari Aalto on Saturday, February 2, 2008 - 1:15 pm. (2 messages)

Next thread: Re: [PATCH 2/2][Perlers?] git-send-email: SIG{TERM,INT} handlers by Junio C Hamano on Saturday, February 2, 2008 - 2:31 pm. (1 message)