Merge branch 'mt/send-email-cc-match-fix' into maint
authorJunio C Hamano <gitster@pobox.com>
Mon, 15 Jul 2013 17:34:36 +0000 (10:34 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 15 Jul 2013 17:34:36 +0000 (10:34 -0700)
Logic used by git-send-email to suppress cc mishandled names like "A
U. Thor" <author@example.xz>, where the human readable part needs to
be quoted (the user input may not have the double quotes around the
name, and comparison was done between quoted and unquoted strings).
It also mishandled names that need RFC2047 quoting.

* mt/send-email-cc-match-fix:
send-email: sanitize author when writing From line
send-email: add test for duplicate utf8 name
test-send-email: test for pre-sanitized self name
t/send-email: test suppress-cc=self with non-ascii
t/send-email: add test with quoted sender
send-email: make --suppress-cc=self sanitize input
t/send-email: test suppress-cc=self on cccmd
send-email: fix suppress-cc=self on cccmd
t/send-email.sh: add test for suppress-cc=self

1  2 
git-send-email.perl
diff --combined git-send-email.perl
index bd13cc812d2a0115edcdd2c3ec146665ddfc5e29,548c75d69aa9214e336c9a916a810c46a6a1f678..cd93490c871be5200b356e6392707be0ff34e9e9
@@@ -54,7 -54,7 +54,7 @@@ git send-email [options] <file | direct
      --[no-]bcc              <str>  * Email Bcc:
      --subject               <str>  * Email "Subject:"
      --in-reply-to           <str>  * Email "In-Reply-To:"
 -    --annotate                     * Review each patch that will be sent in an editor.
 +    --[no-]annotate                * Review each patch that will be sent in an editor.
      --compose                      * Open an editor for introduction.
      --compose-encoding      <str>  * Encoding to assume for introduction.
      --8bit-encoding         <str>  * Encoding to assume 8bit mails if undeclared
@@@ -212,8 -212,7 +212,8 @@@ my %config_bool_settings = 
      "signedoffbycc" => [\$signed_off_by_cc, undef],
      "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
      "validate" => [\$validate, 1],
 -    "multiedit" => [\$multiedit, undef]
 +    "multiedit" => [\$multiedit, undef],
 +    "annotate" => [\$annotate, undef]
  );
  
  my %config_settings = (
@@@ -305,7 -304,7 +305,7 @@@ my $rc = GetOptions("h" => \$help
                    "smtp-debug:i" => \$debug_net_smtp,
                    "smtp-domain:s" => \$smtp_domain,
                    "identity=s" => \$identity,
 -                  "annotate" => \$annotate,
 +                  "annotate!" => \$annotate,
                    "compose" => \$compose,
                    "quiet" => \$quiet,
                    "cc-cmd=s" => \$cc_cmd,
@@@ -760,6 -759,11 +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)? ",
@@@ -1048,47 -1052,6 +1053,47 @@@ sub maildomain 
        return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
  }
  
 +sub smtp_host_string {
 +      if (defined $smtp_server_port) {
 +              return "$smtp_server:$smtp_server_port";
 +      } else {
 +              return $smtp_server;
 +      }
 +}
 +
 +# Returns 1 if authentication succeeded or was not necessary
 +# (smtp_user was not specified), and 0 otherwise.
 +
 +sub smtp_auth_maybe {
 +      if (!defined $smtp_authuser || $auth) {
 +              return 1;
 +      }
 +
 +      # Workaround AUTH PLAIN/LOGIN interaction defect
 +      # with Authen::SASL::Cyrus
 +      eval {
 +              require Authen::SASL;
 +              Authen::SASL->import(qw(Perl));
 +      };
 +
 +      # TODO: Authentication may fail not because credentials were
 +      # invalid but due to other reasons, in which we should not
 +      # reject credentials.
 +      $auth = Git::credential({
 +              'protocol' => 'smtp',
 +              'host' => smtp_host_string(),
 +              'username' => $smtp_authuser,
 +              # if there's no password, "git credential fill" will
 +              # give us one, otherwise it'll just pass this one.
 +              'password' => $smtp_authpass
 +      }, sub {
 +              my $cred = shift;
 +              return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
 +      });
 +
 +      return $auth;
 +}
 +
  # Returns 1 if the message was sent, and 0 otherwise.
  # In actuality, the whole program dies when there
  # is an error sending a message.
@@@ -1113,10 -1076,9 +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 -1095,7 +1137,7 @@@ X-Mailer: git-send-email $gitversio
        }
  
        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;
        }
                else {
                        require Net::SMTP;
                        $smtp_domain ||= maildomain();
 -                      $smtp ||= Net::SMTP->new((defined $smtp_server_port)
 -                                               ? "$smtp_server:$smtp_server_port"
 -                                               : $smtp_server,
 +                      $smtp ||= Net::SMTP->new(smtp_host_string(),
                                                 Hello => $smtp_domain,
                                                 Debug => $debug_net_smtp);
                        if ($smtp_encryption eq 'tls' && $smtp) {
                            defined $smtp_server_port ? " port=$smtp_server_port" : "";
                }
  
 -              if (defined $smtp_authuser) {
 -                      # Workaround AUTH PLAIN/LOGIN interaction defect
 -                      # with Authen::SASL::Cyrus
 -                      eval {
 -                              require Authen::SASL;
 -                              Authen::SASL->import(qw(Perl));
 -                      };
 -
 -                      if (!defined $smtp_authpass) {
 -
 -                              system "stty -echo";
 -
 -                              do {
 -                                      print "Password: ";
 -                                      $_ = <STDIN>;
 -                                      print "\n";
 -                              } while (!defined $_);
 -
 -                              chomp($smtp_authpass = $_);
 -
 -                              system "stty echo";
 -                      }
 -
 -                      $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 -              }
 +              smtp_auth_maybe or die $smtp->message;
  
                $smtp->mail( $raw_from ) or die $smtp->message;
                $smtp->to( @recipients ) or die $smtp->message;
@@@ -1270,6 -1258,7 +1274,7 @@@ foreach my $t (@files) 
        open my $fh, "<", $t or die "can't open file $t";
  
        my $author = undef;
+       my $sauthor = undef;
        my $author_encoding;
        my $has_content_type;
        my $body_encoding;
                        }
                        elsif (/^From:\s+(.*)$/i) {
                                ($author, $author_encoding) = unquote_rfc2047($1);
+                               $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;
                        }
                        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'});
                        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;
                $subject = quote_subject($subject, $auto_8bit_encoding);
        }
  
-       if (defined $author and $author ne $sender) {
+       if (defined $sauthor and $sauthor ne $sender) {
                $message = "From: $author\n\n$message";
                if (defined $author_encoding) {
                        if ($has_content_type) {
  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'";
                $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 $sender and $suppress_cc{'self'});
                push @addresses, $address;
                printf("($prefix) Adding %s: %s from: '%s'\n",
                       $what, $address, $cmd) unless $quiet;