Junio C Hamano <junkio@xxxxxxx> wrote:
> 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.
OK.
> 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.
Right.
> ---
> 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@]+';
This forces a '.' in the domain, while vonbrand@localhost is perfectly
reasonable. Plus it doesn't disallow adyacent '.'s. What about:
my $domain_regexp = '[^.<>"\s@]+(\.[^<>"\s@]+)*';
(but this is probably nitpicking...)
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
-
: 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