How to save images in cakephp 3.4.0 with cakephp-upload while being immutable

Question

I'm working on my cakephp project and I am currently upgrading from 3.3.16 to 3.4.0

The project uses the cakephp-upload plugin to save an image. The Upload plugin needs an existing entity to attach a file to it. A modification of the request is done to grab the avatar, before unsetting it to save the user.

I know this is not a good practice to modify a request, but the code was made this way. With immutable objects in version 3.4.0, it is just not possible anymore. But i dont know how to do it properly.

Here is the error message given by my unit-test, ran by vendor/bin/phpunit --filter testAdd tests/TestCase/Controller/Api/V1/UsersControllerTest.php:

There was 1 failure:

1) App\Test\TestCase\Controller\Api\V1\UsersControllerTest::testAdd
Failed asserting that file "/home/comptoir/Comptoir-srv/webroot/img/files/Users/photo/5/avatar/correctAvatarLogo.jpg" exists.

/home/comptoir/Comptoir-srv/tests/TestCase/Controller/Api/V1/UsersControllerTest.php:208

Here is the actual code:

public function add()
    {
        if (!empty($this->request->data)) {
            $user = $this->Users->newEntity($this->request->data);
        } else {
            $user = $this->Users->newEntity();
        }

        $message = "";
        // Get the avatar before unset it to save the user.
        // The Upload plugin need an existing entity to attach a file to it.

        if ($this->request->is('post')) {
            if (isset($this->request->data['photo']) && !$user->errors()) {
                $avatar = $this->request->data['photo'];
                $this->request->data['photo'] = "";
            }

            $user = $this->Users->patchEntity($user, $this->request->data);

            if ($this->Users->save($user)) {
                $user = $this->Users->get($user->id, ['contain' => []]);

                isset($avatar) ? $this->request->data['photo'] = $avatar : null;

                $user = $this->Users->patchEntity($user, $this->request->data);

                if ($this->Users->save($user)) {
                    $message = "Success";
                    $this->Flash->success(__d("Forms", "Your are registred on the Comptoir du Libre, welcome !"));
                    if (!$this->request->is('json')) {
                        $this->Auth->setUser($this->Auth->identify());
                        $this->redirect([
                            "prefix" => false,
                            "controller" => "Pages",
                            "language" => $this->request->param("language")
                        ]);
                    }
                } else {
                    $message = "Error";
                }
            } else {
                $message = "Error";
                $this->Flash->error(__d("Forms", "Your registration failed, please follow rules in red."));
            }

            $message == "Error" ? $this->set('errors', $user->errors()) : null;
        }


        $this->ValidationRules->config('tableRegistry', "Users");
        $rules = $this->ValidationRules->get();
        $userTypes = $this->Users->UserTypes->find('list', ['limit' => 200]);
        $this->set(compact('user', 'userTypes', 'rules', 'message'));
        $this->set('_serialize', ['user', 'userTypes', 'rules', 'message', 'errors']);
    }

Does anyone know how to do that respecting the immutable rule ?

Answer 1

Your premise is wrong.

The Upload plugin needs an existing entity to attach a file to it

That's actually not correct, uploading files alongside creating new records works fine. There's no need for this stuff in your controller, it should be possible to handle this with a single basic save, ie you should investigate the problem that you're having with that, and fix it.

However looking at your test, it should fail anyways, because the file data that you're passing is invalid, it's neither an actual uploaded file for which is_uploaded_file() would return true, nor is it acceptable for user data to be able to define the temporary file path, and the error code, ie you're not properly validating the data if that test passes as is. Accepting such data is a security vulnerability, it could allow all sorts of attacks, from path traversal to arbitrary file injections!

Ideally your whole upload validation and writing functionality would support \Psr\Http\Message\UploadedFileInterface objects, that would allow for very simply testing by being able to pass instances of that class into the test data, that might be something worth suggesting for the plugin. Without such functionality, your second best bet would probably be something like modifying the table's validation rules before issuing the test request, so that is_uploaded_file() is being skipped, or you're switching to integration tests over HTTP, instead of the simulation in CakePHP.