Bug #28769 for FormValidator-Simple: DUPLICATION doesn't work with undef

Thu Aug 09 15:14:07 2007 claco [...] cpan.org - Ticket created

Subject:

DUPLICATION doesn't work with undef

It appears that this was coded on purpose, but I'm not sure that it's the right thing to do. unless (defined $data1 && defined $data2) { FormValidator::Simple::Exception->throw( qq/validation "DUPLICATION" needs two keys of data./ ); } The problem is that it's not uncommon to have things like password/confirm_password fields, where there are both set to NOT_BLANK, and confirms' set to DUPLICATION(password, confirm_password). If the user doesn't enter one of the fields, NOT_BLANK will fire, but DUPLICATION will bail because one value is undef. Duplication has two keys of data (two params were sent), it's just that one is undef. I'd vote that DUPLICATION should honor the fact that it was given two entries in @{$params} and fail the equality test. If @{$params} only contains one value, then continue to complain that two values must be sent. Patch forthcoming if no one objects.

Thu Aug 09 15:51:26 2007 claco [...] cpan.org - Correspondence added

On Thu Aug 09 15:14:07 2007, CLACO wrote: Show quoted text

> It appears that this was coded on purpose, but I'm not sure that it's > the right thing to do. > > unless (defined $data1 && defined $data2) { > FormValidator::Simple::Exception->throw( > qq/validation "DUPLICATION" needs two keys of data./ > ); > } > > > The problem is that it's not uncommon to have things like > password/confirm_password fields, where there are both set to NOT_BLANK, > and confirms' set to DUPLICATION(password, confirm_password). If the > user doesn't enter one of the fields, NOT_BLANK will fire, but > DUPLICATION will bail because one value is undef. Duplication has two > keys of data (two params were sent), it's just that one is undef. > > I'd vote that DUPLICATION should honor the fact that it was given two > entries in @{$params} and fail the equality test. If @{$params} only > contains one value, then continue to complain that two values must be

sent. Show quoted text

> > Patch forthcoming if no one objects.

Patch+tests attached.

--- Validator.pm.orig Tue Sep 12 05:24:58 2006 +++ Validator.pm Thu Aug 09 15:46:27 2007 @@ -43,7 +43,7 @@ my ($self, $params, $args) = @_; my $data1 = $params->[0]; my $data2 = $params->[1]; - unless (defined $data1 && defined $data2) { + unless (@{$params} == 2) { FormValidator::Simple::Exception->throw( qq/validation "DUPLICATION" needs two keys of data./ );

--- 12_duplication.t.orig Sun Sep 11 12:29:02 2005 +++ 12_duplication.t Thu Aug 09 15:43:16 2007 @@ -1,5 +1,5 @@ use strict; -use Test::More tests => 5; +use Test::More tests => 8; use CGI; BEGIN { use_ok("FormValidator::Simple") } @@ -35,3 +35,23 @@ ] ); ok(!$r4->invalid('email_dup')); + +# defined, but undef +$q->param( email2 => undef ); +my $r5 = FormValidator::Simple->check( $q => [ + { email_dup => [qw/email1 email2/] } => [qw/NOT_DUPLICATION/], +] ); +ok(!$r4->invalid('email_dup')); + +my $r6 = FormValidator::Simple->check( $q => [ + { email_dup => [qw/email1 email2/] } => [qw/DUPLICATION/], +] ); +ok($r6->invalid('email_dup')); + +# only one param passed +eval { + my $r7 = FormValidator::Simple->check( $q => [ + { email_dup => [qw/email1/] } => [qw/DUPLICATION/], + ] ); +}; +like($@, qr/needs two keys/);

Thu Aug 09 15:51:32 2007 claco [...] cpan.org - Status changed from 'new' to 'open'

Thu Aug 09 16:10:06 2007 claco [...] cpan.org - Correspondence added

On Thu Aug 09 15:51:26 2007, CLACO wrote: Show quoted text

> On Thu Aug 09 15:14:07 2007, CLACO wrote:

> > It appears that this was coded on purpose, but I'm not sure that it's > > the right thing to do. > > > > unless (defined $data1 && defined $data2) { > > FormValidator::Simple::Exception->throw( > > qq/validation "DUPLICATION" needs two keys of data./ > > ); > > } > > > > > > The problem is that it's not uncommon to have things like > > password/confirm_password fields, where there are both set to NOT_BLANK, > > and confirms' set to DUPLICATION(password, confirm_password). If the > > user doesn't enter one of the fields, NOT_BLANK will fire, but > > DUPLICATION will bail because one value is undef. Duplication has two > > keys of data (two params were sent), it's just that one is undef. > > > > I'd vote that DUPLICATION should honor the fact that it was given two > > entries in @{$params} and fail the equality test. If @{$params} only > > contains one value, then continue to complain that two values must be

> sent.

> > > > Patch forthcoming if no one objects.

> > Patch+tests attached.

Bah. I forgot about uninit warnings. Update Validator.pm.patch.

--- Validator.pm.orig Tue Sep 12 05:24:58 2006 +++ Validator.pm Thu Aug 09 16:05:09 2007 @@ -41,13 +41,15 @@ sub DUPLICATION { my ($self, $params, $args) = @_; - my $data1 = $params->[0]; - my $data2 = $params->[1]; - unless (defined $data1 && defined $data2) { + unless (@{$params} == 2) { FormValidator::Simple::Exception->throw( qq/validation "DUPLICATION" needs two keys of data./ ); } + my $data1 = $params->[0]; + my $data2 = $params->[1]; + + no warnings 'uninitialized'; return $data1 eq $data2 ? TRUE : FALSE; }