Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password validation rule fails on valid data if reused #38949

Closed
hbgl opened this issue Sep 24, 2021 · 0 comments · Fixed by #38962
Closed

Password validation rule fails on valid data if reused #38949

hbgl opened this issue Sep 24, 2021 · 0 comments · Fixed by #38962

Comments

@hbgl
Copy link

hbgl commented Sep 24, 2021

  • Laravel Version: 8.54
  • PHP Version: 8.0.11
  • Database Driver & Version: n/a

Description:

If a Password validation rule fails once and the same rule object is later reused, then it will continue to fail on any subsequent validation run even if the input is valid.

Steps To Reproduce:

$rules = [
    'password' => Password::default(),
];

$validator1 = Validator::make(['password' => '1234'], $rules);
// Validation fails as expected: Password too short.
$this->assertTrue($validator1->fails());

$validator2 = Validator::make(['password' => '12341234'], $rules);
// Should pass, but it does not. Validation fails again with: Password too short.
$this->assertFalse($validator2->fails());

https://github.com/hbgl/laravel-validation-password-stateful

Suggested fix

TLDR: Clear the $messages field inside the Password class on each call to passes.

The Password rule internally keeps a list or messages that are populated within the passes function. The messages are, however, not reset upon repeated calls to passes, which causes the rule to fail on valid data with the same error messages as before. The messages should be reset each time the passes function is called.

I think it is generally good practice for validation rules to not carry observable state over multiple validation runs. That means that a rule should either be completely stateless or carefully reset itself when called again. Maybe this should also be mentioned as a warning in the custom validation rules documentation

taylorotwell pushed a commit that referenced this issue Sep 26, 2021
* This should fix #38949

* apply ci fixes
victorvilella pushed a commit to cdsistemas/framework that referenced this issue Oct 12, 2021
* This should fix laravel#38949

* apply ci fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant