{"thread":{"id":"44517","subject":"[PATCH] Remove dependency on deprecated Net::SMTP::SSL","startedAt":"2016-11-20T21:25:35Z","lastAt":"2017-06-01T19:42:30Z","messageCount":16,"participants":["Mike Fisher","brian m. carlson","Torsten Bögershausen","Renato Botelho","Dennis Kaarsemaker","Ævar Arnfjörð Bjarmason","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"306207","messageId":"451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu","threadId":"44517","inReplyTo":null,"subject":"[PATCH] Remove dependency on deprecated Net::SMTP::SSL","fromName":"Mike Fisher","fromEmail":"mfisher@csh.rit.edu","sentAt":"2016-11-20T21:18:16Z","receivedAt":"2016-11-20T21:25:35Z","isPatch":true,"sender":{"key":"mfisher@csh.rit.edu","avatar":null},"body":"Refactor send_message() to remove dependency on deprecated\nNet::SMTP::SSL:\n\n<http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>\n\nSigned-off-by: Mike Fisher <mfisher@csh.rit.edu>\n---\n  git-send-email.perl | 54 \n+++++++++++++++++++++++++----------------------------\n  1 file changed, 25 insertions(+), 29 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex da81be4..fc166c5 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1330,15 +1330,17 @@ Message-Id: $message_id\n  \t\tprint $sm \"$header\\n$message\";\n  \t\tclose $sm or die $!;\n  \t} else {\n-\n  \t\tif (!defined $smtp_server) {\n  \t\t\tdie \"The required SMTP server is not properly defined.\"\n  \t\t}\n\n+\t\trequire Net::SMTP;\n+\t\t$smtp_domain ||= maildomain();\n+\t\tmy $smtp_ssl = 0;\n+\n  \t\tif ($smtp_encryption eq 'ssl') {\n  \t\t\t$smtp_server_port ||= 465; # ssmtp\n-\t\t\trequire Net::SMTP::SSL;\n-\t\t\t$smtp_domain ||= maildomain();\n+\t\t\t$smtp_ssl = 1;\n  \t\t\trequire IO::Socket::SSL;\n\n  \t\t\t# Suppress \"variable accessed once\" warning.\n@@ -1347,37 +1349,31 @@ Message-Id: $message_id\n  \t\t\t\t$IO::Socket::SSL::DEBUG = 1;\n  \t\t\t}\n\n-\t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n  \t\t\tIO::Socket::SSL::set_client_defaults(\n  \t\t\t\tssl_verify_params());\n-\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n-\t\t\t\t\t\t      Hello => $smtp_domain,\n-\t\t\t\t\t\t      Port => $smtp_server_port,\n-\t\t\t\t\t\t      Debug => $debug_net_smtp);\n  \t\t}\n  \t\telse {\n-\t\t\trequire Net::SMTP;\n-\t\t\t$smtp_domain ||= maildomain();\n  \t\t\t$smtp_server_port ||= 25;\n-\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n-\t\t\t\t\t\t Hello => $smtp_domain,\n-\t\t\t\t\t\t Debug => $debug_net_smtp,\n-\t\t\t\t\t\t Port => $smtp_server_port);\n-\t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n-\t\t\t\trequire Net::SMTP::SSL;\n-\t\t\t\t$smtp->command('STARTTLS');\n-\t\t\t\t$smtp->response();\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! \".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-\t\t\t\t\t$smtp->hello($smtp_domain);\n-\t\t\t\t} else {\n-\t\t\t\t\tdie \"Server does not support STARTTLS! \".$smtp->message;\n-\t\t\t\t}\n+\t\t}\n+\n+\t\t$smtp ||= Net::SMTP->new($smtp_server,\n+\t\t\t\t\t Hello => $smtp_domain,\n+\t\t\t\t\t Port => $smtp_server_port,\n+\t\t\t\t\t Debug => $debug_net_smtp,\n+\t\t\t\t\t SSL => $smtp_ssl);\n+\n+\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n+\t\t\t$smtp->command('STARTTLS');\n+\t\t\t$smtp->response();\n+\t\t\tif ($smtp->code == 220) {\n+\t\t\t\t$smtp->starttls(ssl_verify_params())\n+\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n+\t\t\t\t$smtp_encryption = '';\n+\t\t\t\t# Send EHLO again to receive fresh\n+\t\t\t\t# supported commands\n+\t\t\t\t$smtp->hello($smtp_domain);\n+\t\t\t} else {\n+\t\t\t\tdie \"Server does not support STARTTLS! \".$smtp->message;\n  \t\t\t}\n  \t\t}\n\n-- \n2.9.3 (Apple Git-75)\n\n"},{"id":"306208","messageId":"20161120215344.jaqt4owlhovig3hz@genre.crustytoothpaste.net","threadId":"44517","inReplyTo":"451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu","subject":"Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2016-11-20T21:53:44Z","receivedAt":"2016-11-20T21:53:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Nov 20, 2016 at 04:18:16PM -0500, Mike Fisher wrote:\n> Refactor send_message() to remove dependency on deprecated\n> Net::SMTP::SSL:\n> \n> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>\n\nAs much as I hate to say this, I think this is going to cause\ncompatibility problems.  Net::SMTP is part of core Perl (as of v5.7.3),\nbut the version you want to rely on (which you did not provide an\nexplicit dependency on) is from October 2014.\n\nThat basically means that no Perl on a Red Hat or CentOS system is going\nto provide that support, since RHEL 7 was released in June 2014.\nProviding an updated Git on those platforms would require replacing the\nsystem Perl or parts of it, which would be undesirable.  This would\naffect Debian 7 as well.\n\nWe currently support Perl 5.8 [0], so if you want to remove support for\nNet::SMTP::SSL, I'd recommend a solution that works with that version.\n\n[0] I personally believe we should drop support for Perl older than\n5.10.1 (if not newer), but that's my opinion and it isn't shared by\nother list regulars.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"306211","messageId":"d5f21248-e70a-4adc-52c5-73fd00f62961@web.de","threadId":"44517","inReplyTo":"451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu","subject":"Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-11-21T05:37:28Z","receivedAt":"2016-11-21T05:37:49Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\n>On 20/11/16 22:18, Mike Fisher  wrote:\n\nThanks for contributing to Git.\nOne comment on the head line:\n >Refactor send_message() to remove dependency on deprecated\nNet::SMTP::SSL\nThe word \"refactor\" may be used in other way: Re-structure the code,\nand use the same API.\n\n\n\"Remove dependency on deprecated Net::SMTP::SSL\"\n\n> Refactor send_message() to remove dependency on deprecated\n> Net::SMTP::SSL:\nIs there a security risk with require Net::SMTP::SSL ?\nIf yes, the commit message should state this.\nIf no:\nEven if it is deprecated, is it still in use somewhere ?\nDoes it hurt someone, is there any OS release where the old code doesn't work \nanymore ?\nOr is it \"only\" nice to have ?\nSince when does Net::SMTP include Net::SMTP::SSL ?\nOn which system has the change been tested ?\n\nI think the commit message could and should give more information like this.\n\nMy comments may be over-critical.\nLets see if other people from the list know more than me.\n\n>\n> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>\n>\n> Signed-off-by: Mike Fisher <mfisher@csh.rit.edu>\n> ---\n>  git-send-email.perl | 54 +++++++++++++++++++++++++----------------------------\n>  1 file changed, 25 insertions(+), 29 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index da81be4..fc166c5 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1330,15 +1330,17 @@ Message-Id: $message_id\n>          print $sm \"$header\\n$message\";\n>          close $sm or die $!;\n>      } else {\n> -\nI can see one refactoring, that is the removal of an empty line.\n\n>\n>          if (!defined $smtp_server) {\n>              die \"The required SMTP server is not properly defined.\"\n>          }\n>\n> +        require Net::SMTP;\n> +        $smtp_domain ||= maildomain();\n> +        my $smtp_ssl = 0;\n> +\n>          if ($smtp_encryption eq 'ssl') {\n>              $smtp_server_port ||= 465; # ssmtp\n> -            require Net::SMTP::SSL;\n> -            $smtp_domain ||= maildomain();\n> +            $smtp_ssl = 1;\n>              require IO::Socket::SSL;\n>\n>              # Suppress \"variable accessed once\" warning.\n> @@ -1347,37 +1349,31 @@ Message-Id: $message_id\n>                  $IO::Socket::SSL::DEBUG = 1;\n>              }\n>\n> -            # Net::SMTP::SSL->new() does not forward any SSL options\n>              IO::Socket::SSL::set_client_defaults(\n>                  ssl_verify_params());\n> -            $smtp ||= Net::SMTP::SSL->new($smtp_server,\n> -                              Hello => $smtp_domain,\n> -                              Port => $smtp_server_port,\n> -                              Debug => $debug_net_smtp);\n>          }\n>          else {\n> -            require Net::SMTP;\n> -            $smtp_domain ||= maildomain();\n>              $smtp_server_port ||= 25;\n> -            $smtp ||= Net::SMTP->new($smtp_server,\n> -                         Hello => $smtp_domain,\n> -                         Debug => $debug_net_smtp,\n> -                         Port => $smtp_server_port);\n> -            if ($smtp_encryption eq 'tls' && $smtp) {\n> -                require Net::SMTP::SSL;\n> -                $smtp->command('STARTTLS');\n> -                $smtp->response();\n> -                if ($smtp->code == 220) {\n> -                    $smtp = Net::SMTP::SSL->start_SSL($smtp,\n> -                                      ssl_verify_params())\n> -                        or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> -                    $smtp_encryption = '';\n> -                    # Send EHLO again to receive fresh\n> -                    # supported commands\n> -                    $smtp->hello($smtp_domain);\n> -                } else {\n> -                    die \"Server does not support STARTTLS! \".$smtp->message;\n> -                }\n> +        }\n> +\n> +        $smtp ||= Net::SMTP->new($smtp_server,\n> +                     Hello => $smtp_domain,\n> +                     Port => $smtp_server_port,\n> +                     Debug => $debug_net_smtp,\n> +                     SSL => $smtp_ssl);\n> +\n> +        if ($smtp_encryption eq 'tls' && $smtp) {\n> +            $smtp->command('STARTTLS');\n> +            $smtp->response();\n> +            if ($smtp->code == 220) {\n> +                $smtp->starttls(ssl_verify_params())\n> +                    or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> +                $smtp_encryption = '';\n> +                # Send EHLO again to receive fresh\n> +                # supported commands\n> +                $smtp->hello($smtp_domain);\n> +            } else {\n> +                die \"Server does not support STARTTLS! \".$smtp->message;\n>              }\n>          }\n>\n\n"},{"id":"309355","messageId":"9D9E7BB6-9AAD-4356-A500-A86DA7C958EF@FreeBSD.org","threadId":"44517","inReplyTo":"20161120215344.jaqt4owlhovig3hz@genre.crustytoothpaste.net","subject":"Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL","fromName":"Renato Botelho","fromEmail":"garga@freebsd.org","sentAt":"2017-01-13T14:59:15Z","receivedAt":"2017-01-13T14:59:19Z","isPatch":true,"sender":{"key":"garga@freebsd.org","avatar":"https://gravatar.com/avatar/695c68fb2f0629998c430204a7212aec683a3f87ec5eeaae236fd2376e790bfc?d=mp&s=160"},"body":"> On 20 Nov 2016, at 19:53, brian m. carlson <sandals@crustytoothpaste.net> wrote:\n> \n> On Sun, Nov 20, 2016 at 04:18:16PM -0500, Mike Fisher wrote:\n>> Refactor send_message() to remove dependency on deprecated\n>> Net::SMTP::SSL:\n>> \n>> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>\n> \n> As much as I hate to say this, I think this is going to cause\n> compatibility problems.  Net::SMTP is part of core Perl (as of v5.7.3),\n> but the version you want to rely on (which you did not provide an\n> explicit dependency on) is from October 2014.\n> \n> That basically means that no Perl on a Red Hat or CentOS system is going\n> to provide that support, since RHEL 7 was released in June 2014.\n> Providing an updated Git on those platforms would require replacing the\n> system Perl or parts of it, which would be undesirable.  This would\n> affect Debian 7 as well.\n> \n> We currently support Perl 5.8 [0], so if you want to remove support for\n> Net::SMTP::SSL, I'd recommend a solution that works with that version.\n> \n> [0] I personally believe we should drop support for Perl older than\n> 5.10.1 (if not newer), but that's my opinion and it isn't shared by\n> other list regulars.\n> -- \n> brian m. carlson / brian with sandals: Houston, Texas, US\n> +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\n> OpenPGP: https://keybase.io/bk2204\n\nNet::SMTP::SSL is marked as DEPRECATED on FreeBSD ports tree and will be removed in 2017-03-31. When it happens users will not be able to run git-send-email anymore. I’m considering to add Mike’s patch to FreeBSD ports tree as an alternative but it would be good to have a official solution for this problem.\n\nFreeBSD bug report can be found at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214335\n\n--\nRenato Botelho\n\n"},{"id":"314566","messageId":"20170318222311.9993-1-dennis@kaarsemaker.net","threadId":"44517","inReplyTo":"451E4A46-BA43-41A5-9E68-DE0D89BE676A@csh.rit.edu","subject":"[PATCH] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-18T22:23:11Z","receivedAt":"2017-03-18T22:24:55Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\nsince version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\nisn't that old yet, keep the old code in place and use it when\nnecessary.\n\nSigned-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n---\n Note: I've only been able to test the starttls bits. None of the smtp servers\n I use actually use ssl, only starttls.\n\n git-send-email.perl | 52 ++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 34 insertions(+), 18 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex eea0a517f7..e247ea39dd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1353,10 +1353,12 @@ EOF\n \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n \t\t}\n \n+\t\trequire Net::SMTP;\n+\t\tmy $use_net_smtp_ssl = $Net::SMTP::VERSION lt \"1.28\";\n+\t\t$smtp_domain ||= maildomain();\n+\n \t\tif ($smtp_encryption eq 'ssl') {\n \t\t\t$smtp_server_port ||= 465; # ssmtp\n-\t\t\trequire Net::SMTP::SSL;\n-\t\t\t$smtp_domain ||= maildomain();\n \t\t\trequire IO::Socket::SSL;\n \n \t\t\t# Suppress \"variable accessed once\" warning.\n@@ -1368,34 +1370,48 @@ EOF\n \t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n \t\t\tIO::Socket::SSL::set_client_defaults(\n \t\t\t\tssl_verify_params());\n-\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n-\t\t\t\t\t\t      Hello => $smtp_domain,\n-\t\t\t\t\t\t      Port => $smtp_server_port,\n-\t\t\t\t\t\t      Debug => $debug_net_smtp);\n+\n+\t\t\tif ($use_net_smtp_ssl) {\n+\t\t\t\trequire Net::SMTP::SSL;\n+\t\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n+\t\t\t\t\t\t\t      Hello => $smtp_domain,\n+\t\t\t\t\t\t\t      Port => $smtp_server_port,\n+\t\t\t\t\t\t\t      Debug => $debug_net_smtp);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n+\t\t\t\t\t\t\t Hello => $smtp_domain,\n+\t\t\t\t\t\t\t Port => $smtp_server_port,\n+\t\t\t\t\t\t\t Debug => $debug_net_smtp,\n+\t\t\t\t\t\t\t SSL => 1);\n+\t\t\t}\n \t\t}\n \t\telse {\n-\t\t\trequire Net::SMTP;\n-\t\t\t$smtp_domain ||= maildomain();\n \t\t\t$smtp_server_port ||= 25;\n \t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n \t\t\t\t\t\t Hello => $smtp_domain,\n \t\t\t\t\t\t Debug => $debug_net_smtp,\n \t\t\t\t\t\t Port => $smtp_server_port);\n \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n-\t\t\t\trequire Net::SMTP::SSL;\n-\t\t\t\t$smtp->command('STARTTLS');\n-\t\t\t\t$smtp->response();\n-\t\t\t\tif ($smtp->code == 220) {\n+\t\t\t\tif ($use_net_smtp_ssl) {\n+\t\t\t\t\t$smtp->command('STARTTLS');\n+\t\t\t\t\t$smtp->response();\n+\t\t\t\t\tif ($smtp->code != 220) {\n+\t\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n+\t\t\t\t\t}\n+\t\t\t\t\trequire Net::SMTP::SSL;\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! \".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-\t\t\t\t\t$smtp->hello($smtp_domain);\n-\t\t\t\t} else {\n-\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n \t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\t$smtp->starttls(ssl_verify_params())\n+\t\t\t\t\t\tor die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n+\t\t\t\t}\n+\t\t\t\t$smtp_encryption = '';\n+\t\t\t\t# Send EHLO again to receive fresh\n+\t\t\t\t# supported commands\n+\t\t\t\t$smtp->hello($smtp_domain);\n \t\t\t}\n \t\t}\n \n-- \n2.12.0-437-g0cc2799\n\n"},{"id":"314570","messageId":"CACBZZX5j1dYk8aeRED7T7iJ=b32aFUpfUWPpMpmtofBL3QnVXQ@mail.gmail.com","threadId":"44517","inReplyTo":"20170318222311.9993-1-dennis@kaarsemaker.net","subject":"Re: [PATCH] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-03-18T22:47:35Z","receivedAt":"2017-03-18T22:48:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Mar 18, 2017 at 11:23 PM, Dennis Kaarsemaker\n<dennis@kaarsemaker.net> wrote:\n> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n> isn't that old yet, keep the old code in place and use it when\n> necessary.\n>\n> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n> ---\n>  Note: I've only been able to test the starttls bits. None of the smtp servers\n>  I use actually use ssl, only starttls.\n>\n>  git-send-email.perl | 52 ++++++++++++++++++++++++++++++++++------------------\n>  1 file changed, 34 insertions(+), 18 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index eea0a517f7..e247ea39dd 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1353,10 +1353,12 @@ EOF\n>                         die __(\"The required SMTP server is not properly defined.\")\n>                 }\n>\n> +               require Net::SMTP;\n> +               my $use_net_smtp_ssl = $Net::SMTP::VERSION lt \"1.28\";\n> +               $smtp_domain ||= maildomain();\n> +\n\nWhile Net::SMTP is unlikely to change its versioning scheme, let's use\ncomparisons via the version module here in case they do change it to\nsomething silly, and this ends up introducing a bug.\n\nE.g. 04.00 would be considered a higher version by CPAN than 1.28, but\nnot by this code:\n\n    $ perl -wE 'my ($x, $y) = @ARGV; my ($vx, $vy) = map {\nversion->parse($_) } ($x, $y); say $vx < $vy ? \"vlower\" : \"vhigher\";\nsay $x lt $y ? \"slower\" : \"shigher\"' 04.00 1.28\n    vhigher\n    slower\n\nIf we grep ::VERSION we can find other cases where we've gotten this\nwrong, unlikely to bite us in practice, but version.pm is in core (so\ncore that you don't even need to use/require it), so let's do this\nbetter for new code.\n\n\n>[...]\n> +                                       if ($smtp->code != 220) {\n> +                                               die sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n\nHere a new message you're adding gets __(), makes sense.\n\n> +                                       }\n> +                                       require Net::SMTP::SSL;\n>                                         $smtp = Net::SMTP::SSL->start_SSL($smtp,\n>                                                                           ssl_verify_params())\n>                                                 or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> -                                       $smtp_encryption = '';\n> -                                       # Send EHLO again to receive fresh\n> -                                       # supported commands\n> -                                       $smtp->hello($smtp_domain);\n> -                               } else {\n> -                                       die sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n>                                 }\n> +                               else {\n> +                                       $smtp->starttls(ssl_verify_params())\n> +                                               or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> +                               }\n\nI see you just copied that from above but I wonder if it makes sense\nto just mark both occurrences with __() too while we're at it.\n"},{"id":"314578","messageId":"1489878863.24742.3.camel@kaarsemaker.net","threadId":"44517","inReplyTo":"CACBZZX5j1dYk8aeRED7T7iJ=b32aFUpfUWPpMpmtofBL3QnVXQ@mail.gmail.com","subject":"Re: [PATCH] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-18T23:14:23Z","receivedAt":"2017-03-18T23:15:28Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Sat, 2017-03-18 at 23:47 +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> On Sat, Mar 18, 2017 at 11:23 PM, Dennis Kaarsemaker\n> <dennis@kaarsemaker.net> wrote:\n>\n> > +               require Net::SMTP;\n> > +               my $use_net_smtp_ssl = $Net::SMTP::VERSION lt \"1.28\";\n> > +               $smtp_domain ||= maildomain();\n> > +\n> \n> While Net::SMTP is unlikely to change its versioning scheme, let's use\n> comparisons via the version module here in case they do change it to\n> something silly, and this ends up introducing a bug.\n\nok.\n\n> > [...]\n> > +                                       if ($smtp->code != 220) {\n> > +                                               die sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> \n> Here a new message you're adding gets __(), makes sense.\n\nDidn't add it, it just moved from a bit further below :)\n\n> > +                               else {\n> > +                                       $smtp->starttls(ssl_verify_params())\n> > +                                               or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n> > +                               }\n> \n> I see you just copied that from above but I wonder if it makes sense\n> to just mark both occurrences with __() too while we're at it.\n\nok.\n\nD.\n"},{"id":"315337","messageId":"20170324213732.29932-1-dennis@kaarsemaker.net","threadId":"44517","inReplyTo":"CACBZZX5j1dYk8aeRED7T7iJ=b32aFUpfUWPpMpmtofBL3QnVXQ@mail.gmail.com","subject":"[PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-24T21:37:32Z","receivedAt":"2017-03-24T21:38:14Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\nsince version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\nisn't that old yet, keep the old code in place and use it when\nnecessary.\n\nWhile we're in the area, mark some messages for translation that were\nnot yet marked as such.\n\nSigned-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n---\n git-send-email.perl | 54 ++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 35 insertions(+), 19 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex eea0a517f7..0d90439d9a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1353,10 +1353,12 @@ EOF\n \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n \t\t}\n \n+\t\trequire Net::SMTP;\n+\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n+\t\t$smtp_domain ||= maildomain();\n+\n \t\tif ($smtp_encryption eq 'ssl') {\n \t\t\t$smtp_server_port ||= 465; # ssmtp\n-\t\t\trequire Net::SMTP::SSL;\n-\t\t\t$smtp_domain ||= maildomain();\n \t\t\trequire IO::Socket::SSL;\n \n \t\t\t# Suppress \"variable accessed once\" warning.\n@@ -1368,34 +1370,48 @@ EOF\n \t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n \t\t\tIO::Socket::SSL::set_client_defaults(\n \t\t\t\tssl_verify_params());\n-\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n-\t\t\t\t\t\t      Hello => $smtp_domain,\n-\t\t\t\t\t\t      Port => $smtp_server_port,\n-\t\t\t\t\t\t      Debug => $debug_net_smtp);\n+\n+\t\t\tif ($use_net_smtp_ssl) {\n+\t\t\t\trequire Net::SMTP::SSL;\n+\t\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n+\t\t\t\t\t\t\t      Hello => $smtp_domain,\n+\t\t\t\t\t\t\t      Port => $smtp_server_port,\n+\t\t\t\t\t\t\t      Debug => $debug_net_smtp);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n+\t\t\t\t\t\t\t Hello => $smtp_domain,\n+\t\t\t\t\t\t\t Port => $smtp_server_port,\n+\t\t\t\t\t\t\t Debug => $debug_net_smtp,\n+\t\t\t\t\t\t\t SSL => 1);\n+\t\t\t}\n \t\t}\n \t\telse {\n-\t\t\trequire Net::SMTP;\n-\t\t\t$smtp_domain ||= maildomain();\n \t\t\t$smtp_server_port ||= 25;\n \t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n \t\t\t\t\t\t Hello => $smtp_domain,\n \t\t\t\t\t\t Debug => $debug_net_smtp,\n \t\t\t\t\t\t Port => $smtp_server_port);\n \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n-\t\t\t\trequire Net::SMTP::SSL;\n-\t\t\t\t$smtp->command('STARTTLS');\n-\t\t\t\t$smtp->response();\n-\t\t\t\tif ($smtp->code == 220) {\n+\t\t\t\tif ($use_net_smtp_ssl) {\n+\t\t\t\t\t$smtp->command('STARTTLS');\n+\t\t\t\t\t$smtp->response();\n+\t\t\t\t\tif ($smtp->code != 220) {\n+\t\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n+\t\t\t\t\t}\n+\t\t\t\t\trequire Net::SMTP::SSL;\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! \".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-\t\t\t\t\t$smtp->hello($smtp_domain);\n-\t\t\t\t} else {\n-\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n+\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\t$smtp->starttls(ssl_verify_params())\n+\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n \t\t\t\t}\n+\t\t\t\t$smtp_encryption = '';\n+\t\t\t\t# Send EHLO again to receive fresh\n+\t\t\t\t# supported commands\n+\t\t\t\t$smtp->hello($smtp_domain);\n \t\t\t}\n \t\t}\n \n-- \n2.12.0-488-gd3584ba\n\n"},{"id":"318728","messageId":"1493881302.20467.3.camel@kaarsemaker.net","threadId":"44517","inReplyTo":"20170324213732.29932-1-dennis@kaarsemaker.net","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-05-04T07:01:42Z","receivedAt":"2017-05-04T07:01:50Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Ping. It's a little over a month since I sent this, but I haven't seen\nany comments. Is this commit good to go?\n\nOn Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:\n> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n> isn't that old yet, keep the old code in place and use it when\n> necessary.\n> \n> While we're in the area, mark some messages for translation that were\n> not yet marked as such.\n> \n> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n> ---\n>  git-send-email.perl | 54 ++++++++++++++++++++++++++++++++++-------------------\n>  1 file changed, 35 insertions(+), 19 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index eea0a517f7..0d90439d9a 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1353,10 +1353,12 @@ EOF\n>  \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n>  \t\t}\n>  \n> +\t\trequire Net::SMTP;\n> +\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n> +\t\t$smtp_domain ||= maildomain();\n> +\n>  \t\tif ($smtp_encryption eq 'ssl') {\n>  \t\t\t$smtp_server_port ||= 465; # ssmtp\n> -\t\t\trequire Net::SMTP::SSL;\n> -\t\t\t$smtp_domain ||= maildomain();\n>  \t\t\trequire IO::Socket::SSL;\n>  \n>  \t\t\t# Suppress \"variable accessed once\" warning.\n> @@ -1368,34 +1370,48 @@ EOF\n>  \t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n>  \t\t\tIO::Socket::SSL::set_client_defaults(\n>  \t\t\t\tssl_verify_params());\n> -\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> -\t\t\t\t\t\t      Hello => $smtp_domain,\n> -\t\t\t\t\t\t      Port => $smtp_server_port,\n> -\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> +\n> +\t\t\tif ($use_net_smtp_ssl) {\n> +\t\t\t\trequire Net::SMTP::SSL;\n> +\t\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> +\t\t\t\t\t\t\t      Hello => $smtp_domain,\n> +\t\t\t\t\t\t\t      Port => $smtp_server_port,\n> +\t\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> +\t\t\t}\n> +\t\t\telse {\n> +\t\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n> +\t\t\t\t\t\t\t Hello => $smtp_domain,\n> +\t\t\t\t\t\t\t Port => $smtp_server_port,\n> +\t\t\t\t\t\t\t Debug => $debug_net_smtp,\n> +\t\t\t\t\t\t\t SSL => 1);\n> +\t\t\t}\n>  \t\t}\n>  \t\telse {\n> -\t\t\trequire Net::SMTP;\n> -\t\t\t$smtp_domain ||= maildomain();\n>  \t\t\t$smtp_server_port ||= 25;\n>  \t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n>  \t\t\t\t\t\t Hello => $smtp_domain,\n>  \t\t\t\t\t\t Debug => $debug_net_smtp,\n>  \t\t\t\t\t\t Port => $smtp_server_port);\n>  \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n> -\t\t\t\trequire Net::SMTP::SSL;\n> -\t\t\t\t$smtp->command('STARTTLS');\n> -\t\t\t\t$smtp->response();\n> -\t\t\t\tif ($smtp->code == 220) {\n> +\t\t\t\tif ($use_net_smtp_ssl) {\n> +\t\t\t\t\t$smtp->command('STARTTLS');\n> +\t\t\t\t\t$smtp->response();\n> +\t\t\t\t\tif ($smtp->code != 220) {\n> +\t\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> +\t\t\t\t\t}\n> +\t\t\t\t\trequire Net::SMTP::SSL;\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! \".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> -\t\t\t\t\t$smtp->hello($smtp_domain);\n> -\t\t\t\t} else {\n> -\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n> +\t\t\t\t}\n> +\t\t\t\telse {\n> +\t\t\t\t\t$smtp->starttls(ssl_verify_params())\n> +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n>  \t\t\t\t}\n> +\t\t\t\t$smtp_encryption = '';\n> +\t\t\t\t# Send EHLO again to receive fresh\n> +\t\t\t\t# supported commands\n> +\t\t\t\t$smtp->hello($smtp_domain);\n>  \t\t\t}\n>  \t\t}\n>  \n"},{"id":"320284","messageId":"1495227246.19473.3.camel@kaarsemaker.net","threadId":"44517","inReplyTo":"1493881302.20467.3.camel@kaarsemaker.net","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-05-19T20:54:06Z","receivedAt":"2017-05-19T20:54:53Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Second ping. This problem is not going away, so if this solution is not\nacceptable, I'd like to know what needs to be improved.\n\nOn Thu, 2017-05-04 at 09:01 +0200, Dennis Kaarsemaker wrote:\n> Ping. It's a little over a month since I sent this, but I haven't seen\n> any comments. Is this commit good to go?\n> \n> On Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:\n> > Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n> > since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n> > isn't that old yet, keep the old code in place and use it when\n> > necessary.\n> > \n> > While we're in the area, mark some messages for translation that were\n> > not yet marked as such.\n> > \n> > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n> > ---\n> >  git-send-email.perl | 54 ++++++++++++++++++++++++++++++++++-------------------\n> >  1 file changed, 35 insertions(+), 19 deletions(-)\n> > \n> > diff --git a/git-send-email.perl b/git-send-email.perl\n> > index eea0a517f7..0d90439d9a 100755\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -1353,10 +1353,12 @@ EOF\n> >  \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n> >  \t\t}\n> >  \n> > +\t\trequire Net::SMTP;\n> > +\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n> > +\t\t$smtp_domain ||= maildomain();\n> > +\n> >  \t\tif ($smtp_encryption eq 'ssl') {\n> >  \t\t\t$smtp_server_port ||= 465; # ssmtp\n> > -\t\t\trequire Net::SMTP::SSL;\n> > -\t\t\t$smtp_domain ||= maildomain();\n> >  \t\t\trequire IO::Socket::SSL;\n> >  \n> >  \t\t\t# Suppress \"variable accessed once\" warning.\n> > @@ -1368,34 +1370,48 @@ EOF\n> >  \t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n> >  \t\t\tIO::Socket::SSL::set_client_defaults(\n> >  \t\t\t\tssl_verify_params());\n> > -\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> > -\t\t\t\t\t\t      Hello => $smtp_domain,\n> > -\t\t\t\t\t\t      Port => $smtp_server_port,\n> > -\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> > +\n> > +\t\t\tif ($use_net_smtp_ssl) {\n> > +\t\t\t\trequire Net::SMTP::SSL;\n> > +\t\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> > +\t\t\t\t\t\t\t      Hello => $smtp_domain,\n> > +\t\t\t\t\t\t\t      Port => $smtp_server_port,\n> > +\t\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> > +\t\t\t}\n> > +\t\t\telse {\n> > +\t\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n> > +\t\t\t\t\t\t\t Hello => $smtp_domain,\n> > +\t\t\t\t\t\t\t Port => $smtp_server_port,\n> > +\t\t\t\t\t\t\t Debug => $debug_net_smtp,\n> > +\t\t\t\t\t\t\t SSL => 1);\n> > +\t\t\t}\n> >  \t\t}\n> >  \t\telse {\n> > -\t\t\trequire Net::SMTP;\n> > -\t\t\t$smtp_domain ||= maildomain();\n> >  \t\t\t$smtp_server_port ||= 25;\n> >  \t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n> >  \t\t\t\t\t\t Hello => $smtp_domain,\n> >  \t\t\t\t\t\t Debug => $debug_net_smtp,\n> >  \t\t\t\t\t\t Port => $smtp_server_port);\n> >  \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n> > -\t\t\t\trequire Net::SMTP::SSL;\n> > -\t\t\t\t$smtp->command('STARTTLS');\n> > -\t\t\t\t$smtp->response();\n> > -\t\t\t\tif ($smtp->code == 220) {\n> > +\t\t\t\tif ($use_net_smtp_ssl) {\n> > +\t\t\t\t\t$smtp->command('STARTTLS');\n> > +\t\t\t\t\t$smtp->response();\n> > +\t\t\t\t\tif ($smtp->code != 220) {\n> > +\t\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> > +\t\t\t\t\t}\n> > +\t\t\t\t\trequire Net::SMTP::SSL;\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! \".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> > -\t\t\t\t\t$smtp->hello($smtp_domain);\n> > -\t\t\t\t} else {\n> > -\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> > +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n> > +\t\t\t\t}\n> > +\t\t\t\telse {\n> > +\t\t\t\t\t$smtp->starttls(ssl_verify_params())\n> > +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n> >  \t\t\t\t}\n> > +\t\t\t\t$smtp_encryption = '';\n> > +\t\t\t\t# Send EHLO again to receive fresh\n> > +\t\t\t\t# supported commands\n> > +\t\t\t\t$smtp->hello($smtp_domain);\n> >  \t\t\t}\n> >  \t\t}\n> >  \n"},{"id":"320303","messageId":"CACBZZX7OE2rRD4W4weGhAoaurFRvA85Js0dN=80zcuxR0xM3SA@mail.gmail.com","threadId":"44517","inReplyTo":"1495227246.19473.3.camel@kaarsemaker.net","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-20T07:56:01Z","receivedAt":"2017-05-20T07:56:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, May 19, 2017 at 10:54 PM, Dennis Kaarsemaker\n<dennis@kaarsemaker.net> wrote:\n> Second ping. This problem is not going away, so if this solution is not\n> acceptable, I'd like to know what needs to be improved.\n\nFWIW:\n\nReviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\n> On Thu, 2017-05-04 at 09:01 +0200, Dennis Kaarsemaker wrote:\n>> Ping. It's a little over a month since I sent this, but I haven't seen\n>> any comments. Is this commit good to go?\n>>\n>> On Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:\n>> > Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n>> > since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n>> > isn't that old yet, keep the old code in place and use it when\n>> > necessary.\n>> >\n>> > While we're in the area, mark some messages for translation that were\n>> > not yet marked as such.\n>> >\n>> > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n>> > ---\n>> >  git-send-email.perl | 54 ++++++++++++++++++++++++++++++++++-------------------\n>> >  1 file changed, 35 insertions(+), 19 deletions(-)\n>> >\n>> > diff --git a/git-send-email.perl b/git-send-email.perl\n>> > index eea0a517f7..0d90439d9a 100755\n>> > --- a/git-send-email.perl\n>> > +++ b/git-send-email.perl\n>> > @@ -1353,10 +1353,12 @@ EOF\n>> >                     die __(\"The required SMTP server is not properly defined.\")\n>> >             }\n>> >\n>> > +           require Net::SMTP;\n>> > +           my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n>> > +           $smtp_domain ||= maildomain();\n>> > +\n>> >             if ($smtp_encryption eq 'ssl') {\n>> >                     $smtp_server_port ||= 465; # ssmtp\n>> > -                   require Net::SMTP::SSL;\n>> > -                   $smtp_domain ||= maildomain();\n>> >                     require IO::Socket::SSL;\n>> >\n>> >                     # Suppress \"variable accessed once\" warning.\n>> > @@ -1368,34 +1370,48 @@ EOF\n>> >                     # Net::SMTP::SSL->new() does not forward any SSL options\n>> >                     IO::Socket::SSL::set_client_defaults(\n>> >                             ssl_verify_params());\n>> > -                   $smtp ||= Net::SMTP::SSL->new($smtp_server,\n>> > -                                                 Hello => $smtp_domain,\n>> > -                                                 Port => $smtp_server_port,\n>> > -                                                 Debug => $debug_net_smtp);\n>> > +\n>> > +                   if ($use_net_smtp_ssl) {\n>> > +                           require Net::SMTP::SSL;\n>> > +                           $smtp ||= Net::SMTP::SSL->new($smtp_server,\n>> > +                                                         Hello => $smtp_domain,\n>> > +                                                         Port => $smtp_server_port,\n>> > +                                                         Debug => $debug_net_smtp);\n>> > +                   }\n>> > +                   else {\n>> > +                           $smtp ||= Net::SMTP->new($smtp_server,\n>> > +                                                    Hello => $smtp_domain,\n>> > +                                                    Port => $smtp_server_port,\n>> > +                                                    Debug => $debug_net_smtp,\n>> > +                                                    SSL => 1);\n>> > +                   }\n>> >             }\n>> >             else {\n>> > -                   require Net::SMTP;\n>> > -                   $smtp_domain ||= maildomain();\n>> >                     $smtp_server_port ||= 25;\n>> >                     $smtp ||= Net::SMTP->new($smtp_server,\n>> >                                              Hello => $smtp_domain,\n>> >                                              Debug => $debug_net_smtp,\n>> >                                              Port => $smtp_server_port);\n>> >                     if ($smtp_encryption eq 'tls' && $smtp) {\n>> > -                           require Net::SMTP::SSL;\n>> > -                           $smtp->command('STARTTLS');\n>> > -                           $smtp->response();\n>> > -                           if ($smtp->code == 220) {\n>> > +                           if ($use_net_smtp_ssl) {\n>> > +                                   $smtp->command('STARTTLS');\n>> > +                                   $smtp->response();\n>> > +                                   if ($smtp->code != 220) {\n>> > +                                           die sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n>> > +                                   }\n>> > +                                   require Net::SMTP::SSL;\n>> >                                     $smtp = Net::SMTP::SSL->start_SSL($smtp,\n>> >                                                                       ssl_verify_params())\n>> > -                                           or die \"STARTTLS failed! \".IO::Socket::SSL::errstr();\n>> > -                                   $smtp_encryption = '';\n>> > -                                   # Send EHLO again to receive fresh\n>> > -                                   # supported commands\n>> > -                                   $smtp->hello($smtp_domain);\n>> > -                           } else {\n>> > -                                   die sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n>> > +                                           or die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n>> > +                           }\n>> > +                           else {\n>> > +                                   $smtp->starttls(ssl_verify_params())\n>> > +                                           or die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n>> >                             }\n>> > +                           $smtp_encryption = '';\n>> > +                           # Send EHLO again to receive fresh\n>> > +                           # supported commands\n>> > +                           $smtp->hello($smtp_domain);\n>> >                     }\n>> >             }\n>> >\n"},{"id":"321203","messageId":"20170531214634.GB81679@aiede.mtv.corp.google.com","threadId":"44517","inReplyTo":"20170324213732.29932-1-dennis@kaarsemaker.net","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-05-31T21:46:34Z","receivedAt":"2017-05-31T21:46:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDennis Kaarsemaker wrote:\n\n> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n> isn't that old yet, keep the old code in place and use it when\n> necessary.\n\nThis broke git send-email for me.  The error message is\n\n  Can't locate object method \"starttls\" via package \"Net::SMTP\" at /usr/lib/git-core/git-send-email line 1410.\n\nIs 1.28 the right minimum version?\n\n  $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); print \"\\n\"'\n  2.31\n  $ grep VERSION /usr/share/perl/5.18.2/Net/SMTP.pm\n  use vars qw($VERSION @ISA);\n  $VERSION = \"2.31\";\n  $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm\n  $ dpkg-query -W perl\n  perl    5.18.2-2ubuntu1.1\n\nPatch left unsnipped for reference.\n\nThanks,\nJonathan\n\n> While we're in the area, mark some messages for translation that were\n> not yet marked as such.\n>\n> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n> ---\n>  git-send-email.perl | 54 ++++++++++++++++++++++++++++++++++-------------------\n>  1 file changed, 35 insertions(+), 19 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index eea0a517f7..0d90439d9a 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1353,10 +1353,12 @@ EOF\n>  \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n>  \t\t}\n>  \n> +\t\trequire Net::SMTP;\n> +\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n> +\t\t$smtp_domain ||= maildomain();\n> +\n>  \t\tif ($smtp_encryption eq 'ssl') {\n>  \t\t\t$smtp_server_port ||= 465; # ssmtp\n> -\t\t\trequire Net::SMTP::SSL;\n> -\t\t\t$smtp_domain ||= maildomain();\n>  \t\t\trequire IO::Socket::SSL;\n>  \n>  \t\t\t# Suppress \"variable accessed once\" warning.\n> @@ -1368,34 +1370,48 @@ EOF\n>  \t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n>  \t\t\tIO::Socket::SSL::set_client_defaults(\n>  \t\t\t\tssl_verify_params());\n> -\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> -\t\t\t\t\t\t      Hello => $smtp_domain,\n> -\t\t\t\t\t\t      Port => $smtp_server_port,\n> -\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> +\n> +\t\t\tif ($use_net_smtp_ssl) {\n> +\t\t\t\trequire Net::SMTP::SSL;\n> +\t\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n> +\t\t\t\t\t\t\t      Hello => $smtp_domain,\n> +\t\t\t\t\t\t\t      Port => $smtp_server_port,\n> +\t\t\t\t\t\t\t      Debug => $debug_net_smtp);\n> +\t\t\t}\n> +\t\t\telse {\n> +\t\t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n> +\t\t\t\t\t\t\t Hello => $smtp_domain,\n> +\t\t\t\t\t\t\t Port => $smtp_server_port,\n> +\t\t\t\t\t\t\t Debug => $debug_net_smtp,\n> +\t\t\t\t\t\t\t SSL => 1);\n> +\t\t\t}\n>  \t\t}\n>  \t\telse {\n> -\t\t\trequire Net::SMTP;\n> -\t\t\t$smtp_domain ||= maildomain();\n>  \t\t\t$smtp_server_port ||= 25;\n>  \t\t\t$smtp ||= Net::SMTP->new($smtp_server,\n>  \t\t\t\t\t\t Hello => $smtp_domain,\n>  \t\t\t\t\t\t Debug => $debug_net_smtp,\n>  \t\t\t\t\t\t Port => $smtp_server_port);\n>  \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n> -\t\t\t\trequire Net::SMTP::SSL;\n> -\t\t\t\t$smtp->command('STARTTLS');\n> -\t\t\t\t$smtp->response();\n> -\t\t\t\tif ($smtp->code == 220) {\n> +\t\t\t\tif ($use_net_smtp_ssl) {\n> +\t\t\t\t\t$smtp->command('STARTTLS');\n> +\t\t\t\t\t$smtp->response();\n> +\t\t\t\t\tif ($smtp->code != 220) {\n> +\t\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> +\t\t\t\t\t}\n> +\t\t\t\t\trequire Net::SMTP::SSL;\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! \".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> -\t\t\t\t\t$smtp->hello($smtp_domain);\n> -\t\t\t\t} else {\n> -\t\t\t\t\tdie sprintf(__(\"Server does not support STARTTLS! %s\"), $smtp->message);\n> +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n> +\t\t\t\t}\n> +\t\t\t\telse {\n> +\t\t\t\t\t$smtp->starttls(ssl_verify_params())\n> +\t\t\t\t\t\tor die sprintf(__(\"STARTTLS failed! %s\"), IO::Socket::SSL::errstr());\n>  \t\t\t\t}\n> +\t\t\t\t$smtp_encryption = '';\n> +\t\t\t\t# Send EHLO again to receive fresh\n> +\t\t\t\t# supported commands\n> +\t\t\t\t$smtp->hello($smtp_domain);\n>  \t\t\t}\n>  \t\t}\n>  \n> -- \n> 2.12.0-488-gd3584ba\n> \n"},{"id":"321210","messageId":"xmqq7f0wabxn.fsf@gitster.mtv.corp.google.com","threadId":"44517","inReplyTo":"20170531214634.GB81679@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-31T22:39:16Z","receivedAt":"2017-05-31T22:39:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Dennis Kaarsemaker wrote:\n>\n>> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n>> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n>> isn't that old yet, keep the old code in place and use it when\n>> necessary.\n>\n> This broke git send-email for me.  The error message is\n>\n>   Can't locate object method \"starttls\" via package \"Net::SMTP\" at /usr/lib/git-core/git-send-email line 1410.\n>\n> Is 1.28 the right minimum version?\n>\n>   $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); print \"\\n\"'\n>   2.31\n>   $ grep VERSION /usr/share/perl/5.18.2/Net/SMTP.pm\n>   use vars qw($VERSION @ISA);\n>   $VERSION = \"2.31\";\n>   $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm\n>   $ dpkg-query -W perl\n>   perl    5.18.2-2ubuntu1.1\n>\nThanks.  \n\nLet's revert the merge for now until we know (this time for certain)\nwhat the minimum version is.\n"},{"id":"321212","messageId":"xmqq37bkabfl.fsf@gitster.mtv.corp.google.com","threadId":"44517","inReplyTo":"1495227246.19473.3.camel@kaarsemaker.net","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-31T22:50:06Z","receivedAt":"2017-05-31T22:50:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n\n> Second ping. This problem is not going away, so if this solution is not\n> acceptable, I'd like to know what needs to be improved.\n\nPerhaps you needed to actually test with older installation that\npeople have, it seems, between pings.  Immediately after this was\nmerged to 'master', we start getting bug reports X-<.\n\nEric Biggers' message \n\n    https://public-inbox.org/git/<20170531222455.GD72735@gmail.com>\n\nseems to indicate that we should cut off at 3.01 not 1.28?\n\nThanks.\n\n> On Thu, 2017-05-04 at 09:01 +0200, Dennis Kaarsemaker wrote:\n>> Ping. It's a little over a month since I sent this, but I haven't seen\n>> any comments. Is this commit good to go?\n>> \n>> On Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:\n>> > Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine\n>> > since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28\n>> > isn't that old yet, keep the old code in place and use it when\n>> > necessary.\n>> > ...\n>> > diff --git a/git-send-email.perl b/git-send-email.perl\n>> > index eea0a517f7..0d90439d9a 100755\n>> > --- a/git-send-email.perl\n>> > +++ b/git-send-email.perl\n>> > @@ -1353,10 +1353,12 @@ EOF\n>> >  \t\t\tdie __(\"The required SMTP server is not properly defined.\")\n>> >  \t\t}\n>> >  \n>> > +\t\trequire Net::SMTP;\n>> > +\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"1.28\");\n"},{"id":"321214","messageId":"20170531225348.GD81679@aiede.mtv.corp.google.com","threadId":"44517","inReplyTo":"xmqq7f0wabxn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-05-31T22:53:48Z","receivedAt":"2017-05-31T22:53:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJun 01, 2017 at 07:39:16AM +0900, Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> This broke git send-email for me.  The error message is\n>>\n>>   Can't locate object method \"starttls\" via package \"Net::SMTP\" at /usr/lib/git-core/git-send-email line 1410.\n>>\n>> Is 1.28 the right minimum version?\n>>\n>>   $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); print \"\\n\"'\n[...]\n>>   $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm\n>>   $ dpkg-query -W perl\n>>   perl    5.18.2-2ubuntu1.1\n>\n> Thanks.\n>\n> Let's revert the merge for now until we know (this time for certain)\n> what the minimum version is.\n\nThanks.  I just sent\nhttp://public-inbox.org/git/20170531224415.GC81679@aiede.mtv.corp.google.com\nin response to another thread.  That uses 3.01 as minimum version,\nsince it is the minimum version imported to core perl with starttls\nsupport.\n\nI haven't tried testing with historical Net::SMTP versions, though.\nIs there a git repository available with all Net::SMTP versions from\nCPAN, or is https://perl5.git.perl.org/perl.git as good as it gets?\n\nRegards,\nJonathan\n"},{"id":"321318","messageId":"1496346143.26495.1.camel@kaarsemaker.net","threadId":"44517","inReplyTo":"xmqq37bkabfl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-06-01T19:42:23Z","receivedAt":"2017-06-01T19:42:30Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Thu, 2017-06-01 at 07:50 +0900, Junio C Hamano wrote:\n> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n> \n> > Second ping. This problem is not going away, so if this solution is not\n> > acceptable, I'd like to know what needs to be improved.\n> \n> Perhaps you needed to actually test with older installation that\n> people have, it seems, between pings.  Immediately after this was\n> merged to 'master', we start getting bug reports X-<.\n> \n> Eric Biggers' message \n> \n>     https://public-inbox.org/git/<20170531222455.GD72735@gmail.com>;\n> \n> seems to indicate that we should cut off at 3.01 not 1.28?\n> \n> Thanks.\n\nApologies for that. The version numbering of libnet is *weird*. The\nlibnet version this was fixed in *is* 1.28, but the Net::SMTP module in\n libnet version 1.28 has version 2.33.\n\nI did test with some older perl versions, but not far enough back it\nseems :(\n\nD.\n"}]}