* [PATCH v3 1/6] send-email: fix suppress-cc=self on cccmd
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 2/6] t/send-email: test " Michael S. Tsirkin
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/6] t/send-email: test suppress-cc=self on cccmd
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 1/6] send-email: fix suppress-cc=self on cccmd Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 3/6] send-email: make --suppress-cc=self sanitize input Michael S. Tsirkin
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/6] send-email: make --suppress-cc=self sanitize input
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 1/6] send-email: fix suppress-cc=self on cccmd Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 2/6] t/send-email: test " Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 4/6] t/send-email: add test with quoted sender Michael S. Tsirkin
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
--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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/6] t/send-email: add test with quoted sender
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
` (2 preceding siblings ...)
2013-06-04 7:56 ` [PATCH v3 3/6] send-email: make --suppress-cc=self sanitize input Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 5/6] t/send-email: test suppress-cc=self with non-ascii Michael S. Tsirkin
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 5/6] t/send-email: test suppress-cc=self with non-ascii
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
` (3 preceding siblings ...)
2013-06-04 7:56 ` [PATCH v3 4/6] t/send-email: add test with quoted sender Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 7:56 ` [PATCH v3 6/6] test-send-email: test for pre-sanitized self name Michael S. Tsirkin
2013-06-04 17:40 ` [PATCH v3 0/6] git send-email suppress-cc=self fixes Junio C Hamano
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 6/6] test-send-email: test for pre-sanitized self name
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
` (4 preceding siblings ...)
2013-06-04 7:56 ` [PATCH v3 5/6] t/send-email: test suppress-cc=self with non-ascii Michael S. Tsirkin
@ 2013-06-04 7:56 ` Michael S. Tsirkin
2013-06-04 17:40 ` [PATCH v3 0/6] git send-email suppress-cc=self fixes Junio C Hamano
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 7:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes
2013-06-04 7:55 [PATCH v3 0/6] git send-email suppress-cc=self fixes Michael S. Tsirkin
` (5 preceding siblings ...)
2013-06-04 7:56 ` [PATCH v3 6/6] test-send-email: test for pre-sanitized self name Michael S. Tsirkin
@ 2013-06-04 17:40 ` Junio C Hamano
2013-06-04 21:49 ` Michael S. Tsirkin
6 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2013-06-04 17:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: git
"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(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes
2013-06-04 17:40 ` [PATCH v3 0/6] git send-email suppress-cc=self fixes Junio C Hamano
@ 2013-06-04 21:49 ` Michael S. Tsirkin
2013-06-04 22:11 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2013-06-04 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/6] git send-email suppress-cc=self fixes
2013-06-04 21:49 ` Michael S. Tsirkin
@ 2013-06-04 22:11 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2013-06-04 22:11 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: git
"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...
^ permalink raw reply [flat|nested] 10+ messages in thread