Skip to content

Commit 5012699

Browse files
jaysoffiangitster
jaysoffian
authored and
gitster
committed
send-email: handle multiple Cc addresses when reading mbox message
When git format-patch is given multiple --cc arguments, it generates a Cc header that looks like: Cc: first@example.com, second@example.com, third@example.com Before this commit, send-email was unable to handle such a message as it did not handle folded header lines, nor multiple recipients in a Cc line. This patch: - Unfolds header lines by pre-processing the header before extracting any of its fields. - Handles Cc lines with multiple recipients. - Adds use of Mail::Address if available for splitting Cc line and the "Who should the emails be sent to?" prompt", with fall back to existing split_addrs() function. - Tests the new functionality and adds two tests for detecting whether "From:" appears correctly in message body when patch author differs from patch sender. Signed-off-by: Jay Soffian <jaysoffian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent eed6ca7 commit 5012699

File tree

2 files changed

+123
-74
lines changed

2 files changed

+123
-74
lines changed

‎git-send-email.perl‎

Lines changed: 86 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ sub format_2822_time {
126126
}
127127

128128
my $have_email_valid = eval { require Email::Valid; 1 };
129+
my $have_mail_address = eval { require Mail::Address; 1 };
129130
my $smtp;
130131
my $auth;
131132

@@ -366,6 +367,14 @@ sub read_config {
366367
die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
367368
}
368369

370+
sub parse_address_line {
371+
if ($have_mail_address) {
372+
return map { $_->format } Mail::Address->parse($_[0]);
373+
} else {
374+
return split_addrs($_[0]);
375+
}
376+
}
377+
369378
sub split_addrs {
370379
return quotewords('\s*,\s*', 1, @_);
371380
}
@@ -602,7 +611,7 @@ ($)
602611
}
603612

604613
my $to = $_;
605-
push @to, split_addrs($to);
614+
push @to, parse_address_line($to);
606615
$prompting++;
607616
}
608617

