Merge branch 'rl/send-email-aliases'
authorJunio C Hamano <gitster@pobox.com>
Mon, 3 Aug 2015 18:01:15 +0000 (11:01 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 3 Aug 2015 18:01:15 +0000 (11:01 -0700)
"git send-email" now performs alias-expansion on names that are
given via --cccmd, etc.

This round comes with a lot more enhanced e-mail address parser,
which makes it a bit scary, but as long as it works as designed, it
makes it wonderful ;-).

* rl/send-email-aliases:
send-email: suppress meaningless whitespaces in from field
send-email: allow multiple emails using --cc, --to and --bcc
send-email: consider quote as delimiter instead of character
send-email: reduce dependencies impact on parse_address_line
send-email: minor code refactoring
send-email: allow use of aliases in the From field of --compose mode
send-email: refactor address list process
t9001-send-email: refactor header variable fields replacement
send-email: allow aliases in patch header and command script outputs
t9001-send-email: move script creation in a setup test

Documentation/git-send-email.txt
git-send-email.perl
perl/Git.pm
t/t9000-addresses.sh [new file with mode: 0755]
t/t9000/test.pl [new file with mode: 0755]
t/t9001-send-email.sh
index 7ae467ba415e5cb4413d0246883b8a620b8960e3..f14705ee04e491e698e63ab10d15d9ec427b56f4 100644 (file)
@@ -49,17 +49,17 @@ Composing
        of 'sendemail.annotate'. See the CONFIGURATION section for
        'sendemail.multiEdit'.
 
---bcc=<address>::
+--bcc=<address>,...::
        Specify a "Bcc:" value for each email. Default is the value of
        'sendemail.bcc'.
 +
-The --bcc option must be repeated for each user you want on the bcc list.
+This option may be specified multiple times.
 
---cc=<address>::
+--cc=<address>,...::
        Specify a starting "Cc:" value for each email.
        Default is the value of 'sendemail.cc'.
 +
-The --cc option must be repeated for each user you want on the cc list.
+This option may be specified multiple times.
 
 --compose::
        Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -110,13 +110,13 @@ is not set, this will be prompted for.
        Only necessary if --compose is also set.  If --compose
        is not set, this will be prompted for.
 
---to=<address>::
+--to=<address>,...::
        Specify the primary recipient of the emails generated. Generally, this
        will be the upstream maintainer of the project involved. Default is the
        value of the 'sendemail.to' configuration value; if that is unspecified,
        and --to-cmd is not specified, this will be prompted for.
 +
-The --to option must be repeated for each user you want on the to list.
+This option may be specified multiple times.
 
 --8bit-encoding=<encoding>::
        When encountering a non-ASCII message or subject that does not
