Horst von Brand <vonbrand@xxxxxxxxxxxx> writes:
>> > # check for a local address:
>> > - return $address if ($address =~ /^([\w\-]+)$/);
>> > + return $address if ($address =~ /^([\w\-.]+)$/);
>>
>> I keep forgetting this, '+' is a valid (and useful) setup, too.
>
> Oops...
>>
>> Actually, I'm retracting my earlier ack on this. This is way too
>> restrictive. I'd rather allow an occasional invalid email address than
>> to reject valid ones. I generally trust git users to know what they're
>> doing when entering email addresses[1].
>>
>> *, $, ^, +, = are all valid characters in the username portion (not sure
>> about local accounts, though), and I'm sure there are more that I don't
>> know about.
>
> As a general principle, I prefer to check what is legal instead of trying
> to filter out what isn't.
If we start doing addr-spec in RFC2822 (page 17) ourselves, we
should rather be using Email::Valid. A permissive sanity check
to catch obvious mistakes would be more appropriate here than
being RFC police.
I think something like the attached, on top of your patch, would
be appropriate for upcoming 1.4.0.
-- >8 --
send-email: be more lenient and just catch obvious mistakes.
This cleans up the pattern matching subroutine by introducing
two variables to hold regexp to approximately match local-part
and domain in the e-mail address. It is meant to catch obvious
mistakes with a cheap check.
The patch also moves "scalar" to force Email::Valid->address()
to work in !wantarray environment to extract_valid_address;
earlier it was in the caller of the subroutine, which was way
too error prone.
---
diff --git a/git-send-email.perl b/git-send-email.perl
index a7a7797..700d0c3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -312,16 +312,18 @@ our ($message_id, $cc, %mail, $subject,
sub extract_valid_address {
my $address = shift;
+ my $local_part_regexp = '[^<>"\s@]+';
+ my $domain_regexp = '[^.<>"\s@]+\.[^<>"\s@]+';
# check for a local address:
- return $address if ($address =~ /^([\w\-.]+)$/);
+ return $address if ($address =~ /^($local_part_regexp)$/);
if ($have_email_valid) {
- return Email::Valid->address($address);
+ return scalar Email::Valid->address($address);
} else {
# less robust/correct than the monster regexp in Email::Valid,
# but still does a 99% job, and one less dependency
- $address =~ /([\w\-.]+@[\w\-.]+)/;
+ $address =~ /($local_part_regexp\@$domain_regexp)/;
return $1;
}
}
@@ -384,7 +386,7 @@ X-Mailer: git-send-email $gitversion
defined $pid or die $!;
if (!$pid) {
exec($smtp_server,'-i',
- map { scalar extract_valid_address($_) }
+ map { extract_valid_address($_) }
@recipients) or die $!;
}
print $sm "$header\n$message";
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html