[PATCH v3 0/6] git send-email suppress-cc=self fixes

[PATCH v3 0/6] git send-email suppress-cc=self fixes

[Michael S. Tsirkin]

This includes bugfixes related to handling of --suppress-cc=self
flag. Tests are also included.

Changes from v2:
	- add a new test, split patches differently add code comments
		 to address comments by Junio
	- rename example addresses in tests from redhat.com to example.com
Changes from v1:
        - tweak coding style in tests to address comments by Junio


Michael S. Tsirkin (6):
  send-email: fix suppress-cc=self on cccmd
  t/send-email: test suppress-cc=self on cccmd
  send-email: make --suppress-cc=self sanitize input
  t/send-email: add test with quoted sender
  t/send-email: test suppress-cc=self with non-ascii
  test-send-email: test for pre-sanitized self name

 git-send-email.perl   | 23 +++++++++++++++--------
 t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 9 deletions(-)

-- 
MST

[PATCH v3 1/6] send-email: fix suppress-cc=self on cccmd

[Michael S. Tsirkin]

When cccmd is used, old-style suppress-from filter
is applied by the newer suppress-cc=self isn't.
Fix this up.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

This makes one line a bit too long, but a follow-up patch
fixes this up.

 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc8..f366baa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1462,7 +1462,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 $sanitized_sender and $suppress_cc{'self'});
 		push @addresses, $address;
 		printf("($prefix) Adding %s: %s from: '%s'\n",
 		       $what, $address, $cmd) unless $quiet;
-- 
MST

[PATCH v3 2/6] t/send-email: test suppress-cc=self on cccmd

[Michael S. Tsirkin]

Check that suppress-cc=self works when applied
to output of cccmd.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t9001-send-email.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e1a7f3e..f81e5f5 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {
 
 		unquoted-$3
 
+		cccmd--$1 <$2>
+
 		Cc: $1 <$2>
 		Signed-off-by: $1 <$2>
 	EOF
 }
 
 test_expect_success $PREREQ 'self name is suppressed' "
-	test_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \
+	test_suppress_self_unquoted 'A U Thor' 'author@example.com' \
 		'self_name_suppressed'
 "
 
-- 
MST

[PATCH v3 3/6] send-email: make --suppress-cc=self sanitize input

[Michael S. Tsirkin]

--suppress-cc=self fails to filter sender address in many cases where it
needs to be sanitized in some way, for example quoted:
"A U. Thor" <author@example.com>
To fix, make send-email sanitize both sender and the address it is
compared against.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 git-send-email.perl | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index f366baa..3b828f6 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;
 	}
