FriendsOfCake / cakephp-upload

CakePHP: Handle file uploading sans ridiculous automagic
https://cakephp-upload.readthedocs.io/
MIT License
551 stars 255 forks source link

"Call to a member function getError() on string" when updating an existing row #535

Closed AntoineRichard4137 closed 4 years ago

AntoineRichard4137 commented 4 years ago

Hello, I use the cake-4.x branch of the plugin, and I encountered an error when trying to update other fields of a record where a file is already loaded. When saving the entity, the beforeSave function of the UploadBehavior is called, and the error says

Call to a member function getError() on string

showing the line 103 of the behavior : if ($entity->get($field)->getError() !== UPLOAD_ERR_OK) {

This specific line is a mystery to me, as the EntityInterface of CakePHP says that the getError() method is supposed to take the field (string) as a parameter. Moreover, it seems like the get() method returns the value of the field, which is mixed (can be a string, an array...) In my case, the getError() method is being called on a string (the name of the file that is currently loaded). As it should be called on an Entity, it throws an error.

I changed the line 103 to the following, which seems more logical to me : if ($entity->getError($field) !== UPLOAD_ERR_OK) { and the error disappeared.

What am I missing ? Is it a bug ?

saeideng commented 4 years ago
$entity->getError($field

returns message of an error not status of error on upload => (UPLOAD_ERR_OK ,....)

What am I missing ? Is it a bug ?

yes this is a bug /incorrect use of getError()

saeideng commented 4 years ago

but if $entity->get($field) returns Psr\Http\Message\UploadedFileInterface you can use ->getError() on it so if ($entity->get($field)->getError() !== UPLOAD_ERR_OK) { is OK when $entity->get($field) returns UploadedFileInterface not string

AntoineRichard4137 commented 4 years ago

Oh okay, I just realized that the getError() method of the UploadedFileInterface is completely different from the EntityInterface one. Would this issue be securely solvable by adding a condition that checks if $entity->get($field) is an Psr\Http\Message\UploadedFileInterface in the beforeSave call of the UploadBehavior ? Thank you for your help.

I guess it would look like this :

if (!is_a($entity->get($field), 'Psr\Http\Message\UploadedFileInterface')) {
    continue;
}
mortinp commented 4 years ago

Hello there, I am having this same problem.

I took a look inside the plugin and realized the implementation for cake-4.x isn't complete as Josegonzalez\Upload\Database\Type\FileType's marshal function NEEDS to return a Psr\Http\Message\UploadedFileInterface, which it isn't right now. It will also need other adjustments.

Are you working on this? Does the plugin already works for 4.x somehow or am I missing something?

Should we go ahead and try to fix this, complete the plugin implementation and create a pull request? (Only if you are not already working on this of course)

Thanks!

ADmad commented 4 years ago

isn't complete as Josegonzalez\Upload\Database\Type\FileType's marshal function NEEDS to return a Psr\Http\Message\UploadedFileInterface, which it isn't right now

FileType::marshal() simply returns back the value it receives as argument and that's enough as the incoming value should already be UploadedFileInterface instance.

mortinp commented 4 years ago

You're right!

What I was missing was to add ['type' => 'file'] when creating the form in addition to putting it in the field control. Like:

$this->Form->create($model, ['type'=>'file']); $this->Form->control('photo', ['type'=>'file']);

You will need BOTH so CakePHP converts the file to an UploadedFile internally. All working just fine now.

Thanks a lot!

gringlas commented 4 years ago

Oh okay, I just realized that the getError() method of the UploadedFileInterface is completely different from the EntityInterface one. Would this issue be securely solvable by adding a condition that checks if $entity->get($field) is an Psr\Http\Message\UploadedFileInterface in the beforeSave call of the UploadBehavior ? Thank you for your help.

I guess it would look like this :

if (!is_a($entity->get($field), 'Psr\Http\Message\UploadedFileInterface')) {
    continue;
}

I'm having the same issue. You're suggestion @KlectikDev is working for me.