Thread (5 messages) flat view 5 messages, 2 authors, 2016-06-15

Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth

From: Brian Norris <computersforpeace@gmail.com>
Date: 2016-06-15 23:06:37

On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote:
quoted hunk ↗ jump to hunk
On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote:
quoted
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {
 
 	# Check mechanism naming as defined in:
 	# https://tools.ietf.org/html/rfc4422#page-8
-	if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
+	if ($smtp_auth && $smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
 		die "invalid smtp auth: '${smtp_auth}'";
 	}
Thanks, makes sense. I wonder if moving the check to the point where
$smtp_auth is actually used (despite the noisier diff) would be cleaner,
like this:
--- 8< ---
diff --git a/git-send-email.perl b/git-send-email.perl
index c5a3f76..2a5ceda 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1134,12 +1134,6 @@ sub smtp_auth_maybe {
 		Authen::SASL->import(qw(Perl));
 	};
 
-	# Check mechanism naming as defined in:
-	# https://tools.ietf.org/html/rfc4422#page-8
-	if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
-		die "invalid smtp auth: '${smtp_auth}'";
-	}
-
 	# TODO: Authentication may fail not because credentials were
 	# invalid but due to other reasons, in which we should not
 	# reject credentials.
@@ -1154,6 +1148,12 @@ sub smtp_auth_maybe {
 		my $cred = shift;
 
 		if ($smtp_auth) {
+			# Check mechanism naming as defined in:
+			# https://tools.ietf.org/html/rfc4422#page-8
+			if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
+				die "invalid smtp auth: '${smtp_auth}'";
+			}
+
 			my $sasl = Authen::SASL->new(
 				mechanism => $smtp_auth,
 				callback => {
--- 8< ---
By moving the patch into the sub-subroutine (is this a lambda? I'm not
too familiar with my perl), I think you change the order of the checks.
So, previously, initial password auth would happen after the
$smtp_auth format check. With your patch, it comes first.

My patch:

  $ git send-email --smtp-auth="@" ...
  ...
  Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
  invalid smtp auth: '@' at ./git-send-email.perl line 1140.

Your patch:

  $ git send-email --smtp-auth="@" ...
  ...
  Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
  Password for xxxx:
  invalid smtp auth: '@' at ./git-send-email.perl line 1155.

Seems like the former is a little better, so you don't have to waste
time with your password too many times.

Regards,
Brian
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help