[PATCH] git-send-email.perl extract_valid_address issue

[PATCH] git-send-email.perl extract_valid_address issue

[Nicolas Troncoso Carrere]

The third fallback was returning if the match was done or not instead of
returning the actual email address that was matched. This prevented sending
the mail to the people included in the CC. This bug only affect those that
dont have Email::Valid.

I initialized $valid_email as undef so it would mimic the behavior of 
Email::Verify->address(), which returns undef if no valid address was found.

Signed-off-by: Nicolas <ntroncos@inf.utfsm.cl>


---

 git-send-email.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

84853ca89c15de7a24e9eb9fd422654b86c63be9
diff --git a/git-send-email.perl b/git-send-email.perl
index 312a4ea..dfff3e6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -316,7 +316,9 @@ sub extract_valid_address {
 	} else {
 		# less robust/correct than the monster regexp in Email::Valid,
 		# but still does a 99% job, and one less dependency
-		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
+                my $valid_email=undef;
+                ($valid_email ) = ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
+                return ($valid_email);
 	}
 }
-- 
Nicolás Troncoso Carrère                        User #272312 counter.li.org
Estudiante Magíster en Ciencias de la Informática
Universidad Técnica Federico Santa María
http://www.alumnos.inf.utfsm.cl/~ntroncos

Re: [PATCH] git-send-email.perl extract_valid_address issue

[Ryan Anderson]

On Mon, May 29, 2006 at 12:00:44AM -0400, Nicolas Troncoso Carrere wrote:
> 
> The third fallback was returning if the match was done or not instead of
> returning the actual email address that was matched. This prevented sending
> the mail to the people included in the CC. This bug only affect those that
> dont have Email::Valid.
> 
> I initialized $valid_email as undef so it would mimic the behavior of 
> Email::Verify->address(), which returns undef if no valid address was found.

Odd, I noticed the same thing this weekend.

> Signed-off-by: Nicolas <ntroncos@inf.utfsm.cl>
Acked-by: Ryan Anderson <ryan@michonline.com>

(Or pick up my patch that fixes this in a slightly different way)

> 
> 
> ---
> 
>  git-send-email.perl |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> 84853ca89c15de7a24e9eb9fd422654b86c63be9
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 312a4ea..dfff3e6 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -316,7 +316,9 @@ sub extract_valid_address {
>  	} else {
>  		# less robust/correct than the monster regexp in Email::Valid,
>  		# but still does a 99% job, and one less dependency
> -		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
> +                my $valid_email=undef;
> +                ($valid_email ) = ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
> +                return ($valid_email);
>  	}
>  }
> -- 
> Nicol?s Troncoso Carr?re                        User #272312 counter.li.org
> Estudiante Mag?ster en Ciencias de la Inform?tica
> Universidad T?cnica Federico Santa Mar?a
> http://www.alumnos.inf.utfsm.cl/~ntroncos
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] git-send-email.perl extract_valid_address issue

[Horst von Brand]

Ryan Anderson <ryan@michonline.com> wrote:
> On Mon, May 29, 2006 at 12:00:44AM -0400, Nicolas Troncoso Carrere wrote:
> > 
> > The third fallback was returning if the match was done or not instead of
> > returning the actual email address that was matched. This prevented sending
> > the mail to the people included in the CC. This bug only affect those that
> > dont have Email::Valid.
> > 
> > I initialized $valid_email as undef so it would mimic the behavior of 
> > Email::Verify->address(), which returns undef if no valid address was found.
> 
> Odd, I noticed the same thing this weekend.
> 
> > Signed-off-by: Nicolas <ntroncos@inf.utfsm.cl>
> Acked-by: Ryan Anderson <ryan@michonline.com>
> 
> (Or pick up my patch that fixes this in a slightly different way)
> 
> > 
> > 
> > ---
> > 
> >  git-send-email.perl |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > 84853ca89c15de7a24e9eb9fd422654b86c63be9
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index 312a4ea..dfff3e6 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -316,7 +316,9 @@ sub extract_valid_address {
> >  	} else {
> >  		# less robust/correct than the monster regexp in Email::Valid,
> >  		# but still does a 99% job, and one less dependency
> > -		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
> > +                my $valid_email=undef;
> > +                ($valid_email ) = ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
> > +                return ($valid_email);
> >  	}
> >  }

I just looked it over, and this is certainly broken in the original. The
first return gives the string back if it matches, the second one evaluates
the list returned by the match in scalar context, and so returns 1 for a
match or 0 for non-match. Besides, the regexps are fishy...

- The first one won't consider 'Joe.Hacker' as a valid local address
- The second one will consider 'Joe.Hacker@example.com"' valid (note ")

Need to make both branches (with and without @) agree here, at least. And
this is certainly severely underpowered... dunno if it merits a better job
(or just bail out if Email::Valid isn't found, instead of kludgeing
around...)

Will propose a fix ASAP.
-- 
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

Re: [PATCH] git-send-email.perl extract_valid_address issue

[Horst von Brand]

Fix regexps to better match email addresses if Email::Valid isn't present
Make it return the address on match containing a @

Signed-off-by:  Horst H. von Brand <vonbrand@inf.utfsm.cl>
---
 git-send-email.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 312a4ea..6e39f2f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -309,14 +309,14 @@ sub extract_valid_address {
 	my $address = shift;
 
 	# check for a local address:
-	return $address if ($address =~ /^([\w\-]+)$/);
+	return $address if ($address =~ /^([\w\-.]+)$/);
 
 	if ($have_email_valid) {
 		return 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
-		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
+		return ($address =~ /([\w\-.]+@[\w\-.]+)/) ? $1 : undef;
 	}
 }
 
-- 
1.3.3.g0f0ac

Re: [PATCH] git-send-email.perl extract_valid_address issue

[Eric Wong]

Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> Fix regexps to better match email addresses if Email::Valid isn't present
> Make it return the address on match containing a @
> 
> Signed-off-by:  Horst H. von Brand <vonbrand@inf.utfsm.cl>
> ---
>  git-send-email.perl |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 312a4ea..6e39f2f 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -309,14 +309,14 @@ sub extract_valid_address {
>  	my $address = shift;
>  
>  	# check for a local address:
> -	return $address if ($address =~ /^([\w\-]+)$/);
> +	return $address if ($address =~ /^([\w\-.]+)$/);
>  
>  	if ($have_email_valid) {
>  		return 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
> -		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
> +		return ($address =~ /([\w\-.]+@[\w\-.]+)/) ? $1 : undef;
>  	}
>  }

Oops on the original bug, my fault :x
Acked-by: Eric Wong <normalperson@yhbt.net>

-- 
Eric Wong