[PATCH] send-email: Fix tls AUTH when sending batch

[PATCH] send-email: Fix tls AUTH when sending batch

[Jules Maselbas]

The variable smtp_encryption must keep it's value between two batches.
Otherwise the authentication is skipped after the first batch.

Signed-off-by: Jules Maselbas <jules.maselbas@grenoble-inp.org>
---
 git-send-email.perl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 8ec70e58e..543e18145 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1501,7 +1501,6 @@ sub send_message {
 					$smtp->starttls(ssl_verify_params())
 						or die sprintf(__("STARTTLS failed! %s"), IO::Socket::SSL::errstr());
 				}
-				$smtp_encryption = '';
 				# Send again to receive fresh
 				# supported commands
 				$smtp->hello($smtp_domain);
-- 
2.18.0

[PATCH v2] send-email: Fix tls AUTH when sending batch

[Jules Maselbas]

The variable smtp_encryption must keep it's value between two batches.
Otherwise the authentication is skipped after the first batch.

Signed-off-by: Jules Maselbas <jules.maselbas@grenoble-inp.org>
---
 git-send-email.perl | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 8ec70e58e..1f9a73f74 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1479,7 +1479,7 @@ sub send_message {
 							 SSL => 1);
 			}
 		}
-		else {
+		elsif (!$smtp) {
 			$smtp_server_port ||= 25;
 			$smtp ||= Net::SMTP->new($smtp_server,
 						 Hello => $smtp_domain,
@@ -1501,7 +1501,6 @@ sub send_message {
 					$smtp->starttls(ssl_verify_params())
 						or die sprintf(__("STARTTLS failed! %s"), IO::Socket::SSL::errstr());
 				}
-				$smtp_encryption = '';
 				# Send again to receive fresh
 				# supported commands
 				$smtp->hello($smtp_domain);
-- 
2.18.0

Re: [PATCH v2] send-email: Fix tls AUTH when sending batch

[Junio C Hamano]

Jules Maselbas <jules.maselbas@grenoble-inp.org> writes:

> The variable smtp_encryption must keep it's value between two batches.
> Otherwise the authentication is skipped after the first batch.
>
> Signed-off-by: Jules Maselbas <jules.maselbas@grenoble-inp.org>
> ---
>  git-send-email.perl | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 8ec70e58e..1f9a73f74 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1479,7 +1479,7 @@ sub send_message {
>  							 SSL => 1);
>  			}
>  		}
> -		else {
> +		elsif (!$smtp) {
>  			$smtp_server_port ||= 25;
>  			$smtp ||= Net::SMTP->new($smtp_server,
>  						 Hello => $smtp_domain,

Puzzled...  The code is prepared to deal with the case where $smtp
is still active at this point by using "$smtp ||=", but this hunk
forces any caller that still has $smtp active from entering this
block.

So the conditional assignment "$smtp ||= Net::SMTP->new()" is
meaningless after this patch---we always do "new" if we reach this
statement.

This hunk did not exist in your v1, yet the proposed log message has
not changed since v1.  With this hunk, it appears that the problem
and the solution are not about $smtp_encryption but is about not
calling ->starttls() or ->command('STARTTLS') when we reuse $smtp
that has been established earlier, and the description in the log
message feels a bit off.  Some exaplantion on why this hunk is a
good thing may be missing, I guess.

Dropping the assignment to $smtp_encryption in this block (i.e. hunk
below) makes sense, as we do not call read_config() to re-read its
value after the batching code quits and unsets $smtp.

> @@ -1501,7 +1501,6 @@ sub send_message {
>  					$smtp->starttls(ssl_verify_params())
>  						or die sprintf(__("STARTTLS failed! %s"), IO::Socket::SSL::errstr());
>  				}
> -				$smtp_encryption = '';
>  				# Send again to receive fresh

By the way, the patch claims that it is against 8ec70e58e, but has
an edit on this context line and does not apply.  Did you hand-edit
your patch after generating it?  Please don't.


>  				# supported commands
>  				$smtp->hello($smtp_domain);