Random t/01_MD5.t failures about text-password-automigration HOT 7 CLOSED

Hi, thank you for reporting Issue

To tell the truth, I knew that sometimes it fails when the salt was empty.
But I can't find the cause yet.

what's wrong do you think? I've already implemented checking length of salts.

sub generate {
    my $self = shift;
    my $length = shift || $self->default();
    my $min = $self->minimum();

    croak "unvalid length was set" unless $length =~ /^\d+$/;
    croak ref($self) . "::generate requires list context" unless wantarray;
    croak ref($self) . "::generate requires at least $min length" if $length < $min;

    my $raw;
    do {	# redo unless it gets enough readability
        $raw = $self->nonce($length);
        return $raw, $self->encrypt($raw) unless $self->readability();
    }while( $raw =~ /[0Oo1Il|!2Zz5sS\$6b9qCcKkUuVvWwXx.,:;~\-^'"`]/i );

    return $raw, $self->encrypt($raw);
}

generate() uses encrypt(), So it's problem of encrypt()

override 'encrypt' => sub {
    my $self = shift;
    my $input = shift;
    my $min = $self->minimum();
    croak __PACKAGE__ ." requires at least $min length" if length $input < $min;
     die __PACKAGE__. " doesn't allow any Wide Characters or white spaces\n"
    if $input !~ /[!-~]/ or $input =~ /\s/;

    my $salt = shift || $self->nonce();
    carp "warning: short lengths salt is set. you don't have to" if length($salt) < 8;
     carp "warning: too many string lengths for salt. unix_md5_crypt() ignores more than 8"
    if $salt and length($salt) > 8;

    return unix_md5_crypt( $input, $salt );
};

I thought that if length of salt was 0, carp will work and shows the warning. but it doesn't.

from text-password-automigration.

I fixed the code to cover this issue.

Now verifying test(returns new MD5) runs 50 times in t/01_MD5.t

And 100 times in t/03_AutoMigrate.t for each verify

According to my local tests, the salts' strings were irrelevant.
Maybe, the bug happens when the salt is null after that being made.
I gave up to quest the details.
So I used do-until the methods returns correct hash we expect.

How do you think about this approach?
Is there more than one way to do it?

from text-password-automigration.

I don't have a better idea.

from text-password-automigration.

Thank you for replying.

How about this way? Good or Bad?

    my $hash;
     do{ $hash = unix_md5_crypt( $input, $self->nonce(8) ) }
    until( $hash =~ /^\$1\$[!-~]{1,8}\$[!-~]{22}$/ );
    return $hash;

This is the implement. I think it is bad because there is no effort to fix the fundamental problem.

If you say bad, I have to find other ways.

from text-password-automigration.

according to latest cpantesters' report(I haven't read well yet), also CORE::crypt may return undef
So, perhaps, it may not be the problem of this module.

It seems that it is the cause of these errors that RARELY these encryption functions return undef.
I will try to catch these problem next week.

the first impression is here

1. invalid salts(or nonce) was threw and the function returned undef

• I have to find out what value is invalid
• And rewrite the tests

2. these functions seldom returns undef

• I don't think it is the cause at all. but if it's true, I have to report this bug ASAP.

from text-password-automigration.

http://matrix.cpantesters.org/?dist=Text-Password-AutoMigration%200.12

It seems that it is the problem of OpenBSD.
But when I add use Devel::AssertOS qw(-OpenBSD); it doesn't work on perl 5.8.x

It's too difficult for me.

from text-password-automigration.

I found out the cause of this Issue!
when the salt includes '$' , encrypting with MD5 returns invalid strings like '$1$$VSbDIidP8E7Q6zF3LWXSw1'

So I fixed the codes so that salts never include '$'.

And it seems The problem of around OpenBSD isn't related to this Issue.
So I have moved that to #4

from text-password-automigration.