@@ -1308,8 +1312,9 @@ foreach my $t (@files) {
 			}
 			elsif (/^From:\s+(.*)$/i) {
 				($author, $author_encoding) = unquote_rfc2047($1);
+				my $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 +1328,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 +1377,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;
@@ -1454,7 +1462,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 +1469,7 @@ sub recipients_cmd {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
-		next if ($address eq $sanitized_sender and $suppress_cc{'self'});
+		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;
-- 
MST

[PATCH v3 4/6] t/send-email: add test with quoted sender

[Michael S. Tsirkin]

add test where sender address needs to be quoted.
Make sure --suppress-cc=self works well in this case.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t9001-send-email.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index f81e5f5..6c0f715 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -211,11 +211,31 @@ test_suppress_self_unquoted () {
 	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 'Show all headers' '
 	git send-email \
 		--dry-run \
-- 
MST

[PATCH v3 5/6] t/send-email: test suppress-cc=self with non-ascii

[Michael S. Tsirkin]

test suppress-cc=self when sender is non-acsii

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t9001-send-email.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6c0f715..0d50fa7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -236,6 +236,11 @@ test_expect_success $PREREQ 'self name with dot is suppressed' "
 		'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 'Show all headers' '
 	git send-email \
 		--dry-run \
-- 
MST

[PATCH v3 6/6] test-send-email: test for pre-sanitized self name

[Michael S. Tsirkin]

Users can sanitize from address manually.
Verify that these are suppressed properly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t9001-send-email.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 0d50fa7..38f407d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -241,6 +241,11 @@ test_expect_success $PREREQ 'non-ascii self name is suppressed' "
 		'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 \
-- 
MST

Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> This includes bugfixes related to handling of --suppress-cc=self
> flag. Tests are also included.

Hmph, flipped the patches without test-applying first?

2/6 adds two lines to test_suppress_self_quoted helper function, but
that is introduced only at 4/6.

>
> Changes from v2:
> 	- add a new test, split patches differently add code comments
> 		 to address comments by Junio
> 	- rename example addresses in tests from redhat.com to example.com
> Changes from v1:
>         - tweak coding style in tests to address comments by Junio
>
>
> Michael S. Tsirkin (6):
>   send-email: fix suppress-cc=self on cccmd
>   t/send-email: test suppress-cc=self on cccmd
>   send-email: make --suppress-cc=self sanitize input
>   t/send-email: add test with quoted sender
>   t/send-email: test suppress-cc=self with non-ascii
>   test-send-email: test for pre-sanitized self name
>
>  git-send-email.perl   | 23 +++++++++++++++--------
>  t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 9 deletions(-)

Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

[Michael S. Tsirkin]

On Tue, Jun 04, 2013 at 10:40:51AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > This includes bugfixes related to handling of --suppress-cc=self
> > flag. Tests are also included.
> 
> Hmph, flipped the patches without test-applying first?

No, I generated the patches with git format-patch,
then processed with git send-email.

> 2/6 adds two lines to test_suppress_self_quoted helper function, but
> that is introduced only at 4/6.

I don't see it
All I see in 2/6 is this:

	diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
	index e1a7f3e..f81e5f5 100755
	--- a/t/t9001-send-email.sh
	+++ b/t/t9001-send-email.sh
	@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {

			unquoted-$3

	+               cccmd--$1 <$2>
	+
			Cc: $1 <$2>
			Signed-off-by: $1 <$2>
		EOF
	 }

	 test_expect_success $PREREQ 'self name is suppressed' "
	-       test_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \
	+       test_suppress_self_unquoted 'A U Thor' 'author@example.com' \
			'self_name_suppressed'
	 "

where's test_suppress_self_quoted here?


> >
> > Changes from v2:
> > 	- add a new test, split patches differently add code comments
> > 		 to address comments by Junio
> > 	- rename example addresses in tests from redhat.com to example.com
> > Changes from v1:
> >         - tweak coding style in tests to address comments by Junio
> >
> >
> > Michael S. Tsirkin (6):
> >   send-email: fix suppress-cc=self on cccmd
> >   t/send-email: test suppress-cc=self on cccmd
> >   send-email: make --suppress-cc=self sanitize input
> >   t/send-email: add test with quoted sender
> >   t/send-email: test suppress-cc=self with non-ascii
> >   test-send-email: test for pre-sanitized self name
> >
> >  git-send-email.perl   | 23 +++++++++++++++--------
> >  t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-
> >  2 files changed, 48 insertions(+), 9 deletions(-)

Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> All I see in 2/6 is this:
>
> 	diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> 	index e1a7f3e..f81e5f5 100755
> 	--- a/t/t9001-send-email.sh
> 	+++ b/t/t9001-send-email.sh
> 	@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {

After noticing what is after "@@" here...

>
> 			unquoted-$3
>
> 	+               cccmd--$1 <$2>
> 	+
> 			Cc: $1 <$2>
> 			Signed-off-by: $1 <$2>
> 		EOF
> 	 }
>
> 	 test_expect_success $PREREQ 'self name is suppressed' "
> 	-       test_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \
> 	+       test_suppress_self_unquoted 'A U Thor' 'author@example.com' \
> 			'self_name_suppressed'
> 	 "
>
> where's test_suppress_self_quoted here?

... isn't addition of "cccmd--$1 <$2>" made to that function?

After applying [PATCH v3 1/6] to 'master', I do not see unquoted-$3
that we see in the context, either.

"git grep unquoted- pu t/t9001*" does show us a hit, but that is
coming from your previous round you are replacing with this series,
so....

Confused...