Merge branch 'mm/send-email-cc-cruft-after-address' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 29 Nov 2016 21:27:54 +0000 (13:27 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 29 Nov 2016 21:27:55 +0000 (13:27 -0800)
"git send-email" attempts to pick up valid e-mails from the
trailers, but people in real world write non-addresses there, like
"Cc: Stable <add@re.ss> # 4.8+", which broke the output depending
on the availability and vintage of Mail::Address perl module.

* mm/send-email-cc-cruft-after-address:
Git.pm: add comment pointing to t9000
t9000-addresses: update expected results after fix
parse_mailboxes: accept extra text after <...> address

1  2 
perl/Git.pm
t/t9001-send-email.sh
diff --combined perl/Git.pm
index ce7e4e8da3947bb2c527c49d6d09e1e49b0392c3,8bb2b7c7e355d152ebfeac9c1f8fddf846afebcd..864123fe8e61f7469b58fef06a04edeb561edd53
@@@ -188,8 -188,7 +188,8 @@@ sub repository 
                };
  
                if ($dir) {
 -                      $dir =~ m#^/# or $dir = $opts{Directory} . '/' . $dir;
 +                      _verify_require();
 +                      File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
                        $opts{Repository} = abs_path($dir);
  
                        # If --git-dir went ok, this shouldn't die either.
@@@ -393,7 -392,7 +393,7 @@@ sub command_close_pipe 
  Execute the given C<COMMAND> in the same way as command_output_pipe()
  does but return both an input pipe filehandle and an output pipe filehandle.
  
 -The function will return return C<($pid, $pipe_in, $pipe_out, $ctx)>.
 +The function will return C<($pid, $pipe_in, $pipe_out, $ctx)>.
  See C<command_close_bidi_pipe()> for details.
  
  =cut
@@@ -871,6 -870,8 +871,8 @@@ Return an array of mailboxes extracted 
  
  =cut
  
+ # Very close to Mail::Address's parser, but we still have minor
+ # differences in some cases (see t9000 for examples).
  sub parse_mailboxes {
        my $re_comment = qr/\((?:[^)]*)\)/;
        my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;
        # 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 } @_;
+       my $end_of_addr_seen = 0;
  
        # add a delimiter to simplify treatment for the last mailbox
        push @tokens, ",";
                if ($token =~ /^[,;]$/) {
                        # if buffer still contains undeterminated strings
                        # append it at the end of @address or @phrase
-                       if (@address) {
-                               push @address, @buffer;
-                       } else {
+                       if ($end_of_addr_seen) {
                                push @phrase, @buffer;
+                       } else {
+                               push @address, @buffer;
                        }
  
                        my $str_phrase = join ' ', @phrase;
                        push @addr_list, $str_mailbox if ($str_mailbox);
  
                        @phrase = @address = @comment = @buffer = ();
+                       $end_of_addr_seen = 0;
                } elsif ($token =~ /^\(/) {
                        push @comment, $token;
                } elsif ($token eq "<") {
                        push @phrase, (splice @address), (splice @buffer);
                } elsif ($token eq ">") {
+                       $end_of_addr_seen = 1;
                        push @address, (splice @buffer);
-               } elsif ($token eq "@") {
+               } elsif ($token eq "@" && !$end_of_addr_seen) {
                        push @address, (splice @buffer), "@";
-               } elsif ($token eq ".") {
-                       push @address, (splice @buffer), ".";
                } else {
                        push @buffer, $token;
                }
diff --combined t/t9001-send-email.sh
index b3355d2c7016e55e99a465414f230569c3807230,37c71cd60fc729adf524ec05cc0fdb0eae38f7ee..3dc4a3454d223d37e85de6fa4ae8656046370e99
@@@ -36,7 -36,7 +36,7 @@@ clean_fake_sendmail () 
  }
  
  test_expect_success $PREREQ 'Extract patches' '
 -      patches=`git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1`
 +      patches=$(git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1)
  '
  
  # Test no confirm early to ensure remaining tests will not hang
@@@ -140,6 -140,35 +140,35 @@@ test_expect_success $PREREQ 'Verify com
        test_cmp expected commandline1
  '
  
+ test_expect_success $PREREQ 'setup expect for cc trailer' "
+ cat >expected-cc <<\EOF
+ !recipient@example.com!
+ !author@example.com!
+ !one@example.com!
+ !two@example.com!
+ !three@example.com!
+ !four@example.com!
+ !five@example.com!
+ EOF
+ "
+ test_expect_success $PREREQ 'cc trailer with various syntax' '
+       test_commit cc-trailer &&
+       test_when_finished "git reset --hard HEAD^" &&
+       git commit --amend -F - <<-EOF &&
+       Test Cc: trailers.
+       Cc: one@example.com
+       Cc: <two@example.com> # this is part of the name
+       Cc: <three@example.com>, <four@example.com> # not.five@example.com
+       Cc: "Some # Body" <five@example.com> [part.of.name.too]
+       EOF
+       clean_fake_sendmail &&
+       git send-email -1 --to=recipient@example.com \
+               --smtp-server="$(pwd)/fake.sendmail" &&
+       test_cmp expected-cc commandline1
+ '
  test_expect_success $PREREQ 'setup expect' "
  cat >expected-show-all-headers <<\EOF
  0001-Second.patch
@@@ -1151,7 -1180,7 +1180,7 @@@ test_expect_success $PREREQ '--no-bcc o
  '
  
  test_expect_success $PREREQ 'patches To headers are used by default' '
 -      patch=`git format-patch -1 --to="bodies@example.com"` &&
 +      patch=$(git format-patch -1 --to="bodies@example.com") &&
        test_when_finished "rm $patch" &&
        git send-email \
                --dry-run \
  '
  
  test_expect_success $PREREQ 'patches To headers are appended to' '
 -      patch=`git format-patch -1 --to="bodies@example.com"` &&
 +      patch=$(git format-patch -1 --to="bodies@example.com") &&
        test_when_finished "rm $patch" &&
        git send-email \
                --dry-run \
  '
  
  test_expect_success $PREREQ 'To headers from files reset each patch' '
 -      patch1=`git format-patch -1 --to="bodies@example.com"` &&
 -      patch2=`git format-patch -1 --to="other@example.com" HEAD~` &&
 +      patch1=$(git format-patch -1 --to="bodies@example.com") &&
 +      patch2=$(git format-patch -1 --to="other@example.com" HEAD~) &&
        test_when_finished "rm $patch1 && rm $patch2" &&
        git send-email \
                --dry-run \
@@@ -1488,7 -1517,7 +1517,7 @@@ test_cover_addresses () 
        clean_fake_sendmail &&
        rm -fr outdir &&
        git format-patch --cover-letter -2 -o outdir &&
 -      cover=`echo outdir/0000-*.patch` &&
 +      cover=$(echo outdir/0000-*.patch) &&
        mv $cover cover-to-edit.patch &&
        perl -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch >"$cover" &&
        git send-email \
@@@ -1527,21 -1556,6 +1556,21 @@@ test_expect_success $PREREQ 'cccover ad
        test_cover_addresses "Cc"
  '
  
 +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' '
 +      clean_fake_sendmail &&
 +      echo "alias sbd \\\"Dot U. Sir\\\" <somebody@example.org>" >.mutt &&
 +      git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" &&
 +      git config sendemail.aliasfiletype mutt &&
 +      git send-email \
 +              --from="Example <nobody@example.com>" \
 +              --to=sbd \
 +              --smtp-server="$(pwd)/fake.sendmail" \
 +              outdir/0001-*.patch \
 +              2>errors >out &&
 +      grep "^!somebody@example\.org!$" commandline1 &&
 +      grep -F "To: \"Dot U. Sir\" <somebody@example.org>" out
 +'
 +
  test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
        clean_fake_sendmail &&
        echo "alias sbd  somebody@example.org" >.mailrc &&
  
  test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
        clean_fake_sendmail &&
 -      echo "alias sbd  someone@example.org" >~/.mailrc &&
 +      echo "alias sbd  someone@example.org" >"$HOME/.mailrc" &&
        git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
        git config sendemail.aliasfiletype mailrc &&
        git send-email \
        grep "^!someone@example\.org!$" commandline1
  '
  
 +test_dump_aliases () {
 +      msg="$1" && shift &&
 +      filetype="$1" && shift &&
 +      printf '%s\n' "$@" >expect &&
 +      cat >.tmp-email-aliases &&
 +
 +      test_expect_success $PREREQ "$msg" '
 +              clean_fake_sendmail && rm -fr outdir &&
 +              git config --replace-all sendemail.aliasesfile \
 +                      "$(pwd)/.tmp-email-aliases" &&
 +              git config sendemail.aliasfiletype "$filetype" &&
 +              git send-email --dump-aliases 2>errors >actual &&
 +              test_cmp expect actual
 +      '
 +}
 +
 +test_dump_aliases '--dump-aliases sendmail format' \
 +      'sendmail' \
 +      'abgroup' \
 +      'alice' \
 +      'bcgrp' \
 +      'bob' \
 +      'chloe' <<-\EOF
 +      alice: Alice W Land <awol@example.com>
 +      bob: Robert Bobbyton <bob@example.com>
 +      chloe: chloe@example.com
 +      abgroup: alice, bob
 +      bcgrp: bob, chloe, Other <o@example.com>
 +      EOF
 +
 +test_dump_aliases '--dump-aliases mutt format' \
 +      'mutt' \
 +      'alice' \
 +      'bob' \
 +      'chloe' \
 +      'donald' <<-\EOF
 +      alias alice Alice W Land <awol@example.com>
 +      alias donald Donald C Carlton <donc@example.com>
 +      alias bob Robert Bobbyton <bob@example.com>
 +      alias chloe chloe@example.com
 +      EOF
 +
 +test_dump_aliases '--dump-aliases mailrc format' \
 +      'mailrc' \
 +      'alice' \
 +      'bob' \
 +      'chloe' \
 +      'eve' <<-\EOF
 +      alias alice   Alice W Land <awol@example.com>
 +      alias eve     Eve <eve@example.com>
 +      alias bob     Robert Bobbyton <bob@example.com>
 +      alias chloe   chloe@example.com
 +      EOF
 +
 +test_dump_aliases '--dump-aliases pine format' \
 +      'pine' \
 +      'alice' \
 +      'bob' \
 +      'chloe' \
 +      'eve' <<-\EOF
 +      alice   Alice W Land    <awol@example.com>
 +      eve     Eve     <eve@example.com>
 +      bob     Robert  Bobbyton <bob@example.com>
 +      chloe           chloe@example.com
 +      EOF
 +
 +test_dump_aliases '--dump-aliases gnus format' \
 +      'gnus' \
 +      'alice' \
 +      'bob' \
 +      'chloe' \
 +      'eve' <<-\EOF
 +      (define-mail-alias "alice" "awol@example.com")
 +      (define-mail-alias "eve" "eve@example.com")
 +      (define-mail-alias "bob" "bob@example.com")
 +      (define-mail-alias "chloe" "chloe@example.com")
 +      EOF
 +
 +test_expect_success '--dump-aliases must be used alone' '
 +      test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting
 +'
 +
 +test_sendmail_aliases () {
 +      msg="$1" && shift &&
 +      expect="$@" &&
 +      cat >.tmp-email-aliases &&
 +
 +      test_expect_success $PREREQ "$msg" '
 +              clean_fake_sendmail && rm -fr outdir &&
 +              git format-patch -1 -o outdir &&
 +              git config --replace-all sendemail.aliasesfile \
 +                      "$(pwd)/.tmp-email-aliases" &&
 +              git config sendemail.aliasfiletype sendmail &&
 +              git send-email \
 +                      --from="Example <nobody@example.com>" \
 +                      --to=alice --to=bcgrp \
 +                      --smtp-server="$(pwd)/fake.sendmail" \
 +                      outdir/0001-*.patch \
 +                      2>errors >out &&
 +              for i in $expect
 +              do
 +                      grep "^!$i!$" commandline1 || return 1
 +              done
 +      '
 +}
 +
 +test_sendmail_aliases 'sendemail.aliasfiletype=sendmail' \
 +      'awol@example\.com' \
 +      'bob@example\.com' \
 +      'chloe@example\.com' \
 +      'o@example\.com' <<-\EOF
 +      alice: Alice W Land <awol@example.com>
 +      bob: Robert Bobbyton <bob@example.com>
 +      # this is a comment
 +         # this is also a comment
 +      chloe: chloe@example.com
 +      abgroup: alice, bob
 +      bcgrp: bob, chloe, Other <o@example.com>
 +      EOF
 +
 +test_sendmail_aliases 'sendmail aliases line folding' \
 +      alice1 \
 +      bob1 bob2 \
 +      chuck1 chuck2 \
 +      darla1 darla2 darla3 \
 +      elton1 elton2 elton3 \
 +      fred1 fred2 \
 +      greg1 <<-\EOF
 +      alice: alice1
 +      bob: bob1,\
 +      bob2
 +      chuck: chuck1,
 +          chuck2
 +      darla: darla1,\
 +      darla2,
 +          darla3
 +      elton: elton1,
 +          elton2,\
 +      elton3
 +      fred: fred1,\
 +          fred2
 +      greg: greg1
 +      bcgrp: bob, chuck, darla, elton, fred, greg
 +      EOF
 +
 +test_sendmail_aliases 'sendmail aliases tolerate bogus line folding' \
 +      alice1 bob1 <<-\EOF
 +          alice: alice1
 +      bcgrp: bob1\
 +      EOF
 +
 +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 &&