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

5.7.14 -> 5.7.19 FormRequest throws validator error #27073

Closed
gskgrek opened this issue Jan 5, 2019 · 12 comments
Closed

5.7.14 -> 5.7.19 FormRequest throws validator error #27073

gskgrek opened this issue Jan 5, 2019 · 12 comments

Comments

@gskgrek
Copy link

gskgrek commented Jan 5, 2019

  • Laravel Version: 5.7.19
  • PHP Version: 7.2.12
  • Database Driver & Version: mariaDB 10.1.37

Description:

After update from 5.7.14 to 5.7.19 all my FormRequest validations throw

Call to a member function validated() on null

error.
After returning to 5.7.14 validation works ok.

Steps To Reproduce:

BrandStoreRequest (extends FormRequest):

public function authorize()
   {
       return true;
   }
   public function rules()
   {
       return [
           'brands_groups_id' => 'required|integer|min:1',
           'name' => 'required|string',
           'sender_email' => 'email|nullable',
           'sender_name' => 'string|nullable',
       ];
   }

BrandsController (extends default created Contreoller):

 public function store(Request $request){
        $request = BrandStoreRequest::createFrom($request);
        $request->setContainer(app());
        $data = $request->validated();
        ...
}
@devcircus
Copy link
Contributor

Looks like your code is taking advantage of a bug that existed before 5.7.16.

@gskgrek
Copy link
Author

gskgrek commented Jan 5, 2019

@devcircus you may be right :D
If i call getValidatorInstance() before validated() it works.
I think that validated() method should check if validator attribute is null and if so call getValidatorInstance() to create it :>

@koenhoeijmakers
Copy link
Contributor

The PR that affects your code is probably #26604, is there a reason why you inject a normal Request instance instead of your BrandStoreRequest?

@mazen23
Copy link

mazen23 commented Jan 6, 2019

Hello, the whole app crashed for the same reason!
Which version shall i revert to?

@mazen23
Copy link

mazen23 commented Jan 6, 2019

The PR that affects your code is probably #26604, is there a reason why you inject a normal Request instance instead of your BrandStoreRequest?

Even if BrandStoreRequest was injected instead of normal request the same error gonna show up dunno why!

@koenhoeijmakers
Copy link
Contributor

The version that "broke" your application is 5.7.16 i suppose, so you should be able to downgrade to 5.7.15 to get your application to work again.

Can you show your controller and request so we can have a look at reproducing?

@gskgrek
Copy link
Author

gskgrek commented Jan 6, 2019

The PR that affects your code is probably #26604, is there a reason why you inject a normal Request instance instead of your BrandStoreRequest?

Routing changes often, part of it is managed by user in panel.
I have dynamic routing (i.e. /{slug}/create -> check if slug is available -> determine controller name by slug -> call create method if exists (passing Request) or return 404 if slug, controller or method doesn't exist)

@gskgrek
Copy link
Author

gskgrek commented Jan 6, 2019

The PR that affects your code is probably #26604, is there a reason why you inject a normal Request instance instead of your BrandStoreRequest?

Even if BrandStoreRequest was injected instead of normal request the same error gonna show up dunno why!

Didn't tried typical path Route::post(path, controller). Probably in that case it works fine (fix was merged to master laravel branch so it passed tests :P)

@gskgrek
Copy link
Author

gskgrek commented Jan 6, 2019

The version that "broke" your application is 5.7.16 i suppose, so you should be able to downgrade to 5.7.15 to get your application to work again.

Can you show your controller and request so we can have a look at reproducing?

Yes, after downgrade to 5.7.15 everything works fine.
Routing:

 Route::post('/{slug}/create', function($slug, Request $request){
            $controllerName = '\App\Http\Controllers\Pages\\' . str_replace('-', '', ucwords($slug, '-')).'Controller';
            // some checks have been removed for the purpose of the example.
            $controller = app()->make($controllerName);
             return $controller->callAction('store', $parameters = array($request));
        })

BrandsController:

namespace App\Http\Controllers\Pages;

use App\Http\Requests\BrandStoreRequest;
use App\Models\Brand;
use Illuminate\Http\Request;
use Illuminate\Routing\Controller

class BrandsController extends Controller
{
         use AuthorizesRequests, DispatchesJobs, ValidatesRequests;

        public function store(Request $request){
        
                $request = BrandStoreRequest::createFrom($request);
                $request->setContainer(app());
                $data = $request->validated(); // this line throws error: Call to a member function validated() on null
                
                $single = new Brand();
                // removed rest of code (save and return)
        }
}

BrandStoreRequest:

namespace App\Http\Requests;

use Illuminate\Contracts\Container\Container;
use Illuminate\Foundation\Http\FormRequest;

class BrandStoreRequest extends FormRequest
{
    public function authorize()
        {
            return true;
        }

    public function rules()
        {
            return [
               'brands_groups_id' => 'required|integer|min:1',
                'name' => 'required|string',
                'sender_email' => 'email|nullable',
                'sender_name' => 'string|nullable',
            ];
        }
}

I've made quick fix for myself by adding to BrandStoreRequest:

public function setContainer(Container $container)
    {
        parent::setContainer($container);
        parent::getValidatorInstance();

        return $this;
    }

@koenhoeijmakers
Copy link
Contributor

koenhoeijmakers commented Jan 6, 2019

As devcircus already mentioned, you were relying on an existing bug which was that new validation instances were being created again and again.

Because you're not injecting the request, the FormRequestServiceProvider isn't going to "boot" the request and validate it, so you'd probably have to mimic what FormRequestServiceProvider is doing, which you're partly doing with ->setContainer().

How you can fix this is:
Add $request->validateResolved() which will throw a validation exception when the validation fails.
Or you could indeed call $request->getValidatorInstance() first as you're already doing.

@driesvints
Copy link
Member

As others have said above you'll probably need to make a few changes to adopt the new behavior.

@MambetniyazovAmir
Copy link

MambetniyazovAmir commented Oct 8, 2022

$companyRequest = new CompanyRequest($request->get('company'));
return $companyRequest->validated();

Although in CompanyRequest added

public function setContainer(Container $container)
{
    parent::setContainer($container);
    parent::getValidatorInstance();
    return $this;
}

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

No branches or pull requests

6 participants