Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Email-Valid CPAN distribution.

Report information
The Basics
Id: 68312
Status: resolved
Priority: 0/
Queue: Email-Valid

People
Owner: Nobody in particular
Requestors: mods [...] hank.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.184
Fixed in: (no value)



Subject: Only domain length is checked / and undefined warning
The domain length is checked to be <= 255, but the local part is not checked (max 64) and the overall length is not checked (limit of 254). See: http://www.rfc-editor.org/errata_search.php?rfc=3696&eid=1690 Also, when length is exceeded a warning is issued because: sub _valid_domain_parts { my ($self, $string) = @_; return unless $string and length $string <= 255; Which can return undefined but then is compared with ">" here: if ($args{fqdn}) { $self->_valid_domain_parts($addr->host) > 1
I've disabled warning for that comparison. I'll look at adding policies for the other lengths in the future. -- rjbs
On Thu Aug 11 13:03:10 2011, RJBS wrote: Show quoted text
> I'll look at adding policies for the other lengths in the future.
I've attached a simple patch I wrote that I believes takes care of this bug. It checks to see if the local part is <= 64. It adds a new arg (true by default). Tests and POD updated. Steve
Subject: bug66312.patch
diff -urN Email-Valid-0.186-orig/lib/Email/Valid.pm Email-Valid-0.186/lib/Email/Valid.pm --- Email-Valid-0.186-orig/lib/Email/Valid.pm 2012-01-22 11:18:47.000000000 -0500 +++ Email-Valid-0.186/lib/Email/Valid.pm 2012-01-26 01:11:40.000000000 -0500 @@ -22,6 +22,7 @@ mxcheck => 1, tldcheck => 1, local_rules => 1, + localpart => 1, ); $NSLOOKUP_PAT = 'preference|serial|expire|mail\s+exchanger'; @@ -58,6 +59,7 @@ $self->{fudge} = 0; $self->{fqdn} = 1; $self->{local_rules} = 0; + $self->{localpart} = 1; $self->{details} = $Details = undef; } @@ -280,6 +282,14 @@ 1; } +sub _valid_local_part { + my ($self, $localpart) = @_; + + return 0 unless $localpart and length $localpart <= 64; + + return 1; +} + sub _valid_domain_parts { my ($self, $string) = @_; @@ -327,6 +337,12 @@ or return $self->details('local_rules'); } + if ($args{localpart}) { + # bug 68312 fix + $self->_valid_local_part($addr->user) > 0 + or return $self->details('localpart'); + } + if ($args{fqdn}) { no warnings 'uninitialized'; # valid domain parts might return undef $self->_valid_domain_parts($addr->host) > 1 @@ -512,6 +528,7 @@ -fudge -fqdn -local_rules + -localpart =item mx ( <ADDRESS>|<DOMAIN> ) @@ -556,6 +573,12 @@ for domain specific restrictions. Currently, this is limited to certain AOL restrictions that I'm aware of. The default is false. +=item localpart ( <TRUE>|<FALSE> ) + +Specifies whether addresses passed to address() should be tested +against the RFC3696 'local part' (username portion) length restriction. +The default is true. + =item mxcheck ( <TRUE>|<FALSE> ) Specifies whether addresses passed to address() should be checked diff -urN Email-Valid-0.186-orig/t/valid.t Email-Valid-0.186/t/valid.t --- Email-Valid-0.186-orig/t/valid.t 2012-01-22 11:15:51.000000000 -0500 +++ Email-Valid-0.186/t/valid.t 2012-01-26 00:54:27.000000000 -0500 @@ -1,7 +1,7 @@ #!perl use strict; -use Test::More tests => 24; +use Test::More tests => 27; BEGIN { use_ok('Email::Valid'); @@ -54,6 +54,20 @@ "comments nicely dropped from an address", ); +is( + $v->address(-address => 'somebody@example.com', -localpart => 1), + 'somebody@example.com', + "localpart with 64 chars or less is valid", +); + +is( + $v->address(-address => 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com', -localpart => 1), + undef, + "localpart with 64 chars or more fails", +); + +is($v->details, 'localpart', "details are localpart"); + ok( $v->address('somebody@ example.com'), "space between @ and domain is valid",
On Thu Jan 26 01:40:59 2012, steveb wrote: Show quoted text
> On Thu Aug 11 13:03:10 2011, RJBS wrote: >
> > I'll look at adding policies for the other lengths in the future.
> > I've attached a simple patch I wrote that I believes takes care of this > bug.
...I forgot about the full address length check. I'll hack that in tomorrow.
On Thu Jan 26 01:40:59 2012, steveb wrote: Show quoted text
> On Thu Aug 11 13:03:10 2011, RJBS wrote: >
> > I'll look at adding policies for the other lengths in the future.
Attached the new patch. I've removed the public visibility of the new arg. This one checks for full address length, as well as the length of the local part. I've also eliminated the warnings workaround by putting in an explicit check for undef.
Subject: bug66312.patch
diff -urN Email-Valid-0.186-orig/lib/Email/Valid.pm Email-Valid-0.186/lib/Email/Valid.pm --- Email-Valid-0.186-orig/lib/Email/Valid.pm 2012-01-22 11:18:47.000000000 -0500 +++ Email-Valid-0.186/lib/Email/Valid.pm 2012-01-26 10:44:15.000000000 -0500 @@ -22,6 +22,7 @@ mxcheck => 1, tldcheck => 1, local_rules => 1, + localpart => 1, ); $NSLOOKUP_PAT = 'preference|serial|expire|mail\s+exchanger'; @@ -58,6 +59,7 @@ $self->{fudge} = 0; $self->{fqdn} = 1; $self->{local_rules} = 0; + $self->{localpart} = 1; $self->{details} = $Details = undef; } @@ -280,6 +282,14 @@ 1; } +sub _valid_local_part { + my ($self, $localpart) = @_; + + return 0 unless $localpart and length $localpart <= 64; + + return 1; +} + sub _valid_domain_parts { my ($self, $string) = @_; @@ -322,15 +332,26 @@ $addr or return $self->details('rfc822'); # This should never happen + if (length($addr->address) > 254) { + return $self->details('address_too_long'); + } + if ($args{local_rules}) { $self->_local_rules( $addr->user, $addr->host ) or return $self->details('local_rules'); } + if ($args{localpart}) { + $self->_valid_local_part($addr->user) > 0 + or return $self->details('localpart'); + } + if ($args{fqdn}) { - no warnings 'uninitialized'; # valid domain parts might return undef - $self->_valid_domain_parts($addr->host) > 1 - or return $self->details('fqdn'); + my $domain_parts = $self->_valid_domain_parts($addr->host); + + if (! $domain_parts or $domain_parts <= 1){ + return $self->details('fqdn'); + } } if ($args{tldcheck}) { diff -urN Email-Valid-0.186-orig/t/valid.t Email-Valid-0.186/t/valid.t --- Email-Valid-0.186-orig/t/valid.t 2012-01-22 11:15:51.000000000 -0500 +++ Email-Valid-0.186/t/valid.t 2012-01-26 10:32:15.000000000 -0500 @@ -1,7 +1,7 @@ #!perl use strict; -use Test::More tests => 24; +use Test::More tests => 29; BEGIN { use_ok('Email::Valid'); @@ -54,6 +54,27 @@ "comments nicely dropped from an address", ); +is ($v->address(-address => 'user@example.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'), + undef, + "address with > 254 chars fails", +); + +is($v->details, 'address_too_long', "details say address is too long"); + +is( + $v->address(-address => 'somebody@example.com', -localpart => 1), + 'somebody@example.com', + "localpart with 64 chars or less is valid", +); + +is( + $v->address(-address => 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com', -localpart => 1), + undef, + "localpart with 64 chars or more fails", +); + +is($v->details, 'localpart', "details are localpart"); + ok( $v->address('somebody@ example.com'), "space between @ and domain is valid",
applied in the git repo; thanks -- rjbs