{"thread":{"id":"48884","subject":"[PATCH] send-email: Fix tls AUTH when sending batch","startedAt":"2018-07-14T02:22:56Z","lastAt":"2018-07-16T22:19:28Z","messageCount":3,"participants":["Jules Maselbas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"352545","messageId":"20180714021440.22995-1-jules.maselbas@grenoble-inp.org","threadId":"48884","inReplyTo":null,"subject":"[PATCH] send-email: Fix tls AUTH when sending batch","fromName":"Jules Maselbas","fromEmail":"jules.maselbas@grenoble-inp.org","sentAt":"2018-07-14T02:14:40Z","receivedAt":"2018-07-14T02:22:56Z","isPatch":true,"sender":{"key":"jules.maselbas@grenoble-inp.org","avatar":null},"body":"The variable smtp_encryption must keep it's value between two batches.\nOtherwise the authentication is skipped after the first batch.\n\nSigned-off-by: Jules Maselbas \n---\n git-send-email.perl | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8ec70e58e..543e18145 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1501,7 +1501,6 @@ sub send_message {\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 again to receive fresh\n \t\t\t\t# supported commands\n \t\t\t\t$smtp->hello($smtp_domain);\n-- \n2.18.0\n\n"},{"id":"352547","messageId":"20180714085848.4031-1-jules.maselbas@grenoble-inp.org","threadId":"48884","inReplyTo":"20180714021440.22995-1-jules.maselbas@grenoble-inp.org","subject":"[PATCH v2] send-email: Fix tls AUTH when sending batch","fromName":"Jules Maselbas","fromEmail":"jules.maselbas@grenoble-inp.org","sentAt":"2018-07-14T08:58:48Z","receivedAt":"2018-07-14T09:07:01Z","isPatch":true,"sender":{"key":"jules.maselbas@grenoble-inp.org","avatar":null},"body":"The variable smtp_encryption must keep it's value between two batches.\nOtherwise the authentication is skipped after the first batch.\n\nSigned-off-by: Jules Maselbas \n---\n git-send-email.perl | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8ec70e58e..1f9a73f74 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1479,7 +1479,7 @@ sub send_message {\n \t\t\t\t\t\t\t SSL => 1);\n \t\t\t}\n \t\t}\n-\t\telse {\n+\t\telsif (!$smtp) {\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@@ -1501,7 +1501,6 @@ sub send_message {\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 again to receive fresh\n \t\t\t\t# supported commands\n \t\t\t\t$smtp->hello($smtp_domain);\n-- \n2.18.0\n\n"},{"id":"352702","messageId":"xmqqwoturfh0.fsf@gitster-ct.c.googlers.com","threadId":"48884","inReplyTo":"20180714085848.4031-1-jules.maselbas@grenoble-inp.org","subject":"Re: [PATCH v2] send-email: Fix tls AUTH when sending batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T22:19:23Z","receivedAt":"2018-07-16T22:19:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jules Maselbas writes:\n\n> The variable smtp_encryption must keep it's value between two batches.\n> Otherwise the authentication is skipped after the first batch.\n>\n> Signed-off-by: Jules Maselbas \n> ---\n> git-send-email.perl | 3 +--\n> 1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 8ec70e58e..1f9a73f74 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1479,7 +1479,7 @@ sub send_message {\n> \t\t\t\t\t\t\t SSL => 1);\n> \t\t\t}\n> \t\t}\n> -\t\telse {\n> +\t\telsif (!$smtp) {\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\nPuzzled... The code is prepared to deal with the case where $smtp\nis still active at this point by using \"$smtp ||=\", but this hunk\nforces any caller that still has $smtp active from entering this\nblock.\n\nSo the conditional assignment \"$smtp ||= Net::SMTP->new()\" is\nmeaningless after this patch---we always do \"new\" if we reach this\nstatement.\n\nThis hunk did not exist in your v1, yet the proposed log message has\nnot changed since v1. With this hunk, it appears that the problem\nand the solution are not about $smtp_encryption but is about not\ncalling ->starttls() or ->command('STARTTLS') when we reuse $smtp\nthat has been established earlier, and the description in the log\nmessage feels a bit off. Some exaplantion on why this hunk is a\ngood thing may be missing, I guess.\n\nDropping the assignment to $smtp_encryption in this block (i.e. hunk\nbelow) makes sense, as we do not call read_config() to re-read its\nvalue after the batching code quits and unsets $smtp.\n\n> @@ -1501,7 +1501,6 @@ sub send_message {\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 again to receive fresh\n\nBy the way, the patch claims that it is against 8ec70e58e, but has\nan edit on this context line and does not apply. Did you hand-edit\nyour patch after generating it? Please don't.\n\n\n> \t\t\t\t# supported commands\n> \t\t\t\t$smtp->hello($smtp_domain);\n"}]}