[PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Greg KH]

When using git-send-email.perl on a changeset that has:
	Cc: <stable@kernel.org>
in the body of the description, and the Email::Valid perl module is
installed on the system, the email address will be deemed "invalid" for
some reason (Email::Valid isn't smart enough to handle this?) and
complain and not send the address the email.

Anyway, this tiny patch fixes this problem for me.  Note, my perl-foo is
quite week, so this could probably be easily done in one line for those
with better reg-ex skills.


Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -410,6 +410,9 @@ sub extract_valid_address {
 	return $address if ($address =~ /^($local_part_regexp)$/);
 
 	if ($have_email_valid) {
+		if ($address =~ s/^<//) {
+			$address =~ s/>$//;
+		}
 		return scalar Email::Valid->address($address);
 	} else {
 		# less robust/correct than the monster regexp in Email::Valid,

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Junio C Hamano]

Greg KH <greg@kroah.com> writes:

> When using git-send-email.perl on a changeset that has:
> 	Cc: <stable@kernel.org>
> in the body of the description, and the Email::Valid perl module is
> installed on the system, the email address will be deemed "invalid" for
> some reason (Email::Valid isn't smart enough to handle this?) and
> complain and not send the address the email.

That appears to be the case.

        bad foo
        bad <foo@bar.baz>
        ok  foo@bar.baz
        ok  Foo <foo@bar.baz>

> Anyway, this tiny patch fixes this problem for me.  Note, my perl-foo is
> quite week, so this could probably be easily done in one line for those
> with better reg-ex skills.

> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -410,6 +410,9 @@ sub extract_valid_address {
>  	return $address if ($address =~ /^($local_part_regexp)$/);
>  
>  	if ($have_email_valid) {
> +		if ($address =~ s/^<//) {
> +			$address =~ s/>$//;
> +		}
>  		return scalar Email::Valid->address($address);
>  	} else {

I'd probably do:

	if ($have_email_valid) {
		$address =~ s/^<(.*)>$/$1/;
		return scalar Email::Valid->address($address);
	} else {

instead, but they are moral equivalents.

Thanks for a fix.

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Greg KH]

On Thu, Jul 12, 2007 at 10:47:17PM -0700, Junio C Hamano wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > When using git-send-email.perl on a changeset that has:
> > 	Cc: <stable@kernel.org>
> > in the body of the description, and the Email::Valid perl module is
> > installed on the system, the email address will be deemed "invalid" for
> > some reason (Email::Valid isn't smart enough to handle this?) and
> > complain and not send the address the email.
> 
> That appears to be the case.
> 
>         bad foo
>         bad <foo@bar.baz>
>         ok  foo@bar.baz
>         ok  Foo <foo@bar.baz>
> 
> > Anyway, this tiny patch fixes this problem for me.  Note, my perl-foo is
> > quite week, so this could probably be easily done in one line for those
> > with better reg-ex skills.
> 
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -410,6 +410,9 @@ sub extract_valid_address {
> >  	return $address if ($address =~ /^($local_part_regexp)$/);
> >  
> >  	if ($have_email_valid) {
> > +		if ($address =~ s/^<//) {
> > +			$address =~ s/>$//;
> > +		}
> >  		return scalar Email::Valid->address($address);
> >  	} else {
> 
> I'd probably do:
> 
> 	if ($have_email_valid) {
> 		$address =~ s/^<(.*)>$/$1/;

Ah, yeah, that looks better :)

Thanks for the fix,

greg k-h

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Thu, 12 Jul 2007 23:34:14 -0700 Greg KH <greg@kroah.com> wrote:
>
> On Thu, Jul 12, 2007 at 10:47:17PM -0700, Junio C Hamano wrote:
> > Greg KH <greg@kroah.com> writes:
> > 
> > > When using git-send-email.perl on a changeset that has:
> > > 	Cc: <stable@kernel.org>
> > > in the body of the description, and the Email::Valid perl module is
> > > installed on the system, the email address will be deemed "invalid" for
> > > some reason (Email::Valid isn't smart enough to handle this?) and
> > > complain and not send the address the email.
> > 
> > That appears to be the case.
> > 
> >         bad foo
> >         bad <foo@bar.baz>
> >         ok  foo@bar.baz
> >         ok  Foo <foo@bar.baz>

This would be a bug in Email::Valid as it complains that the second
address fails the rfc822 check, however rfc822 says that the "display
name" before the '<' is optional.

> > I'd probably do:
> > 
> > 	if ($have_email_valid) {
> > 		$address =~ s/^<(.*)>$/$1/;

	$address =~ s/^\s*<(.*)>\s*$/$1/;

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Greg KH]

On Fri, Jul 13, 2007 at 06:28:18PM +1000, Stephen Rothwell wrote:
> On Thu, 12 Jul 2007 23:34:14 -0700 Greg KH <greg@kroah.com> wrote:
> >
> > On Thu, Jul 12, 2007 at 10:47:17PM -0700, Junio C Hamano wrote:
> > > Greg KH <greg@kroah.com> writes:
> > > 
> > > > When using git-send-email.perl on a changeset that has:
> > > > 	Cc: <stable@kernel.org>
> > > > in the body of the description, and the Email::Valid perl module is
> > > > installed on the system, the email address will be deemed "invalid" for
> > > > some reason (Email::Valid isn't smart enough to handle this?) and
> > > > complain and not send the address the email.
> > > 
> > > That appears to be the case.
> > > 
> > >         bad foo
> > >         bad <foo@bar.baz>
> > >         ok  foo@bar.baz
> > >         ok  Foo <foo@bar.baz>
> 
> This would be a bug in Email::Valid as it complains that the second
> address fails the rfc822 check, however rfc822 says that the "display
> name" before the '<' is optional.

I agree, do you know how to get such a fix made?

thanks,

greg k-h

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Fri, 13 Jul 2007 21:00:50 -0700 Greg KH <greg@kroah.com> wrote:
>
> On Fri, Jul 13, 2007 at 06:28:18PM +1000, Stephen Rothwell wrote:
> > 
> > This would be a bug in Email::Valid as it complains that the second
> > address fails the rfc822 check, however rfc822 says that the "display
> > name" before the '<' is optional.
> 
> I agree, do you know how to get such a fix made?

I would be inclined to make it my distribution's problem :-) i.e. on
Debian I would report a bug against libemail-valid-perl.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] make git-send-email.perl handle email addresses with no names when Email::Valid is present

[Uwe Kleine-König]

Hello,

Greg KH wrote:
> When using git-send-email.perl on a changeset that has:
> 	Cc: <stable@kernel.org>
> in the body of the description, and the Email::Valid perl module is
> installed on the system, the email address will be deemed "invalid" for
> some reason (Email::Valid isn't smart enough to handle this?) and
> complain and not send the address the email.
The reason is probably that it is indeed invalid.  From rfc822:

mailbox		= addr-spec / phrase route-addr
addr-spec	= local-part "@" domain
phrase		= 1*word
word		= atom / quoted-string
atom		= 1*<any CHAR except specials, SPACE and CTLs>
quoted-string	= <"> *(qtext/quoted-pair) <">
route-addr	= "<" [route] addr-spec ">"
...

where 1* means "at least one of".

That is, either you must not use <...> or you need a non-empty phrase.

Actually this grammar looks wrong, because as I read it it would not
allow spaces between words in phrase.  But that's another issue.

BTW: Outlook depends on this, because if you use Cc:
<stable@kernel.org>, it doesn't show anything in the Cc: line---at least
in the default configuration.

So I suggest the following:

---- >8 ----
send-email: rfc822 forbids using <address@domain> without a non-empty "phrase"

Email::Valid does respect this considering such a mailbox specification
invalid.  b06c6bc831cbb9e9eb82fd3ffd5a2b674cd940d0 addressed the issue, but
only if Email::Valid is available.

Signed-off-by: Uwe Kleine-König <ukleinek@informatik.uni-freiburg.de>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 39e433b..a02ab96 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -408,8 +408,8 @@ sub extract_valid_address {
 	# check for a local address:
 	return $address if ($address =~ /^($local_part_regexp)$/);
 
+	$address =~ s/^\s*<(.*)>\s*$/$1/;
 	if ($have_email_valid) {
-		$address =~ s/^\s*<(.*)>\s*$/$1/;
 		return scalar Email::Valid->address($address);
 	} else {
 		# less robust/correct than the monster regexp in Email::Valid,
-- 
1.5.3.rc3.943.g14c81

-- 
Uwe Kleine-König

If a lawyer and an IRS agent were both drowning, and you could only save
one of them, would you go to lunch or read the paper?