git-send-email fixes, cleanups and improvements

10 messages, 1 author, 2016-06-15 · open the first message on its own page

git-send-email fixes, cleanups and improvements

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

The following series of patches perform a number of fixes, cleanups and
improvements to git-send-email.

These were spurred by the dual factors of git-send-email losing some of my
email due to the envelope sender, as well as the malformatting of the headers
(due to the period after my middle initial) that caused strange breakages with
some sendmail binaries.

01/09  Document --dry-run parameter to send-email.
02/09  Prefix Dry- to the message status to denote dry-runs.
03/09  Debugging cleanup improvements
04/09  Change the scope of the $cc variable as it is not needed outside of send_message.
05/09  Perform correct quoting of recipient names.
06/09  Validate @recipients before using it for sendmail and Net::SMTP.
07/09  Ensure clean addresses are always used with Net::SMTP
08/09  Allow users to optionally specify their envelope sender.
09/09  Document --dry-run and envelope-sender for git-send-email.

[PATCH 1/9] Document --dry-run parameter to send-email.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

Looks like --dry-run was added to the code, but never to the --help output.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index d6b1548..604168e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -77,6 +77,8 @@ Options:
    --quiet	  Make git-send-email less verbose.  One line per email
                   should be all that is output.
 
+   --dry-run	  Do everything except actually send the emails.
+
 EOT
 	exit(1);
 }
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 2/9] Prefix Dry- to the message status to denote dry-runs.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

While doing testing, it's useful to see that a dry run was actually done,
instead of a real one.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 604168e..00f8181 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -491,9 +491,9 @@ X-Mailer: git-send-email $gitversion
 		$smtp->ok or die "Failed to send $subject\n".$smtp->message;
 	}
 	if ($quiet) {
-		printf "Sent %s\n", $subject;
+		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
-		print "OK. Log says:\nDate: $date\n";
+		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
 		if ($smtp) {
 			print "Server: $smtp_server\n";
 		} else {
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 3/9] Debugging cleanup improvements

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

The debug output is much more helpful if it has the parameters that were used.
Pull the sendmail parameters into a seperate array for that, and also include
similar data during the Net::SMTP case.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 00f8181..8af235f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -468,15 +468,15 @@ X-Mailer: git-send-email $gitversion
 		$header .= join("\n", @xh) . "\n";
 	}
 
