{"thread":{"id":"40383","subject":"[PATCH] send-email: fix uninitialized var warning for $smtp_auth","startedAt":"2015-09-18T22:12:50Z","lastAt":"2015-09-21T17:52:36Z","messageCount":6,"participants":["Brian Norris","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"270319","messageId":"1442614370-39880-1-git-send-email-computersforpeace@gmail.com","threadId":"40383","inReplyTo":null,"subject":"[PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Brian Norris","fromEmail":"computersforpeace@gmail.com","sentAt":"2015-09-18T22:12:50Z","receivedAt":"2015-09-18T22:12:50Z","isPatch":true,"sender":{"key":"computersforpeace@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2268082?v=4"},"body":"On the latest version of git-send-email, I see this error just before\nrunning SMTP auth (I didn't provide any --smtp-auth= parameter):\n\n  Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139.\n\nSigned-off-by: Brian Norris <computersforpeace@gmail.com>\nCc: Jan Viktorin <viktorin@rehivetech.com>\nCc: Eric Sunshine <sunshine@sunshineco.com>\nCc: Junio C Hamano <gitster@pobox.com>\n---\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex c5a3f766f7fd..e3ff44b4d0cd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {\n \n \t# Check mechanism naming as defined in:\n \t# https://tools.ietf.org/html/rfc4422#page-8\n-\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n+\tif ($smtp_auth && $smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n \n-- \n2.6.0.rc0.131.gf624c3d\n"},{"id":"270320","messageId":"20150918222524.GA22410@flurp.local","threadId":"40383","inReplyTo":"1442614370-39880-1-git-send-email-computersforpeace@gmail.com","subject":"Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-18T22:25:24Z","receivedAt":"2015-09-18T22:25:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote:\n> On the latest version of git-send-email, I see this error just before\n> running SMTP auth (I didn't provide any --smtp-auth= parameter):\n> \n>   Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139.\n> \n> Signed-off-by: Brian Norris <computersforpeace@gmail.com>\n> ---\n>  git-send-email.perl | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index c5a3f766f7fd..e3ff44b4d0cd 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {\n>  \n>  \t# Check mechanism naming as defined in:\n>  \t# https://tools.ietf.org/html/rfc4422#page-8\n> -\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> +\tif ($smtp_auth && $smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n>  \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n>  \t}\n\nThanks, makes sense. I wonder if moving the check to the point where\n$smtp_auth is actually used (despite the noisier diff) would be cleaner,\nlike this:\n\n--- 8< ---\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex c5a3f76..2a5ceda 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1134,12 +1134,6 @@ sub smtp_auth_maybe {\n \t\tAuthen::SASL->import(qw(Perl));\n \t};\n \n-\t# Check mechanism naming as defined in:\n-\t# https://tools.ietf.org/html/rfc4422#page-8\n-\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n-\t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n-\t}\n-\n \t# TODO: Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n@@ -1154,6 +1148,12 @@ sub smtp_auth_maybe {\n \t\tmy $cred = shift;\n \n \t\tif ($smtp_auth) {\n+\t\t\t# Check mechanism naming as defined in:\n+\t\t\t# https://tools.ietf.org/html/rfc4422#page-8\n+\t\t\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n+\t\t\t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n+\t\t\t}\n+\n \t\t\tmy $sasl = Authen::SASL->new(\n \t\t\t\tmechanism => $smtp_auth,\n \t\t\t\tcallback => {\n--- 8< ---\n"},{"id":"270321","messageId":"20150918224242.GA45466@google.com","threadId":"40383","inReplyTo":"20150918222524.GA22410@flurp.local","subject":"Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Brian Norris","fromEmail":"computersforpeace@gmail.com","sentAt":"2015-09-18T22:42:42Z","receivedAt":"2015-09-18T22:42:42Z","isPatch":true,"sender":{"key":"computersforpeace@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2268082?v=4"},"body":"On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote:\n> On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote:\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {\n> >  \n> >  \t# Check mechanism naming as defined in:\n> >  \t# https://tools.ietf.org/html/rfc4422#page-8\n> > -\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> > +\tif ($smtp_auth && $smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> >  \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n> >  \t}\n> \n> Thanks, makes sense. I wonder if moving the check to the point where\n> $smtp_auth is actually used (despite the noisier diff) would be cleaner,\n> like this:\n> \n> --- 8< ---\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index c5a3f76..2a5ceda 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1134,12 +1134,6 @@ sub smtp_auth_maybe {\n>  \t\tAuthen::SASL->import(qw(Perl));\n>  \t};\n>  \n> -\t# Check mechanism naming as defined in:\n> -\t# https://tools.ietf.org/html/rfc4422#page-8\n> -\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> -\t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n> -\t}\n> -\n>  \t# TODO: Authentication may fail not because credentials were\n>  \t# invalid but due to other reasons, in which we should not\n>  \t# reject credentials.\n> @@ -1154,6 +1148,12 @@ sub smtp_auth_maybe {\n>  \t\tmy $cred = shift;\n>  \n>  \t\tif ($smtp_auth) {\n> +\t\t\t# Check mechanism naming as defined in:\n> +\t\t\t# https://tools.ietf.org/html/rfc4422#page-8\n> +\t\t\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> +\t\t\t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n> +\t\t\t}\n> +\n>  \t\t\tmy $sasl = Authen::SASL->new(\n>  \t\t\t\tmechanism => $smtp_auth,\n>  \t\t\t\tcallback => {\n> --- 8< ---\n\nBy moving the patch into the sub-subroutine (is this a lambda? I'm not\ntoo familiar with my perl), I think you change the order of the checks.\nSo, previously, initial password auth would happen after the\n$smtp_auth format check. With your patch, it comes first.\n\nMy patch:\n\n  $ git send-email --smtp-auth=\"@\" ...\n  ...\n  Send this email? ([y]es|[n]o|[q]uit|[a]ll): y\n  invalid smtp auth: '@' at ./git-send-email.perl line 1140.\n\nYour patch:\n\n  $ git send-email --smtp-auth=\"@\" ...\n  ...\n  Send this email? ([y]es|[n]o|[q]uit|[a]ll): y\n  Password for xxxx:\n  invalid smtp auth: '@' at ./git-send-email.perl line 1155.\n\nSeems like the former is a little better, so you don't have to waste\ntime with your password too many times.\n\nRegards,\nBrian\n"},{"id":"270322","messageId":"CAPig+cR+W0Kt2y+vspS5dF6FHU=XLWZXJR46r7Rv4JP4gNiKTA@mail.gmail.com","threadId":"40383","inReplyTo":"20150918224242.GA45466@google.com","subject":"Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-18T22:47:20Z","receivedAt":"2015-09-18T22:47:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 18, 2015 at 6:42 PM, Brian Norris\n<computersforpeace@gmail.com> wrote:\n> On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote:\n>> Thanks, makes sense. I wonder if moving the check to the point where\n>> $smtp_auth is actually used (despite the noisier diff) would be cleaner,\n>\n> By moving the patch into the sub-subroutine (is this a lambda? I'm not\n> too familiar with my perl), I think you change the order of the checks.\n> So, previously, initial password auth would happen after the\n> $smtp_auth format check. With your patch, it comes first.\n>\n> My patch:\n>\n>   $ git send-email --smtp-auth=\"@\" ...\n>   ...\n>   Send this email? ([y]es|[n]o|[q]uit|[a]ll): y\n>   invalid smtp auth: '@' at ./git-send-email.perl line 1140.\n>\n> Your patch:\n>\n>   $ git send-email --smtp-auth=\"@\" ...\n>   ...\n>   Send this email? ([y]es|[n]o|[q]uit|[a]ll): y\n>   Password for xxxx:\n>   invalid smtp auth: '@' at ./git-send-email.perl line 1155.\n>\n> Seems like the former is a little better, so you don't have to waste\n> time with your password too many times.\n\nMakes sense (again). Thanks for the explanation.\n"},{"id":"270323","messageId":"20150918225009.GA46959@google.com","threadId":"40383","inReplyTo":"CAPig+cR+W0Kt2y+vspS5dF6FHU=XLWZXJR46r7Rv4JP4gNiKTA@mail.gmail.com","subject":"Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Brian Norris","fromEmail":"computersforpeace@gmail.com","sentAt":"2015-09-18T22:50:09Z","receivedAt":"2015-09-18T22:50:09Z","isPatch":true,"sender":{"key":"computersforpeace@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2268082?v=4"},"body":"On Fri, Sep 18, 2015 at 06:47:20PM -0400, Eric Sunshine wrote:\n> Makes sense (again). Thanks for the explanation.\n\nNo problem. Thanks for the review.\n"},{"id":"270413","messageId":"xmqqr3lracd7.fsf@gitster.mtv.corp.google.com","threadId":"40383","inReplyTo":"1442614370-39880-1-git-send-email-computersforpeace@gmail.com","subject":"Re: [PATCH] send-email: fix uninitialized var warning for $smtp_auth","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-21T17:52:36Z","receivedAt":"2015-09-21T17:52:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Norris <computersforpeace@gmail.com> writes:\n\n> On the latest version of git-send-email, I see this error just before\n> running SMTP auth (I didn't provide any --smtp-auth= parameter):\n>\n>   Use of uninitialized value $smtp_auth in pattern match (m//) at /usr/local/google/home/briannorris/git/git/git-send-email.perl line 1139.\n>\n> Signed-off-by: Brian Norris <computersforpeace@gmail.com>\n> Cc: Jan Viktorin <viktorin@rehivetech.com>\n> Cc: Eric Sunshine <sunshine@sunshineco.com>\n> Cc: Junio C Hamano <gitster@pobox.com>\n> ---\n>  git-send-email.perl | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index c5a3f766f7fd..e3ff44b4d0cd 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {\n>  \n>  \t# Check mechanism naming as defined in:\n>  \t# https://tools.ietf.org/html/rfc4422#page-8\n> -\tif ($smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n> +\tif ($smtp_auth && $smtp_auth !~ /^(\\b[A-Z0-9-_]{1,20}\\s*)*$/) {\n>  \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n>  \t}\n\nThanks.\n"}]}