Re: [PATCHv4 6/6] git-send-email: use git credential to obtain password
On Wed, Feb 27 2013, Junio C Hamano gits...@pobox.com wrote:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Michal Nazarewicz m...@google.com writes:
+ $auth = Git::credential({
+ 'protocol' = 'smtp',
+ 'host' = join(':', $smtp_server, $smtp_server_port),
At this point, $smtp_server_port is not always defined. I just tested
and got
Use of uninitialized value $smtp_server_port in join or string at
git-send-email line 1077.
Other than that, the whole series looks good.
Given that there is another place that conditionally append :$port
to the host string, I think we should follow suit here. Perhaps
like the attached diff?
Damn meetings, you beat me to it… I was just about to send a patch. ;)
Thanks for a review.
git-send-email.perl | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 76bbfc3..c3501d9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,14 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+sub smtp_host_string {
+ if (defined $smtp_server_port) {
+ return $smtp_server:$smtp_server_port;
+ } else {
+ return $smtp_server;
+ }
+}
+
# Returns 1 if authentication succeeded or was not necessary
# (smtp_user was not specified), and 0 otherwise.
@@ -1065,7 +1073,7 @@ sub smtp_auth_maybe {
# reject credentials.
$auth = Git::credential({
'protocol' = 'smtp',
- 'host' = join(':', $smtp_server, $smtp_server_port),
+ '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.
@@ -1188,9 +1196,7 @@ sub send_message {
else {
require Net::SMTP;
$smtp_domain ||= maildomain();
- $smtp ||= Net::SMTP-new((defined $smtp_server_port)
- ?
$smtp_server:$smtp_server_port
- : $smtp_server,
+ $smtp ||= Net::SMTP-new(smtp_host_string(),
Hello = $smtp_domain,
Debug = $debug_net_smtp);
if ($smtp_encryption eq 'tls' $smtp) {
From reading of SMTP.pm, it seems that this could be changed to:
- $smtp ||= Net::SMTP-new((defined $smtp_server_port)
-?
$smtp_server:$smtp_server_port
-: $smtp_server,
+ $smtp ||= Net::SMTP-new($smtp_server,
+Port = $smtp_server_port,
and than the other part would become:
@@ -1060,12 +1060,17 @@ sub smtp_auth_maybe {
Authen::SASL-import(qw(Perl));
};
+ my $host = $smtp_server;
+ if (defined $smtp_server_port) {
+ $host .= ':' . $smtp_server_port;
+ }
+
# 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' = join(':', $smtp_server, $smtp_server_port),
+ 'host' = $host,
'username' = $smtp_authuser,
# if there's no password, git credential fill will
# give us one, otherwise it'll just pass this one.
Either way, looks good to me.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--
pgpjon85cv7ZQ.pgp
Description: PGP signature
[PATCHv4 6/6] git-send-email: use git credential to obtain password
From: Michal Nazarewicz min...@mina86.com
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++-
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=host::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL-import(qw(Perl));
+ };
+
+ # 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' = join(':', $smtp_server, $smtp_server_port),
+ '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'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ?
port=$smtp_server_port : ;
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL-import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system stty -echo;
-
- do {
- print Password: ;
- $_ = STDIN;
- print \n;
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system stty echo;
- }
-
- $auth ||= $smtp-auth( $smtp_authuser, $smtp_authpass )
or die $smtp-message;
- }
+ smtp_auth_maybe or die $smtp-message;
$smtp-mail( $raw_from ) or die $smtp-message;
$smtp-to( @recipients ) or die $smtp-message;
--
1.8.1.3.572.g32bae1f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