+	my @sendmail_parameters = ('-i', map { extract_valid_address($_) } @recipients);
+
 	if ($dry_run) {
 		# We don't want to send the email.
 	} elsif ($smtp_server =~ m#^/#) {
 		my $pid = open my $sm, '|-';
 		defined $pid or die $!;
 		if (!$pid) {
-			exec($smtp_server,'-i',
-			     map { extract_valid_address($_) }
-			     @recipients) or die $!;
+			exec($smtp_server, @sendmail_parameters) or die $!;
 		}
 		print $sm "$header\n$message";
 		close $sm or die $?;
@@ -496,8 +496,10 @@ X-Mailer: git-send-email $gitversion
 		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
 		if ($smtp) {
 			print "Server: $smtp_server\n";
+			print "MAIL FROM: $from\n";
+			print "RCPT TO: ".join(',',@recipients)."\n";
 		} else {
-			print "Sendmail: $smtp_server\n";
+			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
 		print "From: $from\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
 		if ($smtp) {
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 6/9] Validate @recipients before using it for sendmail and Net::SMTP.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

Ensure that @recipients is only raw addresses when it is handed to the sendmail
binary OR Net::SMTP, otherwise BCC cases might get an extra <, or wierd stuff
might be passed to the exec.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index c052760..0e1cc16 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -449,6 +449,7 @@ sub send_message
 	@cc = (map { sanitize_address_rfc822($_) } @cc);
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
+	@recipients = (map { extract_valid_address($_) } @recipients);
 	my $date = format_2822_time($time++);
 	my $gitversion = '@@GIT_VERSION@@';
 	if ($gitversion =~ m/..GIT_VERSION../) {
@@ -477,7 +478,7 @@ X-Mailer: git-send-email $gitversion
 		$header .= join("\n", @xh) . "\n";
 	}
 
-	my @sendmail_parameters = ('-i', map { extract_valid_address($_) } @recipients);
+	my @sendmail_parameters = ('-i', @recipients);
 
 	if ($dry_run) {
 		# We don't want to send the email.
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 5/9] Perform correct quoting of recipient names.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

Always perform quoting of the recipient names if they contain periods,
previously only the author's address was treated this way. This stops sendmail
binaries from exploding the name into bad addresses.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5210a40..c052760 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -431,9 +431,22 @@ sub unquote_rfc2047 {
 	return "$_";
 }
 
+# If an address contains a . in the name portion, the name must be quoted.
+sub sanitize_address_rfc822 
+{
+	my ($recipient) = @_;
+	my ($recipient_name) = ($recipient =~ /^(.*?)\s+</);
+	if($recipient_name && $recipient_name =~ /\./ && $recipient_name !~ /^".*"$/) {
+		my ($name, $addr) = ($recipient =~ /^(.*?)(\s+<.*)/);
+		$recipient = "\"$name\"$addr";
+	}
+	return $recipient;
+}
+
 sub send_message
 {
 	my @recipients = unique_email_list(@to);
+	@cc = (map { sanitize_address_rfc822($_) } @cc);
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
 	my $date = format_2822_time($time++);
@@ -442,11 +455,7 @@ sub send_message
 	    $gitversion = Git::version();
 	}
 
-	my ($author_name) = ($from =~ /^(.*?)\s+</);
-	if ($author_name && $author_name =~ /\./ && $author_name !~ /^".*"$/) {
-		my ($name, $addr) = ($from =~ /^(.*?)(\s+<.*)/);
-		$from = "\"$name\"$addr";
-	}
+	$from = sanitize_address_rfc822($from);
 	my $cc = join(", ", unique_email_list(@cc));
 	my $ccline = "";
 	if ($cc ne '') {
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 9/9] Document --dry-run and envelope-sender for git-send-email.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

Catch the documentation up with the rest of this patchset.

Signed-off-by: Robin H. Johnson <redacted>
---
 Documentation/git-send-email.txt |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 682313e..393d79a 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -85,6 +85,15 @@ The --cc option must be repeated for each user you want on the cc list.
 	Do not add the From: address to the cc: list, if it shows up in a From:
 	line.
 
+--dry-run::
+	Do everything except actually send the emails.
+
+--envelope-sender::
+	Specify the envelope sender used to send the emails. 
+	This is useful if your default address is not the address that is
+	subscribed to a list. If you use the sendmail binary, you must have
+	suitable privileges for the -f parameter.
+
 --to::
 	Specify the primary recipient of the emails generated.
 	Generally, this will be the upstream maintainer of the
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 8/9] Allow users to optionally specify their envelope sender.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

If your normal user is not the same user you are subscribed to a list with,
then the default envelope sender used will cause your messages to bounce or
silently vanish into the ether.

This patch provides an optional parameter to set the envelope sender.
To use it with the sendmail binary, you must have privileges to use the -f
parameter!

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 82468bd..e1562b3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -79,6 +79,8 @@ Options:
 
    --dry-run	  Do everything except actually send the emails.
 
+   --envelope-sender	Specify the envelope sender used to send the emails.
+
 EOT
 	exit(1);
 }
