Description
After reviewing the documentation, and working with phalcon models. I found an issue using model setters. https://docs.phalconphp.com/en/latest/reference/models.html#public-properties-vs-setters-getters mentions that we can automagically use a method named "set" + camelize(property) to handle setting non-public properties. This however is not what is happening.
Consider the example
class User extends \Phalcon\Mvc\Model
{
protected $id;
public function setId($id) {
echo "Got here.";
$this->id = $id;
}
}
...
$user = new User();
$user->save(['id' => 1], ['id']); // This will print "Got here."
...
$user = User::findFirst(1);
$user->id = 13; // This does assign 13 as the id but does not print "Got here."After digging through the code, I noticed that the model magic __set method made no check for the possible setter, and instead just assigns the property.
phalcon/mvc/model.zep
...
abstract class Model implements EntityInterface, ModelInterface, ResultInterface, InjectionAwareInterface, \Serializable, \JsonSerializable
{
...
public function __set(string property, value)
{
var lowerProperty, related, modelName, manager, lowerKey,
relation, referencedModel, key, item, dirtyState;
...
/**
* Fallback assigning the value to the instance
*/
let this->{property} = value;
return value;
}
...This makes no mention or use of the possible setter.
Furthurmore second to last line completely bypasses the purpose of making a property private or protected, as it will always set the property regardless.
I can see the line in Model::assign() that utilizes the possible setter.
let possibleSetter = "set" . camelize(attributeField);
if method_exists(this, possibleSetter) {
this->{possibleSetter}(value);
} else {
let this->{attributeField} = value;
}I believe this should be refactored so the possible setter is utilized in __set and also fix the issue with __set completely bypassing private and protected properties.
I will provide a proof-of-concept patch in the near future.