* [PATCH] send-email: fix uninitialized var warning for $smtp_auth @ 2015-09-18 22:12 Brian Norris 2015-09-18 22:25 ` Eric Sunshine 2015-09-21 17:52 ` Junio C Hamano 0 siblings, 2 replies; 6+ messages in thread From: Brian Norris @ 2015-09-18 22:12 UTC (permalink / raw) To: git; +Cc: Brian Norris, Jan Viktorin, Eric Sunshine, Junio C Hamano On the latest version of git-send-email, I see this error just before running SMTP auth (I didn't provide any --smtp-auth= parameter): Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139. Signed-off-by: Brian Norris <computersforpeace@gmail.com> Cc: Jan Viktorin <viktorin@rehivetech.com> Cc: Eric Sunshine <sunshine@sunshineco.com> Cc: Junio C Hamano <gitster@pobox.com> --- git-send-email.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index c5a3f766f7fd..e3ff44b4d0cd 100755 --- 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}'"; } -- 2.6.0.rc0.131.gf624c3d ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth 2015-09-18 22:12 [PATCH] send-email: fix uninitialized var warning for $smtp_auth Brian Norris @ 2015-09-18 22:25 ` Eric Sunshine 2015-09-18 22:42 ` Brian Norris 2015-09-21 17:52 ` Junio C Hamano 1 sibling, 1 reply; 6+ messages in thread From: Eric Sunshine @ 2015-09-18 22:25 UTC (permalink / raw) To: Brian Norris; +Cc: git, Jan Viktorin, Junio C Hamano On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote: > On the latest version of git-send-email, I see this error just before > running SMTP auth (I didn't provide any --smtp-auth= parameter): > > Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > git-send-email.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index c5a3f766f7fd..e3ff44b4d0cd 100755 > --- 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< --- ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth 2015-09-18 22:25 ` Eric Sunshine @ 2015-09-18 22:42 ` Brian Norris 2015-09-18 22:47 ` Eric Sunshine 0 siblings, 1 reply; 6+ messages in thread From: Brian Norris @ 2015-09-18 22:42 UTC (permalink / raw) To: Eric Sunshine; +Cc: git, Jan Viktorin, Junio C Hamano On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote: > On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote: > > --- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth 2015-09-18 22:42 ` Brian Norris @ 2015-09-18 22:47 ` Eric Sunshine 2015-09-18 22:50 ` Brian Norris 0 siblings, 1 reply; 6+ messages in thread From: Eric Sunshine @ 2015-09-18 22:47 UTC (permalink / raw) To: Brian Norris; +Cc: Git List, Jan Viktorin, Junio C Hamano On Fri, Sep 18, 2015 at 6:42 PM, Brian Norris <computersforpeace@gmail.com> wrote: > On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote: >> 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, > > 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. Makes sense (again). Thanks for the explanation. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth 2015-09-18 22:47 ` Eric Sunshine @ 2015-09-18 22:50 ` Brian Norris 0 siblings, 0 replies; 6+ messages in thread From: Brian Norris @ 2015-09-18 22:50 UTC (permalink / raw) To: Eric Sunshine; +Cc: Git List, Jan Viktorin, Junio C Hamano On Fri, Sep 18, 2015 at 06:47:20PM -0400, Eric Sunshine wrote: > Makes sense (again). Thanks for the explanation. No problem. Thanks for the review. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth 2015-09-18 22:12 [PATCH] send-email: fix uninitialized var warning for $smtp_auth Brian Norris 2015-09-18 22:25 ` Eric Sunshine @ 2015-09-21 17:52 ` Junio C Hamano 1 sibling, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2015-09-21 17:52 UTC (permalink / raw) To: Brian Norris; +Cc: git, Jan Viktorin, Eric Sunshine Brian Norris <computersforpeace@gmail.com> writes: > On the latest version of git-send-email, I see this error just before > running SMTP auth (I didn't provide any --smtp-auth= parameter): > > Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > Cc: Jan Viktorin <viktorin@rehivetech.com> > Cc: Eric Sunshine <sunshine@sunshineco.com> > Cc: Junio C Hamano <gitster@pobox.com> > --- > git-send-email.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index c5a3f766f7fd..e3ff44b4d0cd 100755 > --- 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-21 17:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-18 22:12 [PATCH] send-email: fix uninitialized var warning for $smtp_auth Brian Norris 2015-09-18 22:25 ` Eric Sunshine 2015-09-18 22:42 ` Brian Norris 2015-09-18 22:47 ` Eric Sunshine 2015-09-18 22:50 ` Brian Norris 2015-09-21 17:52 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).