{"thread":{"id":"34882","subject":"[PATCH] send-email: don't call methods on undefined values","startedAt":"2013-09-08T20:54:34Z","lastAt":"2013-09-13T06:09:45Z","messageCount":4,"participants":["brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"227150","messageId":"1378673674-97212-1-git-send-email-sandals@crustytoothpaste.net","threadId":"34882","inReplyTo":null,"subject":"[PATCH] send-email: don't call methods on undefined values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-09-08T20:54:34Z","receivedAt":"2013-09-08T20:54:34Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"If SSL verification is enabled in git send-email, we could attempt to call a\nmethod on an undefined value if the verification failed, since $smtp would end\nup being undef. Look up the error string in a way that will produce a helpful\nerror message and not cause further errors.\n\nSigned-off-by: brian m. carlson \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 2162478..3782c3b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion\n \t\t\t\tif ($smtp->code == 220) {\n \t\t\t\t\t$smtp = Net::SMTP::SSL->start_SSL($smtp,\n \t\t\t\t\t\t\t\t\t ssl_verify_params())\n-\t\t\t\t\t\tor die \"STARTTLS failed! \".$smtp->message;\n+\t\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n \t\t\t\t\t$smtp_encryption = '';\n \t\t\t\t\t# Send EHLO again to receive fresh\n \t\t\t\t\t# supported commands\n-- \n1.8.4.rc3\n"},{"id":"227237","messageId":"xmqqli363pwp.fsf@gitster.dls.corp.google.com","threadId":"34882","inReplyTo":"1378673674-97212-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH] send-email: don't call methods on undefined values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-09T16:45:10Z","receivedAt":"2013-09-09T16:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> If SSL verification is enabled in git send-email, we could attempt to call a\n> method on an undefined value if the verification failed, since $smtp would end\n> up being undef. Look up the error string in a way that will produce a helpful\n> error message and not cause further errors.\n>\n> Signed-off-by: brian m. carlson \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 2162478..3782c3b 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion\n> \t\t\t\tif ($smtp->code == 220) {\n> \t\t\t\t\t$smtp = Net::SMTP::SSL->start_SSL($smtp,\n> \t\t\t\t\t\t\t\t\t ssl_verify_params())\n> -\t\t\t\t\t\tor die \"STARTTLS failed! \".$smtp->message;\n> +\t\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n\nI agree that $smtp->message may be bogus at this point, but could\n\"require IO::Socket::SSL\" have failed on us in ssl_verify_params?\nIn that degraded mode, we do not do SSL peer verification, but\notherwise we would still attempt to talk with the peer, and in such\na case, IO::Socket::SSL would not be available to us at this point,\nno?\n\n> \t\t\t\t\t$smtp_encryption = '';\n> \t\t\t\t\t# Send EHLO again to receive fresh\n> \t\t\t\t\t# supported commands\n"},{"id":"227274","messageId":"20130909224950.GA101065@vauxhall.crustytoothpaste.net","threadId":"34882","inReplyTo":"xmqqli363pwp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] send-email: don't call methods on undefined values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-09-09T22:49:50Z","receivedAt":"2013-09-09T22:49:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Sep 09, 2013 at 09:45:10AM -0700, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion\n> > \t\t\t\tif ($smtp->code == 220) {\n> > \t\t\t\t\t$smtp = Net::SMTP::SSL->start_SSL($smtp,\n> > \t\t\t\t\t\t\t\t\t ssl_verify_params())\n> > -\t\t\t\t\t\tor die \"STARTTLS failed! \".$smtp->message;\n> > +\t\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> \n> I agree that $smtp->message may be bogus at this point, but could\n> \"require IO::Socket::SSL\" have failed on us in ssl_verify_params?\n> In that degraded mode, we do not do SSL peer verification, but\n> otherwise we would still attempt to talk with the peer, and in such\n> a case, IO::Socket::SSL would not be available to us at this point,\n> no?\n\nSince Net::SMTP::SSL uses IO::Socket::SSL (in fact, it is an\nIO::Socket::SSL), we can be guaranteed that it is, in fact, available at\nthis point. I guess strictly we don't need that require in\nIO::Socket::SSL since we'll already be guaranteed that it exists by the\nrequire of Net::SMTP::SSL.\n\nI tried using Net::SMTP::SSL::errstr() instead, but that didn't seem to\nproduce useful output.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"227614","messageId":"xmqqd2odl0ba.fsf@gitster.dls.corp.google.com","threadId":"34882","inReplyTo":"20130909224950.GA101065@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH] send-email: don't call methods on undefined values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-13T06:09:45Z","receivedAt":"2013-09-13T06:09:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On Mon, Sep 09, 2013 at 09:45:10AM -0700, Junio C Hamano wrote:\n>> \"brian m. carlson\" writes:\n>> > --- a/git-send-email.perl\n>> > +++ b/git-send-email.perl\n>> > @@ -1234,7 +1234,7 @@ X-Mailer: git-send-email $gitversion\n>> > \t\t\t\tif ($smtp->code == 220) {\n>> > \t\t\t\t\t$smtp = Net::SMTP::SSL->start_SSL($smtp,\n>> > \t\t\t\t\t\t\t\t\t ssl_verify_params())\n>> > -\t\t\t\t\t\tor die \"STARTTLS failed! \".$smtp->message;\n>> > +\t\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n>> \n>> I agree that $smtp->message may be bogus at this point, but could\n>> \"require IO::Socket::SSL\" have failed on us in ssl_verify_params?\n>> In that degraded mode, we do not do SSL peer verification, but\n>> otherwise we would still attempt to talk with the peer, and in such\n>> a case, IO::Socket::SSL would not be available to us at this point,\n>> no?\n>\n> Since Net::SMTP::SSL uses IO::Socket::SSL (in fact, it is an\n> IO::Socket::SSL), we can be guaranteed that it is, in fact, available at\n> this point. I guess strictly we don't need that require in\n> IO::Socket::SSL since we'll already be guaranteed that it exists by the\n> require of Net::SMTP::SSL.\n\nThat \"require\" came from around $gmane/230533, which was about\nmaking sure the update to pacify newer Net::SMTP::SSL does not break\nfolks with older packages, I think.\n\nI just didn't/don't know the history of Net::SMTP::SSL, and that was\nwhere my comments came from.\n\nAs long as \"Net::SMTP::SSL uses IO::Socket::SSL\" has been true since\nthe ancient past, I agree that that 'require' of the latter does not\nneed to be guarded by an 'eval'; at that point, we are already in\nthe Net::SMTP::SSL codepath. And your patch I replied to should be\nthe right thing to do.\n\nThanks for clarifying.\n"}]}