@@ -139,6 +141,7 @@ my (@to,@cc,@initial_cc,@bcclist,@xh,
 my ($chain_reply_to, $quiet, $suppress_from, $no_signed_off_cc,
 	$dry_run) = (1, 0, 0, 0, 0);
 my $smtp_server;
+my $envelope_sender;
 
 # Example reply to:
 #$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
@@ -177,6 +180,7 @@ my $rc = GetOptions("from=s" => \$from,
 		    "suppress-from" => \$suppress_from,
 		    "no-signed-off-cc|no-signed-off-by-cc" => \$no_signed_off_cc,
 		    "dry-run" => \$dry_run,
+		    "envelope-sender=s" => \$envelope_sender,
 	 );
 
 unless ($rc) {
@@ -479,7 +483,11 @@ X-Mailer: git-send-email $gitversion
 	}
 
 	my @sendmail_parameters = ('-i', @recipients);
-	my $raw_from = extract_valid_address($from);
+	my $raw_from = $from;
+	$raw_from = $envelope_sender if (defined $envelope_sender);
+	$raw_from = extract_valid_address($raw_from);
+	unshift (@sendmail_parameters,
+			'-f', $raw_from) if(defined $envelope_sender);
 
 	if ($dry_run) {
 		# We don't want to send the email.
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 4/9] Change the scope of the $cc variable as it is not needed outside of send_message.

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

$cc is only used inside the send_message scope, so lets clean it out of the global scope.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 8af235f..5210a40 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,7 +379,7 @@ if (@files) {
 }
 
 # Variables we set as part of the loop over files
-our ($message_id, $cc, %mail, $subject, $reply_to, $references, $message);
+our ($message_id, %mail, $subject, $reply_to, $references, $message);
 
 sub extract_valid_address {
 	my $address = shift;
@@ -420,7 +420,6 @@ sub make_message_id
 
 
 
-$cc = "";
 $time = time - scalar $#files;
 
 sub unquote_rfc2047 {
@@ -448,6 +447,7 @@ sub send_message
 		my ($name, $addr) = ($from =~ /^(.*?)(\s+<.*)/);
 		$from = "\"$name\"$addr";
 	}
+	my $cc = join(", ", unique_email_list(@cc));
 	my $ccline = "";
 	if ($cc ne '') {
 		$ccline = "\nCc: $cc";
@@ -594,7 +594,6 @@ foreach my $t (@files) {
 		$message = "From: $author_not_sender\n\n$message";
 	}
 
-	$cc = join(", ", unique_email_list(@cc));
 
 	send_message();
 
-- 
1.5.2.rc0.43.g2f4c7

[PATCH 7/9] Ensure clean addresses are always used with Net::SMTP

From: Robin H. Johnson <hidden>
Date: 2016-06-15 22:43:07

From: Robin H. Johnson <redacted>

Always pass in clean addresses to Net::SMTP for the MAIL FROM, and use them on
the SMTP non-quiet output as well.

Signed-off-by: Robin H. Johnson <redacted>
---
 git-send-email.perl |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 0e1cc16..82468bd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -479,6 +479,7 @@ X-Mailer: git-send-email $gitversion
 	}
 
 	my @sendmail_parameters = ('-i', @recipients);
+	my $raw_from = extract_valid_address($from);
 
 	if ($dry_run) {
 		# We don't want to send the email.
@@ -493,7 +494,7 @@ X-Mailer: git-send-email $gitversion
 	} else {
 		require Net::SMTP;
 		$smtp ||= Net::SMTP->new( $smtp_server );
-		$smtp->mail( $from ) or die $smtp->message;
+		$smtp->mail( $raw_from ) or die $smtp->message;
 		$smtp->to( @recipients ) or die $smtp->message;
 		$smtp->data or die $smtp->message;
 		$smtp->datasend("$header\n$message") or die $smtp->message;
@@ -504,10 +505,10 @@ X-Mailer: git-send-email $gitversion
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
 		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
-		if ($smtp) {
+		if ($smtp_server !~ m#^/#) {
 			print "Server: $smtp_server\n";
-			print "MAIL FROM: $from\n";
-			print "RCPT TO: ".join(',',@recipients)."\n";
+			print "MAIL FROM:<$raw_from>\n";
+			print "RCPT TO:".join(',',(map { "<$_>" } @recipients))."\n";
 		} else {
 			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
-- 
1.5.2.rc0.43.g2f4c7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help