index ae9f8698c5a4842c2c0d63db51dde067ddeeb1c9..b660cc238223b101762017c7effda67de6f70113 100755 (executable)
@@ -460,25 +460,11 @@ sub read_config {
 ($repoauthor) = Git::ident_person(@repo, 'author');
 ($repocommitter) = Git::ident_person(@repo, 'committer');
 
-# Verify the user input
-
-foreach my $entry (@initial_to) {
-       die "Comma in --to entry: $entry'\n" unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
-       die "Comma in --cc entry: $entry'\n" unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
-       die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
-}
-
 sub parse_address_line {
        if ($have_mail_address) {
                return map { $_->format } Mail::Address->parse($_[0]);
        } else {
-               return split_addrs($_[0]);
+               return Git::parse_mailboxes($_[0]);
        }
 }
 
@@ -561,8 +547,6 @@ sub parse_sendmail_aliases {
        }
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
 # is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
 # $f is a revision list specification to be passed to format-patch.
 sub is_format_patch_arg {
@@ -807,7 +791,10 @@ sub file_declares_8bit_cte {
        }
 }
 
-if (!defined $sender) {
+if (defined $sender) {
+       $sender =~ s/^\s+|\s+$//g;
+       ($sender) = expand_aliases($sender);
+} else {
        $sender = $repoauthor || $repocommitter || '';
 }
 
@@ -839,12 +826,9 @@ sub expand_one_alias {
        return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
 }
 
-@initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
-@initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
-@bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@initial_to = process_address_list(@initial_to);
+@initial_cc = process_address_list(@initial_cc);
+@bcclist = process_address_list(@bcclist);
 
 if ($thread && !defined $initial_reply_to && $prompting) {
        $initial_reply_to = ask(
@@ -1037,15 +1021,17 @@ sub sanitize_address {
                return $recipient;
        }
 
+       # remove non-escaped quotes
+       $recipient_name =~ s/(^|[^\\])"/$1/g;
+
        # rfc2047 is needed if a non-ascii char is included
        if ($recipient_name =~ /[^[:ascii:]]/) {
-               $recipient_name =~ s/^"(.*)"$/$1/;
                $recipient_name = quote_rfc2047($recipient_name);
        }
 
        # double quotes are needed if specials or CTLs are included
        elsif ($recipient_name =~ /[][()<>@,;:\\".\000-\037\177]/) {
-               $recipient_name =~ s/(["\\\r])/\\$1/g;
+               $recipient_name =~ s/([\\\r])/\\$1/g;
                $recipient_name = qq["$recipient_name"];
        }
 
@@ -1057,6 +1043,14 @@ sub sanitize_address_list {
        return (map { sanitize_address($_) } @_);
 }
 
+sub process_address_list {
+       my @addr_list = map { parse_address_line($_) } @_;
+       @addr_list = expand_aliases(@addr_list);
+       @addr_list = sanitize_address_list(@addr_list);
+       @addr_list = validate_address_list(@addr_list);
+       return @addr_list;
+}
+
 # Returns the local Fully Qualified Domain Name (FQDN) if available.
 #
 # Tightly configured MTAa require that a caller sends a real DNS
@@ -1566,8 +1560,8 @@ sub send_message {
                ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
        $needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc);
 
-       @to = validate_address_list(sanitize_address_list(@to));
-       @cc = validate_address_list(sanitize_address_list(@cc));
+       @to = process_address_list(@to);
+       @cc = process_address_list(@cc);
 
        @to = (@initial_to, @to);
        @cc = (@initial_cc, @cc);
index 9026a7bb980a984086a62536f46ec3837588f277..19ef081103a56e3c723749dc172ccc356a9b795c 100644 (file)
@@ -864,6 +864,73 @@ sub ident_person {
        return "$ident[0] <$ident[1]>";
 }
 
+=item parse_mailboxes
+
+Return an array of mailboxes extracted from a string.
+
+=cut
+
+sub parse_mailboxes {
+       my $re_comment = qr/\((?:[^)]*)\)/;
+       my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;
+       my $re_word = qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
+
+       # divide the string in tokens of the above form
+       my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/;
+       my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_;
+
+       # add a delimiter to simplify treatment for the last mailbox
+       push @tokens, ",";
+
+       my (@addr_list, @phrase, @address, @comment, @buffer) = ();
+       foreach my $token (@tokens) {
+               if ($token =~ /^[,;]$/) {
+                       # if buffer still contains undeterminated strings
+                       # append it at the end of @address or @phrase
+                       if (@address) {
+                               push @address, @buffer;
+                       } else {
+                               push @phrase, @buffer;
+                       }
+
+                       my $str_phrase = join ' ', @phrase;
+                       my $str_address = join '', @address;
+                       my $str_comment = join ' ', @comment;
+
+                       # quote are necessary if phrase contains
+                       # special characters
+                       if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
+                               $str_phrase =~ s/(^|[^\\])"/$1/g;
+                               $str_phrase = qq["$str_phrase"];
+                       }
+
+                       # add "<>" around the address if necessary
+                       if ($str_address ne "" && $str_phrase ne "") {
+                               $str_address = qq[<$str_address>];
+                       }
+
+                       my $str_mailbox = "$str_phrase $str_address $str_comment";
+                       $str_mailbox =~ s/^\s*|\s*$//g;
+                       push @addr_list, $str_mailbox if ($str_mailbox);
+
+                       @phrase = @address = @comment = @buffer = ();
+               } elsif ($token =~ /^\(/) {
+                       push @comment, $token;
+               } elsif ($token eq "<") {
+                       push @phrase, (splice @address), (splice @buffer);
+               } elsif ($token eq ">") {
+                       push @address, (splice @buffer);
+               } elsif ($token eq "@") {
+                       push @address, (splice @buffer), "@";
+               } elsif ($token eq ".") {
+                       push @address, (splice @buffer), ".";
+               } else {
+                       push @buffer, $token;
+               }
+       }
+
+       return @addr_list;
+}
 
 =item hash_object ( TYPE, FILENAME )
 
diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
new file mode 100755 (executable)
index 0000000..a1ebef6
--- /dev/null
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+test_description='compare address parsing with and without Mail::Address'
+. ./test-lib.sh
+
+if ! test_have_prereq PERL; then
+       skip_all='skipping perl interface tests, perl not available'
+       test_done
+fi
+
+perl -MTest::More -e 0 2>/dev/null || {
+       skip_all="Perl Test::More unavailable, skipping test"
+       test_done
+}
+
+perl -MMail::Address -e 0 2>/dev/null || {
+       skip_all="Perl Mail::Address unavailable, skipping test"
+       test_done
+}
+
+test_external_has_tap=1
+
+test_external_without_stderr \
+       'Perl address parsing function' \
+       perl "$TEST_DIRECTORY"/t9000/test.pl
+
+test_done
diff --git a/t/t9000/test.pl b/t/t9000/test.pl
new file mode 100755 (executable)
index 0000000..2d05d3e
--- /dev/null
@@ -0,0 +1,67 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use 5.008;
+use warnings;
+use strict;
+
+use Test::More qw(no_plan);
+use Mail::Address;
+
+BEGIN { use_ok('Git') }
+
+my @success_list = (q[Jane],
+       q[jdoe@example.com],
+       q[<jdoe@example.com>],
+       q[Jane <jdoe@example.com>],
+       q[Jane Doe <jdoe@example.com>],
+       q["Jane" <jdoe@example.com>],
+       q["Doe, Jane" <jdoe@example.com>],
+       q["Jane@:;\>.,()<Doe" <jdoe@example.com>],
+       q[Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>],
+       q["<jdoe@example.com>"],
+       q["Jane jdoe@example.com"],
+       q[Jane Doe <jdoe    @   example.com  >],
+       q[Jane       Doe <  jdoe@example.com  >],
+       q[Jane @ Doe @ Jane @ Doe],
+       q["Jane, 'Doe'" <jdoe@example.com>],
+       q['Doe, "Jane' <jdoe@example.com>],
+       q["Jane" "Do"e <jdoe@example.com>],
+       q["Jane' Doe" <jdoe@example.com>],
+       q["Jane Doe <jdoe@example.com>" <jdoe@example.com>],
+       q["Jane\" Doe" <jdoe@example.com>],
+       q[Doe, jane <jdoe@example.com>],
+       q["Jane Doe <jdoe@example.com>],
+       q['Jane 'Doe' <jdoe@example.com>]);
+
+my @known_failure_list = (q[Jane\ Doe <jdoe@example.com>],
+       q["Doe, Ja"ne <jdoe@example.com>],
+       q["Doe, Katarina" Jane <jdoe@example.com>],
+       q[Jane@:;\.,()<>Doe <jdoe@example.com>],
+       q[Jane jdoe@example.com],
+       q[<jdoe@example.com> Jane Doe],
+       q[Jane <jdoe@example.com> Doe],
+       q["Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>],
+       q[Jane Doe],
+       q[Jane "Doe <jdoe@example.com>"],
+       q[\"Jane Doe <jdoe@example.com>],
+       q[Jane\"\" Doe <jdoe@example.com>],
+       q['Jane "Katarina\" \' Doe' <jdoe@example.com>]);
+
+foreach my $str (@success_list) {
+       my @expected = map { $_->format } Mail::Address->parse("$str");
+       my @actual = Git::parse_mailboxes("$str");
+       is_deeply(\@expected, \@actual, qq[same output : $str]);
+}
+
+TODO: {
+       local $TODO = "known breakage";
+       foreach my $str (@known_failure_list) {
+               my @expected = map { $_->format } Mail::Address->parse("$str");
+               my @actual = Git::parse_mailboxes("$str");
+               is_deeply(\@expected, \@actual, qq[same output : $str]);
+       }
+}
+
+my $is_passing = eval { Test::More->is_passing };
+exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;
index db2f45e83b14fb5ae5b5f32b4178d7cd1ce97bf8..5b4a5ce06b94355725fafc84c31f9ed67b8b15ed 100755 (executable)
@@ -312,13 +312,19 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
        )
 '
 
+test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+       write_script tocmd-sed <<-\EOF &&
+       sed -n -e "s/^tocmd--//p" "$1"
+       EOF
+       write_script cccmd-sed <<-\EOF
+       sed -n -e "s/^cccmd--//p" "$1"
+       EOF
+'
+
 test_expect_success $PREREQ 'tocmd works' '
        clean_fake_sendmail &&
        cp $patches tocmd.patch &&
        echo tocmd--tocmd@example.com >>tocmd.patch &&
-       write_script tocmd-sed <<-\EOF &&
-       sed -n -e "s/^tocmd--//p" "$1"
-       EOF
        git send-email \
                --from="Example <nobody@example.com>" \
                --to-cmd=./tocmd-sed \
@@ -332,9 +338,6 @@ test_expect_success $PREREQ 'cccmd works' '
        clean_fake_sendmail &&
        cp $patches cccmd.patch &&
        echo "cccmd--  cccmd@example.com" >>cccmd.patch &&
-       write_script cccmd-sed <<-\EOF &&
-       sed -n -e "s/^cccmd--//p" "$1"
-       EOF
        git send-email \
                --from="Example <nobody@example.com>" \
                --to=nobody@example.com \
@@ -519,6 +522,12 @@ Result: OK
 EOF
 "
 
+replace_variable_fields () {
+       sed     -e "s/^\(Date:\).*/\1 DATE-STRING/" \
+               -e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+               -e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/"
+}
+
 test_suppression () {
        git send-email \
                --dry-run \
@@ -526,10 +535,7 @@ test_suppression () {
                --from="Example <from@example.com>" \
                --to=to@example.com \
                --smtp-server relay.example.com \
-               $patches |
-       sed     -e "s/^\(Date:\).*/\1 DATE-STRING/" \
-               -e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
-               -e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+               $patches | replace_variable_fields \
                >actual-suppress-$1${2+"-$2"} &&
        test_cmp expected-suppress-$1${2+"-$2"} actual-suppress-$1${2+"-$2"}
 }
@@ -1621,6 +1627,66 @@ test_sendmail_aliases 'sendmail aliases tolerate bogus line folding' \
 test_sendmail_aliases 'sendmail aliases empty' alice bcgrp <<-\EOF
        EOF
 
+test_expect_success $PREREQ 'alias support in To header' '
+       clean_fake_sendmail &&
+       echo "alias sbd  someone@example.org" >.mailrc &&
+       test_config sendemail.aliasesfile ".mailrc" &&
+       test_config sendemail.aliasfiletype mailrc &&
+       git format-patch --stdout -1 --to=sbd >aliased.patch &&
+       git send-email \
+               --from="Example <nobody@example.com>" \
+               --smtp-server="$(pwd)/fake.sendmail" \
+               aliased.patch \
+               2>errors >out &&
+       grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'alias support in Cc header' '
+       clean_fake_sendmail &&
+       echo "alias sbd  someone@example.org" >.mailrc &&
+       test_config sendemail.aliasesfile ".mailrc" &&
+       test_config sendemail.aliasfiletype mailrc &&
+       git format-patch --stdout -1 --cc=sbd >aliased.patch &&
+       git send-email \
+               --from="Example <nobody@example.com>" \
+               --smtp-server="$(pwd)/fake.sendmail" \
+               aliased.patch \
+               2>errors >out &&
+       grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'tocmd works with aliases' '
+       clean_fake_sendmail &&
+       echo "alias sbd  someone@example.org" >.mailrc &&
+       test_config sendemail.aliasesfile ".mailrc" &&
+       test_config sendemail.aliasfiletype mailrc &&
+       git format-patch --stdout -1 >tocmd.patch &&
+       echo tocmd--sbd >>tocmd.patch &&
+       git send-email \
+               --from="Example <nobody@example.com>" \
+               --to-cmd=./tocmd-sed \
+               --smtp-server="$(pwd)/fake.sendmail" \
+               tocmd.patch \
+               2>errors >out &&
+       grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'cccmd works with aliases' '
+       clean_fake_sendmail &&
+       echo "alias sbd  someone@example.org" >.mailrc &&
+       test_config sendemail.aliasesfile ".mailrc" &&
+       test_config sendemail.aliasfiletype mailrc &&
+       git format-patch --stdout -1 >cccmd.patch &&
+       echo cccmd--sbd >>cccmd.patch &&
+       git send-email \
+               --from="Example <nobody@example.com>" \
+               --cc-cmd=./cccmd-sed \
+               --smtp-server="$(pwd)/fake.sendmail" \
+               cccmd.patch \
+               2>errors >out &&
+       grep "^!someone@example\.org!$" commandline1
+'
+
 do_xmailer_test () {
        expected=$1 params=$2 &&
        git format-patch -1 &&
@@ -1654,4 +1720,72 @@ test_expect_success $PREREQ '--[no-]xmailer with sendemail.xmailer=false' '
        do_xmailer_test 1 "--xmailer"
 '
 
+test_expect_success $PREREQ 'setup expected-list' '
+       git send-email \
+       --dry-run \
+       --from="Example <from@example.com>" \
+       --to="To 1 <to1@example.com>" \
+       --to="to2@example.com" \
+       --to="to3@example.com" \
+       --cc="Cc 1 <cc1@example.com>" \
+       --cc="Cc2 <cc2@example.com>" \
+       --bcc="bcc1@example.com" \
+       --bcc="bcc2@example.com" \
+       0001-add-master.patch | replace_variable_fields \
+       >expected-list
+'
+
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '
+       git send-email \
+       --dry-run \
+       --from="Example <from@example.com>" \
+       --to="To 1 <to1@example.com>, to2@example.com" \
+       --to="to3@example.com" \
+       --cc="Cc 1 <cc1@example.com>, Cc2 <cc2@example.com>" \
+       --bcc="bcc1@example.com, bcc2@example.com" \
+       0001-add-master.patch | replace_variable_fields \
+       >actual-list &&
+       test_cmp expected-list actual-list
+'
+
+test_expect_success $PREREQ 'aliases work with email list' '
+       echo "alias to2 to2@example.com" >.mutt &&
+       echo "alias cc1 Cc 1 <cc1@example.com>" >>.mutt &&
+       test_config sendemail.aliasesfile ".mutt" &&
+       test_config sendemail.aliasfiletype mutt &&
+       git send-email \
+       --dry-run \
+       --from="Example <from@example.com>" \
+       --to="To 1 <to1@example.com>, to2, to3@example.com" \
+       --cc="cc1, Cc2 <cc2@example.com>" \
+       --bcc="bcc1@example.com, bcc2@example.com" \
+       0001-add-master.patch | replace_variable_fields \
+       >actual-list &&
+       test_cmp expected-list actual-list
+'
+
+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
+       echo "alias to2 to2@example.com" >.mutt &&
+       echo "alias cc1 Cc 1 <cc1@example.com>" >>.mutt &&
+       test_config sendemail.aliasesfile ".mutt" &&
+       test_config sendemail.aliasfiletype mutt &&
+       TO1=$(echo "QTo 1 <to1@example.com>" | q_to_tab) &&
+       TO2=$(echo "QZto2" | qz_to_tab_space) &&
+       CC1=$(echo "cc1" | append_cr) &&
+       BCC1=$(echo "Q bcc1@example.com Q" | q_to_nul) &&
+       git send-email \
+       --dry-run \
+       --from="        Example <from@example.com>" \
+       --to="$TO1" \
+       --to="$TO2" \
+       --to="  to3@example.com   " \
+       --cc="$CC1" \
+       --cc="Cc2 <cc2@example.com>" \
+       --bcc="$BCC1" \
+       --bcc="bcc2@example.com" \
+       0001-add-master.patch | replace_variable_fields \
+       >actual-list &&
+       test_cmp expected-list actual-list
+'
+
 test_done