| Subject: | Logic error in _checkVariation() |
Thanks for the helpful module.
Unless I'm looking at how the variation check should work, I believe there is a logic error here in "sub _checkVariation()":
foreach (@{$self->{'word_list'}}) {
($self->{result}->{'check_variation'} = 0 && return) if /$regexp/i;
}
I think it should be:
foreach (@{$self->{'word_list'}}) {
($self->{result}->{'check_variation'} = 0 || return) if /$regexp/i;
}
Because "$self->{result}->{'check_variation'} = 0" always returns "false", the return never executes if they are connected via a "&&", so the loop always falls through to the "$self->{result}->{'check_variation'}" shown below the loop.
Or use:
foreach (@{$self->{'word_list'}}) {
if (/$regexp/i) {
$self->{result}->{'check_variation'} = 0;
return);
}
}
Here is a test that demonstrates it:
#!/usr/bin/env perl
use strict;
use warnings;
use Data::Password::Filter;
use Test::More;
my $password = Data::Password::Filter->new();
# Need at least one passing call _checkDictionary() first or _checkVariation()
# skips its checking
ok(!$password->_checkDictionary('appliance'), "'Fail: seed _checkDictionary()");
ok( $password->_checkDictionary('aB4Ds4Xfj'), "'Pass: seed _checkDictionary()");
ok(!$password->_checkVariation('appliance'), "Fail: Straight dictionary word");
ok(!$password->_checkVariation('Xppliance'), "Fail: Change one char (first)");
ok(!$password->_checkVariation('applXance'), "Fail: Change one char (mid)");
ok(!$password->_checkVariation('appliancX'), "Fail: Change one char (final)");
ok( $password->_checkVariation('applXXnce'), "Pass: Change two chars (mid)");
ok( $password->_checkVariation('XppliancX'), "Pass: Change two chars (first & final)");
ok( $password->_checkVariation('applXancX'), "Pass: Change two chars (mid & final)");
done_testing();
Also, a few other comments that might help others get up to speed in it's use:
1) Would help to document that the dictionary test is not case sensitive (it's good you made it that way, but might help others know you covered that case)
2) Would be good to explain what the "variation" check is doing (looking for passwords that only vary by one character from a dictionary word)
3) I found it helpful to call the various methods like _checkDictionary(), _checkVariation(), _checkLength(), _checkUppercaseLetter(), etc. directory to have more granular feedback on where the failure occurred. It might be helpful to make these "non private" and document there use in the POD for others.
Thanks again