$request->validated() doesn't include all validated attributes

[crashkonijn]

• Laravel Version: 5.5.40

Description:

$request->validated() only includes the attributes which have been defined in the Request->rules() function, not those who have been added via $validator->sometimes()

Steps To Reproduce:

class TestRequest extends Request
{
    public function authorize()
    {
        return true;
    }

    public function rules()
    {
        return [
            'email' => 'required|email'
        ];
    }
	
    public function getValidatorInstance(): Validator
    {
        $validator = parent::getValidatorInstance();

        $validator->sometimes('password', 'required', function () {
	    return true;
	});

        return $validator;
    }
}

class TestController extends Controller
{
    public function index(TestRequest $request)
    {
        dd($request->validated());
    }
}

Validated won't include the password, even though it does get actually validated.

Possible fix

Making this change in FormRequest fixes the issue:

public function validated()
{
    //$rules = $this->container->call([$this, 'rules']);
    $rules = $this->getValidatorInstance()->getRules();

    return $this->only(collect($rules)->keys()->map(function ($rule) {
        return str_contains($rule, '.') ? explode('.', $rule)[0] : $rule;
    })->unique()->toArray());
}

[brunogaspar]

@crashkonijn You can perhaps submit a PR with the fix (since you've already managed to fix it on your end :)) and add a unit test to ensure it's really fixed and to also avoid regression later.

[crashkonijn]

Hi there @brunogaspar,

I will give it a try as soon as I have time. The reason I didn't yet do it because I don't understand exactly what '$this->container->call([$this, 'rules'])' does, and why it wasn't was '$this->getValidatorInstance()->getRules()' to begin with.

Anyway, I guess that I'd then also have to add the getRules() function to 'Illuminate\Contracts\Validation\Validator' right?

[tautvydasr]

@crashkonijn

It is done in this way $this->container->call([$this, 'rules']) and not in that $this->getValidatorInstance()->getRules() because the first approach provides you a way to inject services registered in container to rules method.

[crashkonijn]

Hi @tautvydasr,

Thanks for your explanation! If I understand you correctly then my fix will introduce other problems right?

[brunogaspar]

Honestly, i'm not sure it matters much because in the end what the validated() is supposed to do, is to return the name of the "fields" that were under validation, it doesn't need to resolve any dependencies at that point (i think hehe), since those were already resolved when it validated the data, but, it would better to write a test or two to ensure it does what's expected.

But i can be wrong though :)

[tautvydasr]

But potentially if $this->container->call([$this, 'rules']) would be removed it can be a breaking change. Validated fields can relate on service injected to rules and it's logic, what I am talking about:

public function rules(RulesResolver $resolver)
{
    $rules = [
        'email' => 'required|email',
    ];
    
    if ($resolver->isSomething()) {
        $rules['username'] = 'required';
    }
    
    return $rules;
}

Edited:

But as I see when creating default validator resolved rules should be passed so I guess provided solution can be valid.

protected function createDefaultValidator(ValidationFactory $factory)
{
    return $factory->make(
        $this->validationData(), $this->container->call([$this, 'rules']),
        $this->messages(), $this->attributes()
    );
}

[deleugpn]

Easy PR since the author did practically 90% of the work. It's just a matter of writing tests now.

[ttsuru]

This issue is fixed😀

[crashkonijn]

Awesome, thanks!