@@ -929,88 +938,98 @@ sub send_message
929938
@cc = @initial_cc;
930939
@xh = ();
931940
my $input_format = undef;
932-
my $header_done = 0;
941+
my @header = ();
933942
$message = "";
943+
# First unfold multiline header fields
934944
while(<F>) {
935-
if (!$header_done) {
936-
if (/^From /) {
937-
$input_format = 'mbox';
938-
next;
945+
last if /^\s*$/;
946+
if (/^\s+\S/ and @header) {
947+
chomp($header[$#header]);
948+
s/^\s+/ /;
949+
$header[$#header] .= $_;
950+
} else {
951+
push(@header, $_);
952+
}
953+
}
954+
# Now parse the header
955+
foreach(@header) {
956+
if (/^From /) {
957+
$input_format = 'mbox';
958+
next;
959+
}
960+
chomp;
961+
if (!defined $input_format && /^[-A-Za-z]+:\s/) {
962+
$input_format = 'mbox';
963+
}
964+
965+
if (defined $input_format && $input_format eq 'mbox') {
966+
if (/^Subject:\s+(.*)$/) {
967+
$subject = $1;
939968
}
940-
chomp;
941-
if (!defined $input_format && /^[-A-Za-z]+:\s/) {
942-
$input_format = 'mbox';
969+
elsif (/^From:\s+(.*)$/) {
970+
($author, $author_encoding) = unquote_rfc2047($1);
971+
next if $suppress_cc{'author'};
972+
next if $suppress_cc{'self'} and $author eq $sender;
973+
printf("(mbox) Adding cc: %s from line '%s'\n",
974+
$1, $_) unless $quiet;
975+
push @cc, $1;
943976
}
944-
945-
if (defined $input_format && $input_format eq 'mbox') {
946-
if (/^Subject:\s+(.*)$/) {
947-
$subject = $1;
948-
949-
} elsif (/^(Cc|From):\s+(.*)$/) {
950-
if (unquote_rfc2047($2) eq $sender) {
977+
elsif (/^Cc:\s+(.*)$/) {
978+
foreach my $addr (parse_address_line($1)) {
979+
if (unquote_rfc2047($addr) eq $sender) {
951980
next if ($suppress_cc{'self'});
952-
}
953-
elsif ($1 eq 'From') {
954-
($author, $author_encoding)
955-
= unquote_rfc2047($2);
956-
next if ($suppress_cc{'author'});
957981
} else {
958982
next if ($suppress_cc{'cc'});
959983
}
960984
printf("(mbox) Adding cc: %s from line '%s'\n",
961-
$2, $_) unless $quiet;
962-
push @cc, $2;
985+
$addr, $_) unless $quiet;
986+
push @cc, $addr;
963987
}
964-
elsif (/^Content-type:/i) {
965-
$has_content_type = 1;
966-
if (/charset="?([^ "]+)/) {
967-
$body_encoding = $1;
968-
}
969-
push @xh, $_;
970-
}
971-
elsif (/^Message-Id: (.*)/i) {
972-
$message_id = $1;
973-
}
974-
elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
975-
push @xh, $_;
976-
}
977-
978-
} else {
979-
# In the traditional
980-
# "send lots of email" format,
981-
# line 1 = cc
982-
# line 2 = subject
983-
# So let's support that, too.
984-
$input_format = 'lots';
985-
if (@cc == 0 && !$suppress_cc{'cc'}) {
986-
printf("(non-mbox) Adding cc: %s from line '%s'\n",
987-
$_, $_) unless $quiet;
988-
989-
push @cc, $_;
990-
991-
} elsif (!defined $subject) {
992-
$subject = $_;
988+
}
989+
elsif (/^Content-type:/i) {
990+
$has_content_type = 1;
991+
if (/charset="?([^ "]+)/) {
992+
$body_encoding = $1;
993993
}
994+
push @xh, $_;
994995
}
995-
996-
# A whitespace line will terminate the headers
997-
if (m/^\s*$/) {
998-
$header_done = 1;
996+
elsif (/^Message-Id: (.*)/i) {
997+
$message_id = $1;
999998
}
999+
elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
1000+
push @xh, $_;
1001+
}
1002+
10001003
} else {
1001-
$message .= $_;
1002-
if (/^(Signed-off-by|Cc): (.*)$/i) {
1003-
next if ($suppress_cc{'sob'});
1004-
chomp;
1005-
my $c = $2;
1006-
chomp $c;
1007-
next if ($c eq $sender and $suppress_cc{'self'});
1008-
push @cc, $c;
1009-
printf("(sob) Adding cc: %s from line '%s'\n",
1010-
$c, $_) unless $quiet;
1004+
# In the traditional
1005+
# "send lots of email" format,
1006+
# line 1 = cc
1007+
# line 2 = subject
1008+
# So let's support that, too.
1009+
$input_format = 'lots';
1010+
if (@cc == 0 && !$suppress_cc{'cc'}) {
1011+
printf("(non-mbox) Adding cc: %s from line '%s'\n",
1012+
$_, $_) unless $quiet;
1013+
push @cc, $_;
1014+
} elsif (!defined $subject) {
1015+
$subject = $_;
10111016
}
10121017
}
10131018
}
1019+
# Now parse the message body
1020+
while(<F>) {
1021+
$message .= $_;
1022+
if (/^(Signed-off-by|Cc): (.*)$/i) {
1023+
next if ($suppress_cc{'sob'});
1024+
chomp;
1025+
my $c = $2;
1026+
chomp $c;
1027+
next if ($c eq $sender and $suppress_cc{'self'});
1028+
push @cc, $c;
1029+
printf("(sob) Adding cc: %s from line '%s'\n",
1030+
$c, $_) unless $quiet;
1031+
}
1032+
}
10141033
close F;
10151034

10161035
if (defined $cc_cmd && !$suppress_cc{'cccmd'}) {
@@ -1029,7 +1048,7 @@ sub send_message
10291048
or die "(cc-cmd) failed to close pipe to '$cc_cmd'";
10301049
}
10311050

1032-
if (defined $author) {
1051+
if (defined $author and $author ne $sender) {
10331052
$message = "From: $author\n\n$message";
10341053
if (defined $author_encoding) {
10351054
if ($has_content_type) {

‎t/t9001-send-email.sh‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ clean_fake_sendmail() {
3232
}
3333

3434
test_expect_success 'Extract patches' '
35-
patches=`git format-patch -n HEAD^1`
35+
patches=`git format-patch --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
3636
'
3737

3838
test_expect_success 'Send patches' '
@@ -42,6 +42,8 @@ test_expect_success 'Send patches' '
4242
cat >expected <<\EOF
4343
!nobody@example.com!
4444
!author@example.com!
45+
!one@example.com!
46+
!two@example.com!
4547
EOF
4648
test_expect_success \
4749
'Verify commandline' \
@@ -50,13 +52,15 @@ test_expect_success \
5052
cat >expected-show-all-headers <<\EOF
5153
0001-Second.patch
5254
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
55+
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
56+
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
5357
Dry-OK. Log says:
5458
Server: relay.example.com
5559
MAIL FROM:<from@example.com>
56-
RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<bcc@example.com>
60+
RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<bcc@example.com>
5761
From: Example <from@example.com>
5862
To: to@example.com
59-
Cc: cc@example.com, A <author@example.com>
63+
Cc: cc@example.com, A <author@example.com>, One <one@example.com>, two@example.com
6064
Subject: [PATCH 1/1] Second.
6165
Date: DATE-STRING
6266
Message-Id: MESSAGE-ID-STRING
@@ -104,6 +108,28 @@ test_expect_success 'no patch was sent' '
104108
! test -e commandline1
105109
'
106110

111+
test_expect_success 'Author From: in message body' '
112+
clean_fake_sendmail &&
113+
git send-email \
114+
--from="Example <nobody@example.com>" \
115+
--to=nobody@example.com \
116+
--smtp-server="$(pwd)/fake.sendmail" \
117+
$patches &&
118+
sed "1,/^$/d" < msgtxt1 > msgbody1
119+
grep "From: A <author@example.com>" msgbody1
120+
'
121+
122+
test_expect_success 'Author From: not in message body' '
123+
clean_fake_sendmail &&
124+
git send-email \
125+
--from="A <author@example.com>" \
126+
--to=nobody@example.com \
127+
--smtp-server="$(pwd)/fake.sendmail" \
128+
$patches &&
129+
sed "1,/^$/d" < msgtxt1 > msgbody1
130+
! grep "From: A <author@example.com>" msgbody1
131+
'
132+
107133
test_expect_success 'allow long lines with --no-validate' '
108134
git send-email \
109135
--from="Example <nobody@example.com>" \
@@ -170,13 +196,15 @@ test_expect_success 'second message is patch' '
170196
cat >expected-show-all-headers <<\EOF
171197
0001-Second.patch
172198
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
199+
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
200+
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
173201
Dry-OK. Log says:
174202
Server: relay.example.com
175203
MAIL FROM:<from@example.com>
176-
RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>
204+
RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
177205
From: Example <from@example.com>
178206
To: to@example.com
179-
Cc: cc@example.com, A <author@example.com>
207+
Cc: cc@example.com, A <author@example.com>, One <one@example.com>, two@example.com
180208
Subject: [PATCH 1/1] Second.
181209
Date: DATE-STRING
182210
Message-Id: MESSAGE-ID-STRING
@@ -203,13 +231,15 @@ test_expect_success 'sendemail.cc set' '
203231
cat >expected-show-all-headers <<\EOF
204232
0001-Second.patch
205233
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
234+
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
235+
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
206236
Dry-OK. Log says:
207237
Server: relay.example.com
208238
MAIL FROM:<from@example.com>
209-
RCPT TO:<to@example.com>,<author@example.com>
239+
RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
210240
From: Example <from@example.com>
211241
To: to@example.com
212-
Cc: A <author@example.com>
242+
Cc: A <author@example.com>, One <one@example.com>, two@example.com
213243
Subject: [PATCH 1/1] Second.
214244
Date: DATE-STRING
215245
Message-Id: MESSAGE-ID-STRING

0 commit comments

Comments
 (0)