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

[11.x] Fix validated method #50319

Merged
merged 1 commit into from
Mar 4, 2024
Merged

Conversation

nshiro
Copy link
Contributor

@nshiro nshiro commented Mar 1, 2024

If we manually create a validator instance and use the validated or safe method without validating first, there is a chance that we will get incorrect data.

Below is an example.

Route::post('/', function (Request $request) {
    $validator = Validator::make($request->all(), [
        'name' => ['required', 'string'],
        'address' => ['required', 'string'],
    ]);

    $data = $validator->safe()->all();

    dd($data);
});

There is no passes or validate method above, but still this somehow works as people expect, because the framework checks if there are any validation errors.

But this validated method has a little problem because the invalid method that the validated method uses only returns the data that people actually send.

For example, in the code above, you will still get data even if some bad guy doesn't send the name field at all.

This is more of a bug fix, but I'm sending this PR to master because if you already have potential bugs, this PR affects them.

Example.

Route::post('/', function (Request $request) {
    $validator = Validator::make($request->all(), [
        'name' => ['required', 'string'],
        'address' => ['required', 'string'],
        'field_people_never_send' => ['required', 'string'],   // This is  a bug.
    ]);

    $data = $validator->safe()->all();

    dd($data);
});

@taylorotwell taylorotwell merged commit a3cf850 into laravel:master Mar 4, 2024
30 checks passed
@nshiro nshiro deleted the fix_validated_11 branch March 4, 2024 21:49
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 this pull request may close these issues.

2 participants