Merge branch 'mt/send-email-cc-match-fix' into maint

Logic used by git-send-email to suppress cc mishandled names like "A
U. Thor" <author@example.xz>, where the human readable part needs to
be quoted (the user input may not have the double quotes around the
name, and comparison was done between quoted and unquoted strings).
It also mishandled names that need RFC2047 quoting.

* mt/send-email-cc-match-fix:
  send-email: sanitize author when writing From line
  send-email: add test for duplicate utf8 name
  test-send-email: test for pre-sanitized self name
  t/send-email: test suppress-cc=self with non-ascii
  t/send-email: add test with quoted sender
  send-email: make --suppress-cc=self sanitize input
  t/send-email: test suppress-cc=self on cccmd
  send-email: fix suppress-cc=self on cccmd
  t/send-email.sh: add test for suppress-cc=self

2 files changed, 106 insertions, 9 deletions

diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc812d..cd93490c87 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -760,6 +760,11 @@ if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
 }
 
+# $sender could be an already sanitized address
+# (e.g. sendemail.from could be manually sanitized by user).
+# But it's a no-op to run sanitize_address on an already sanitized address.
+$sender = sanitize_address($sender);
+
 my $prompting = 0;
 if (!@initial_to && !defined $to_cmd) {
 	my $to = ask("Who should the emails be sent to (if any)? ",
@@ -1113,10 +1118,9 @@ sub send_message {
 	if ($cc ne '') {
 		$ccline = "\nCc: $cc";
 	}
-	my $sanitized_sender = sanitize_address($sender);
 	make_message_id() unless defined($message_id);
 
-	my $header = "From: $sanitized_sender
+	my $header = "From: $sender
 To: $to${ccline}
 Subject: $subject
 Date: $date
@@ -1133,7 +1137,7 @@ X-Mailer: git-send-email $gitversion
 	}
 
 	my @sendmail_parameters = ('-i', @recipients);
-	my $raw_from = $sanitized_sender;
+	my $raw_from = $sender;
 	if (defined $envelope_sender && $envelope_sender ne "auto") {
 		$raw_from = $envelope_sender;
 	}
@@ -1270,6 +1274,7 @@ foreach my $t (@files) {
 	open my $fh, "<", $t or die "can't open file $t";
 
 	my $author = undef;
+	my $sauthor = undef;
 	my $author_encoding;
 	my $has_content_type;
 	my $body_encoding;
@@ -1308,8 +1313,9 @@ foreach my $t (@files) {
 			}
 			elsif (/^From:\s+(.*)$/i) {
 				($author, $author_encoding) = unquote_rfc2047($1);
+				$sauthor = sanitize_address($author);
 				next if $suppress_cc{'author'};
-				next if $suppress_cc{'self'} and $author eq $sender;
+				next if $suppress_cc{'self'} and $sauthor eq $sender;
 				printf("(mbox) Adding cc: %s from line '%s'\n",
 					$1, $_) unless $quiet;
 				push @cc, $1;
@@ -1323,7 +1329,9 @@ foreach my $t (@files) {
 			}
 			elsif (/^Cc:\s+(.*)$/i) {
 				foreach my $addr (parse_address_line($1)) {
-					if (unquote_rfc2047($addr) eq $sender) {
+					my $qaddr = unquote_rfc2047($addr);
+					my $saddr = sanitize_address($qaddr);
+					if ($saddr eq $sender) {
 						next if ($suppress_cc{'self'});
 					} else {
 						next if ($suppress_cc{'cc'});
@@ -1370,7 +1378,8 @@ foreach my $t (@files) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
-			if ($c eq $sender) {
+			my $sc = sanitize_address($c);
+			if ($sc eq $sender) {
 				next if ($suppress_cc{'self'});
 			} else {
 				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
@@ -1400,7 +1409,7 @@ foreach my $t (@files) {
 		$subject = quote_subject($subject, $auto_8bit_encoding);
 	}
 
-	if (defined $author and $author ne $sender) {
+	if (defined $sauthor and $sauthor ne $sender) {
 		$message = "From: $author\n\n$message";
 		if (defined $author_encoding) {
 			if ($has_content_type) {
@@ -1454,7 +1463,6 @@ foreach my $t (@files) {
 sub recipients_cmd {
 	my ($prefix, $what, $cmd, $file) = @_;
 
-	my $sanitized_sender = sanitize_address($sender);
 	my @addresses = ();
 	open my $fh, "-|", "$cmd \Q$file\E"
 	    or die "($prefix) Could not execute '$cmd'";
@@ -1462,7 +1470,7 @@ sub recipients_cmd {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
-		next if ($address eq $sanitized_sender and $suppress_from);
+		next if ($address eq $sender and $suppress_cc{'self'});
 		push @addresses, $address;
 		printf("($prefix) Adding %s: %s from: '%s'\n",
 		       $what, $address, $cmd) unless $quiet;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ebd5c5db45..d6f0271866 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -171,6 +171,81 @@ Result: OK
 EOF
 "
 
+test_suppress_self () {
+	test_commit $3 &&
+	test_when_finished "git reset --hard HEAD^" &&
+
+	write_script cccmd-sed <<-EOF &&
+		sed -n -e s/^cccmd--//p "\$1"
+	EOF
+
+	git commit --amend --author="$1 <$2>" -F - &&
+	clean_fake_sendmail &&
+	git format-patch --stdout -1 >"suppress-self-$3.patch" &&
+
+	git send-email --from="$1 <$2>" \
+		--to=nobody@example.com \
+		--cc-cmd=./cccmd-sed \
+		--suppress-cc=self \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		suppress-self-$3.patch &&
+
+	mv msgtxt1 msgtxt1-$3 &&
+	sed -e '/^$/q' msgtxt1-$3 >"msghdr1-$3" &&
+	>"expected-no-cc-$3" &&
+
+	(grep '^Cc:' msghdr1-$3 >"actual-no-cc-$3";
+	 test_cmp expected-no-cc-$3 actual-no-cc-$3)
+}
+
+test_suppress_self_unquoted () {
+	test_suppress_self "$1" "$2" "unquoted-$3" <<-EOF
+		test suppress-cc.self unquoted-$3 with name $1 email $2
+
+		unquoted-$3
+
+		cccmd--$1 <$2>
+
+		Cc: $1 <$2>
+		Signed-off-by: $1 <$2>
+	EOF
+}
+
+test_suppress_self_quoted () {
+	test_suppress_self "$1" "$2" "quoted-$3" <<-EOF
+		test suppress-cc.self quoted-$3 with name $1 email $2
+
+		quoted-$3
+
+		cccmd--"$1" <$2>
+
+		Cc: $1 <$2>
+		Cc: "$1" <$2>
+		Signed-off-by: $1 <$2>
+		Signed-off-by: "$1" <$2>
+	EOF
+}
+
+test_expect_success $PREREQ 'self name is suppressed' "
+	test_suppress_self_unquoted 'A U Thor' 'author@example.com' \
+		'self_name_suppressed'
+"
+
+test_expect_success $PREREQ 'self name with dot is suppressed' "
+	test_suppress_self_quoted 'A U. Thor' 'author@example.com' \
+		'self_name_dot_suppressed'
+"
+
+test_expect_success $PREREQ 'non-ascii self name is suppressed' "
+	test_suppress_self_quoted 'Füñný Nâmé' 'odd_?=mail@example.com' \
+		'non_ascii_self_suppressed'
+"
+
+test_expect_success $PREREQ 'sanitized self name is suppressed' "
+	test_suppress_self_unquoted '\"A U. Thor\"' 'author@example.com' \
+		'self_name_sanitized_suppressed'
+"
+
 test_expect_success $PREREQ 'Show all headers' '
 	git send-email \
 		--dry-run \
@@ -881,6 +956,20 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' '
 	grep "^From: Füñný Nâmé <odd_?=mail@example.com>" msgtxt1
 '
 
+test_expect_success $PREREQ 'utf8 sender is not duplicated' '
+	clean_fake_sendmail &&
+	test_commit weird_sender &&
+	test_when_finished "git reset --hard HEAD^" &&
+	git commit --amend --author "Füñný Nâmé <odd_?=mail@example.com>" &&
+	git format-patch --stdout -1 >funny_name.patch &&
+	git send-email --from="Füñný Nâmé <odd_?=mail@example.com>" \
+	  --to=nobody@example.com \
+	  --smtp-server="$(pwd)/fake.sendmail" \
+	  funny_name.patch &&
+	grep "^From: " msgtxt1 >msgfrom &&
+	test_line_count = 1 msgfrom
+'
+
 test_expect_success $PREREQ 'sendemail.composeencoding works' '
 	clean_fake_sendmail &&
 	git config sendemail.composeencoding iso-8859-1 &&