{"thread":{"id":"34021","subject":"[PATCH v3 0/6] git send-email suppress-cc=self fixes","startedAt":"2013-06-04T07:55:59Z","lastAt":"2013-06-04T22:11:34Z","messageCount":10,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":6},"messages":[{"id":"219331","messageId":"1370332482-12329-1-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":null,"subject":"[PATCH v3 0/6] git send-email suppress-cc=self fixes","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:55:59Z","receivedAt":"2013-06-04T07:55:59Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"This includes bugfixes related to handling of --suppress-cc=self\nflag. Tests are also included.\n\nChanges from v2:\n\t- add a new test, split patches differently add code comments\n\t\t to address comments by Junio\n\t- rename example addresses in tests from redhat.com to example.com\nChanges from v1:\n - tweak coding style in tests to address comments by Junio\n\n\nMichael S. Tsirkin (6):\n send-email: fix suppress-cc=self on cccmd\n t/send-email: test suppress-cc=self on cccmd\n send-email: make --suppress-cc=self sanitize input\n t/send-email: add test with quoted sender\n t/send-email: test suppress-cc=self with non-ascii\n test-send-email: test for pre-sanitized self name\n\n git-send-email.perl | 23 +++++++++++++++--------\n t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-\n 2 files changed, 48 insertions(+), 9 deletions(-)\n\n-- \nMST\n"},{"id":"219333","messageId":"1370332482-12329-2-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 1/6] send-email: fix suppress-cc=self on cccmd","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:02Z","receivedAt":"2013-06-04T07:56:02Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"When cccmd is used, old-style suppress-from filter\nis applied by the newer suppress-cc=self isn't.\nFix this up.\n\nSigned-off-by: Michael S. Tsirkin \n---\n\nThis makes one line a bit too long, but a follow-up patch\nfixes this up.\n\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex bd13cc8..f366baa 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1462,7 +1462,7 @@ sub recipients_cmd {\n \t\t$address =~ s/^\\s*//g;\n \t\t$address =~ s/\\s*$//g;\n \t\t$address = sanitize_address($address);\n-\t\tnext if ($address eq $sanitized_sender and $suppress_from);\n+\t\tnext if ($address eq $sanitized_sender and $suppress_cc{'self'});\n \t\tpush @addresses, $address;\n \t\tprintf(\"($prefix) Adding %s: %s from: '%s'\\n\",\n \t\t $what, $address, $cmd) unless $quiet;\n-- \nMST\n"},{"id":"219332","messageId":"1370332482-12329-3-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 2/6] t/send-email: test suppress-cc=self on cccmd","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:05Z","receivedAt":"2013-06-04T07:56:05Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Check that suppress-cc=self works when applied\nto output of cccmd.\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t9001-send-email.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex e1a7f3e..f81e5f5 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {\n \n \t\tunquoted-$3\n \n+\t\tcccmd--$1 <$2>\n+\n \t\tCc: $1 <$2>\n \t\tSigned-off-by: $1 <$2>\n \tEOF\n }\n \n test_expect_success $PREREQ 'self name is suppressed' \"\n-\ttest_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \\\n+\ttest_suppress_self_unquoted 'A U Thor' 'author@example.com' \\\n \t\t'self_name_suppressed'\n \"\n \n-- \nMST\n"},{"id":"219334","messageId":"1370332482-12329-4-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 3/6] send-email: make --suppress-cc=self sanitize input","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:07Z","receivedAt":"2013-06-04T07:56:07Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"--suppress-cc=self fails to filter sender address in many cases where it\nneeds to be sanitized in some way, for example quoted:\n\"A U. Thor\" \nTo fix, make send-email sanitize both sender and the address it is\ncompared against.\n\nSigned-off-by: Michael S. Tsirkin \n---\n git-send-email.perl | 23 +++++++++++++++--------\n 1 file changed, 15 insertions(+), 8 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f366baa..3b828f6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -760,6 +760,11 @@ if (!defined $sender) {\n \t$sender = $repoauthor || $repocommitter || '';\n }\n \n+# $sender could be an already sanitized address\n+# (e.g. sendemail.from could be manually sanitized by user).\n+# But it's a no-op to run sanitize_address on an already sanitized address.\n+$sender = sanitize_address($sender);\n+\n my $prompting = 0;\n if (!@initial_to && !defined $to_cmd) {\n \tmy $to = ask(\"Who should the emails be sent to (if any)? \",\n@@ -1113,10 +1118,9 @@ sub send_message {\n \tif ($cc ne '') {\n \t\t$ccline = \"\\nCc: $cc\";\n \t}\n-\tmy $sanitized_sender = sanitize_address($sender);\n \tmake_message_id() unless defined($message_id);\n \n-\tmy $header = \"From: $sanitized_sender\n+\tmy $header = \"From: $sender\n To: $to${ccline}\n Subject: $subject\n Date: $date\n@@ -1133,7 +1137,7 @@ X-Mailer: git-send-email $gitversion\n \t}\n \n \tmy @sendmail_parameters = ('-i', @recipients);\n-\tmy $raw_from = $sanitized_sender;\n+\tmy $raw_from = $sender;\n \tif (defined $envelope_sender && $envelope_sender ne \"auto\") {\n \t\t$raw_from = $envelope_sender;\n \t}\n@@ -1308,8 +1312,9 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/i) {\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n+\t\t\t\tmy $sauthor = sanitize_address($author);\n \t\t\t\tnext if $suppress_cc{'author'};\n-\t\t\t\tnext if $suppress_cc{'self'} and $author eq $sender;\n+\t\t\t\tnext if $suppress_cc{'self'} and $sauthor eq $sender;\n \t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t$1, $_) unless $quiet;\n \t\t\t\tpush @cc, $1;\n@@ -1323,7 +1328,9 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^Cc:\\s+(.*)$/i) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n-\t\t\t\t\tif (unquote_rfc2047($addr) eq $sender) {\n+\t\t\t\t\tmy $qaddr = unquote_rfc2047($addr);\n+\t\t\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\t\t\tif ($saddr eq $sender) {\n \t\t\t\t\t\tnext if ($suppress_cc{'self'});\n \t\t\t\t\t} else {\n \t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n@@ -1370,7 +1377,8 @@ foreach my $t (@files) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n-\t\t\tif ($c eq $sender) {\n+\t\t\tmy $sc = sanitize_address($c);\n+\t\t\tif ($sc eq $sender) {\n \t\t\t\tnext if ($suppress_cc{'self'});\n \t\t\t} else {\n \t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n@@ -1454,7 +1462,6 @@ foreach my $t (@files) {\n sub recipients_cmd {\n \tmy ($prefix, $what, $cmd, $file) = @_;\n \n-\tmy $sanitized_sender = sanitize_address($sender);\n \tmy @addresses = ();\n \topen my $fh, \"-|\", \"$cmd \\Q$file\\E\"\n \t or die \"($prefix) Could not execute '$cmd'\";\n@@ -1462,7 +1469,7 @@ sub recipients_cmd {\n \t\t$address =~ s/^\\s*//g;\n \t\t$address =~ s/\\s*$//g;\n \t\t$address = sanitize_address($address);\n-\t\tnext if ($address eq $sanitized_sender and $suppress_cc{'self'});\n+\t\tnext if ($address eq $sender and $suppress_cc{'self'});\n \t\tpush @addresses, $address;\n \t\tprintf(\"($prefix) Adding %s: %s from: '%s'\\n\",\n \t\t $what, $address, $cmd) unless $quiet;\n-- \nMST\n"},{"id":"219335","messageId":"1370332482-12329-5-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 4/6] t/send-email: add test with quoted sender","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:10Z","receivedAt":"2013-06-04T07:56:10Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"add test where sender address needs to be quoted.\nMake sure --suppress-cc=self works well in this case.\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t9001-send-email.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex f81e5f5..6c0f715 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -211,11 +211,31 @@ test_suppress_self_unquoted () {\n \tEOF\n }\n \n+test_suppress_self_quoted () {\n+\ttest_suppress_self \"$1\" \"$2\" \"quoted-$3\" <<-EOF\n+\t\ttest suppress-cc.self quoted-$3 with name $1 email $2\n+\n+\t\tquoted-$3\n+\n+\t\tcccmd--\"$1\" <$2>\n+\n+\t\tCc: $1 <$2>\n+\t\tCc: \"$1\" <$2>\n+\t\tSigned-off-by: $1 <$2>\n+\t\tSigned-off-by: \"$1\" <$2>\n+\tEOF\n+}\n+\n test_expect_success $PREREQ 'self name is suppressed' \"\n \ttest_suppress_self_unquoted 'A U Thor' 'author@example.com' \\\n \t\t'self_name_suppressed'\n \"\n \n+test_expect_success $PREREQ 'self name with dot is suppressed' \"\n+\ttest_suppress_self_quoted 'A U. Thor' 'author@example.com' \\\n+\t\t'self_name_dot_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"219337","messageId":"1370332482-12329-6-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 5/6] t/send-email: test suppress-cc=self with non-ascii","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:15Z","receivedAt":"2013-06-04T07:56:15Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"test suppress-cc=self when sender is non-acsii\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t9001-send-email.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 6c0f715..0d50fa7 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -236,6 +236,11 @@ test_expect_success $PREREQ 'self name with dot is suppressed' \"\n \t\t'self_name_dot_suppressed'\n \"\n \n+test_expect_success $PREREQ 'non-ascii self name is suppressed' \"\n+\ttest_suppress_self_quoted 'Füñný Nâmé' 'odd_?=mail@example.com' \\\n+\t\t'non_ascii_self_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"219336","messageId":"1370332482-12329-7-git-send-email-mst@redhat.com","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 6/6] test-send-email: test for pre-sanitized self name","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T07:56:19Z","receivedAt":"2013-06-04T07:56:19Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Users can sanitize from address manually.\nVerify that these are suppressed properly.\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t9001-send-email.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0d50fa7..38f407d 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -241,6 +241,11 @@ test_expect_success $PREREQ 'non-ascii self name is suppressed' \"\n \t\t'non_ascii_self_suppressed'\n \"\n \n+test_expect_success $PREREQ 'sanitized self name is suppressed' \"\n+\ttest_suppress_self_unquoted '\\\"A U. Thor\\\"' 'author@example.com' \\\n+\t\t'self_name_sanitized_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'Show all headers' '\n \tgit send-email \\\n \t\t--dry-run \\\n-- \nMST\n"},{"id":"219388","messageId":"7v4ndd93ks.fsf@alter.siamese.dyndns.org","threadId":"34021","inReplyTo":"1370332482-12329-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-04T17:40:51Z","receivedAt":"2013-06-04T17:40:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> This includes bugfixes related to handling of --suppress-cc=self\n> flag. Tests are also included.\n\nHmph, flipped the patches without test-applying first?\n\n2/6 adds two lines to test_suppress_self_quoted helper function, but\nthat is introduced only at 4/6.\n\n>\n> Changes from v2:\n> \t- add a new test, split patches differently add code comments\n> \t\t to address comments by Junio\n> \t- rename example addresses in tests from redhat.com to example.com\n> Changes from v1:\n> - tweak coding style in tests to address comments by Junio\n>\n>\n> Michael S. Tsirkin (6):\n> send-email: fix suppress-cc=self on cccmd\n> t/send-email: test suppress-cc=self on cccmd\n> send-email: make --suppress-cc=self sanitize input\n> t/send-email: add test with quoted sender\n> t/send-email: test suppress-cc=self with non-ascii\n> test-send-email: test for pre-sanitized self name\n>\n> git-send-email.perl | 23 +++++++++++++++--------\n> t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-\n> 2 files changed, 48 insertions(+), 9 deletions(-)\n"},{"id":"219417","messageId":"20130604214921.GA30400@redhat.com","threadId":"34021","inReplyTo":"7v4ndd93ks.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-04T21:49:22Z","receivedAt":"2013-06-04T21:49:22Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Tue, Jun 04, 2013 at 10:40:51AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > This includes bugfixes related to handling of --suppress-cc=self\n> > flag. Tests are also included.\n> \n> Hmph, flipped the patches without test-applying first?\n\nNo, I generated the patches with git format-patch,\nthen processed with git send-email.\n\n> 2/6 adds two lines to test_suppress_self_quoted helper function, but\n> that is introduced only at 4/6.\n\nI don't see it\nAll I see in 2/6 is this:\n\n\tdiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n\tindex e1a7f3e..f81e5f5 100755\n\t--- a/t/t9001-send-email.sh\n\t+++ b/t/t9001-send-email.sh\n\t@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {\n\n\t\t\tunquoted-$3\n\n\t+ cccmd--$1 <$2>\n\t+\n\t\t\tCc: $1 <$2>\n\t\t\tSigned-off-by: $1 <$2>\n\t\tEOF\n\t }\n\n\t test_expect_success $PREREQ 'self name is suppressed' \"\n\t- test_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \\\n\t+ test_suppress_self_unquoted 'A U Thor' 'author@example.com' \\\n\t\t\t'self_name_suppressed'\n\t \"\n\nwhere's test_suppress_self_quoted here?\n\n\n> >\n> > Changes from v2:\n> > \t- add a new test, split patches differently add code comments\n> > \t\t to address comments by Junio\n> > \t- rename example addresses in tests from redhat.com to example.com\n> > Changes from v1:\n> > - tweak coding style in tests to address comments by Junio\n> >\n> >\n> > Michael S. Tsirkin (6):\n> > send-email: fix suppress-cc=self on cccmd\n> > t/send-email: test suppress-cc=self on cccmd\n> > send-email: make --suppress-cc=self sanitize input\n> > t/send-email: add test with quoted sender\n> > t/send-email: test suppress-cc=self with non-ascii\n> > test-send-email: test for pre-sanitized self name\n> >\n> > git-send-email.perl | 23 +++++++++++++++--------\n> > t/t9001-send-email.sh | 34 +++++++++++++++++++++++++++++++++-\n> > 2 files changed, 48 insertions(+), 9 deletions(-)\n"},{"id":"219421","messageId":"7v4ndd5xwp.fsf@alter.siamese.dyndns.org","threadId":"34021","inReplyTo":"20130604214921.GA30400@redhat.com","subject":"Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-04T22:11:34Z","receivedAt":"2013-06-04T22:11:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> All I see in 2/6 is this:\n>\n> \tdiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> \tindex e1a7f3e..f81e5f5 100755\n> \t--- a/t/t9001-send-email.sh\n> \t+++ b/t/t9001-send-email.sh\n> \t@@ -204,13 +204,15 @@ test_suppress_self_unquoted () {\n\nAfter noticing what is after \"@@\" here...\n\n>\n> \t\t\tunquoted-$3\n>\n> \t+ cccmd--$1 <$2>\n> \t+\n> \t\t\tCc: $1 <$2>\n> \t\t\tSigned-off-by: $1 <$2>\n> \t\tEOF\n> \t }\n>\n> \t test_expect_success $PREREQ 'self name is suppressed' \"\n> \t- test_suppress_self_unquoted 'A U Thor' 'author@redhat.com' \\\n> \t+ test_suppress_self_unquoted 'A U Thor' 'author@example.com' \\\n> \t\t\t'self_name_suppressed'\n> \t \"\n>\n> where's test_suppress_self_quoted here?\n\n... isn't addition of \"cccmd--$1 <$2>\" made to that function?\n\nAfter applying [PATCH v3 1/6] to 'master', I do not see unquoted-$3\nthat we see in the context, either.\n\n\"git grep unquoted- pu t/t9001*\" does show us a hit, but that is\ncoming from your previous round you are replacing with this series,\nso....\n\nConfused...\n"}]}