Re: One whitespace breaks sending mail (using mailrc aliases wit send-emal)

From: Jeff King <peff@peff.net>
Date: Mar 17, 2016, 02:00 UTC
Message-ID: <20160317020015.GC12830@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kbRdPMkbs_-BGhCQ44hOsccG3aX16RJpE3ZHcBmDz40zQ@mail.gmail.com>

On Wed, Mar 16, 2016 at 06:28:59PM -0700, Stefan Beller wrote:

Show 17 quoted lines

> To reproduce:
> 
> $ git config sendemail.aliasesfile ~/.mailrc
> $ git config sendemail.aliasfiletype mailrc
> $ echo "alias zzz_wrong_entry jon@do.e "
> # notice the white space at the end of the alias
> 
> $ git send-email 0001-rebase-x-do-not-die-without-i.patch --cc=zzz_wrong_entry
> Use of uninitialized value $alias in hash element at
> /usr/local/google/home/sbeller/libexec/git-core/git-send-email line
> 847.
> [...]
> The stack trace doesn't even show the wrong address, so debugging that was hard.
> Not sure I am asking for help, as I found the problem and could fix
> it, but maybe we can
> improve the error message (I have no knowledge about perl, so I may
> not be super helpful there)

I don't think so. The problem is that we return `undef` for one of the aliases, and then later when we look at that, perl obviously is unhappy. It's sort of the equivalent of a C program dereferencing NULL, but the bug is assigning NULL in the first place much earlier.

You _can_ tell perl not to bother warning about accessing undef like this, but it's usually a good idea to have such warnings on, as they are helpful for finding real bugs.

I imagine your problem is by:

diff --git a/git-send-email.perl b/git-send-email.perl
index d356901..c45b22a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -533,7 +533,7 @@ my %parse_alias = (
 			$aliases{$alias} = \@addr
 		}}},
 	mailrc => sub { my $fh = shift; while (<$fh>) {
-		if (/^alias\s+(\S+)\s+(.*)$/) {
+		if (/^alias\s+(\S+)\s+(.*?)\s*$/) {
 			# spaces delimit multiple addresses
 			$aliases{$1} = [ quotewords('\s+', 0, $2) ];
 		}}},

to suppress the extra whitespace at the end (alternatively, there may be
a way to tell quotewords() to throw out a trailing delimiter, but I
don't know the Text::ParseWords module well enough to say offhand).

-Peff