Re: [dfv] Problem with basic constraints (+patch)

[Evan A. Z. <e-...@za...>]

On Wed, Oct 25, 2006 at 02:48:55PM -0700, Brian E. Lozier wrote:

> I'm having a problem getting constraints working properly.  I'm
[...]

> I've tracked the culprit down to two spots.  First, in the BEGIN of
> Constraints.pm, the match_zip function is called thusly:
> 
>                    return &{"match_$func"}(\@_);

This is part of the closure generation for the "new school"
constraint_methods. You'll notice in the routine that is
generated (that contains the line above), the first argument is
shifted off the argument list (my $dfv = shift).

> I go into the match_zip function and it looks like this:
> 
> sub match_zip {
>    my $val = shift;
[...]

> Very straightforward.  However, "val" is always the number 1.  I did a
> Data::Dumper(\@_) inside match_zip, reran the test, and it reports
> that @_ contains two elements, the first is a DFV object, and the
> second is the actual value to be matched.
> 
> If I put a my $self = shift; at the top, it works fine.  All the other
> match_* functions suffer from the same problem.

Right. The problem is that when a built-in ('zip') is specified
by name in the constraint_methods section of a profile, the
built-in function (either match_zip or valid_zip) is invoked as a
method directly -- i.e., not through the closure that was
generated in the BEGIN block noted above.

> Since this is such a glaring error, I'm wondering if  A) nobody else
> uses the built-in constraints, or B) something is wrong with the way
> I'm setting up the profile, or C) something is wrong with my perl.

I think you are following the documentation correctly, but have
uncovered a bug.

> Thanks in advance for any help you can offer.

For now, you can just replace the use of the built-in as a string
with a method call:

    use Data::FormValidator::Constraints qw(:closures);

    my %profile = (
      optional => ['zip'],
      constraint_methods => {
          zip => zip(),
      }
    );

Or, if you use the "old school" constraints directive, you can
use the built-in as a string:

    my %profile = (
      optional => ['zip'],
      constraints => {
          zip => 'zip', # or ['zip']
      },
    );


To address this bug, I updated the code that sets the is_method
attribute for each constraint (in _constraint_hash_build()). For
example, if we have to qualify a built-in from a string (name) to
a match_* routine, is_method should be set to false. This appears
to solve the problem.

Attached is a patch against 4.49_1, including a new test script
based on Brian's report. Note that it caused some tests in
t/untaint.pl to fail.

Those tests appear to be broken, however. They are checking for
taintedness of the "valid" values returned, but those values are
empty, so they always appear untainted. Adding several tests to
t/untaint.pl confirmed that they are broken in 4.49_1, so I
updated t/untaint.pl as well.

There are fifteen new tests, twelve of which fail against 4.49_1,
all of which pass after the attached patch is applied.

Let me know if you think the approach I've taken is the best one
to address the problem Brian discovered.

Thanks,
-E