ObjectProperty Hydrator should only hydrate public properties (fix + new test) #6869
Conversation
…nt invalid data being added
…th modification to 1 property to make protected)
| } | ||
|
|
||
| /** | ||
| * @expectedException BadMethodCallException |
There was a problem hiding this comment.
Please use $this->setExpectedException() instead.
… to setExpectedException instead of annotation
|
Conversation with @Ocramius in IRC led to find that to prevent the random addition of non existent properties to objects the way forward would be to use a FilterComposite to remove items that didn't exist prior to hydration. I've modified the Hydrator to now just exclude private/protected properties to prevent fatal errors occurring in these situations. Have removed the final test in the new testcase as was no longer applicable to the outcome. |
|
I'd assume that many people (including myself) used the ObjectProperty hydrator to hydrate and extract protected and private properties. Changing this would break BC. Making it an option, defaulting to the prior behaviour, is probably the best way to go right now. |
Right now it will just crash (current impl). |
| @@ -68,6 +68,15 @@ public function hydrate(array $data, $object) | |||
| '%s expects the provided $object to be a PHP object)', __METHOD__ | |||
| )); | |||
| } | |||
|
|
|||
| $reflection = new ReflectionObject($object); | |||
There was a problem hiding this comment.
Checking this at every hydration is going to kill performance badly. Needs to be a bit smarter
|
Ah nevermind, I'm actually using the ReflectionHydrator for that, ignore me ;) |
|
|
||
| $propertyList = (array)$object; | ||
| $privatePropertyPrefix = "\0" . get_class($object) . "\0"; | ||
| $protectedPropertyPrefix = "\0*\0"; |
There was a problem hiding this comment.
You can just look for any property starting with "\0": that is enough to decide to skip them.
There was a problem hiding this comment.
wouldn't work as the array has the \0 version, not the property in
iteration
On 22 Nov 2014 07:56, "Marco Pivetta" [email protected] wrote:
In library/Zend/Stdlib/Hydrator/ObjectProperty.php:
@@ -68,8 +68,20 @@ public function hydrate(array $data, $object)
'%s expects the provided $object to be a PHP object)', METHOD
));
}
+
$propertyList = (array)$object;$privatePropertyPrefix = "\0" . get_class($object) . "\0";$protectedPropertyPrefix = "\0*\0";You can just look for any property starting with "\0": that is enough to
decide to skip them.—
Reply to this email directly or view it on GitHub
https://github.com/zendframework/zf2/pull/6869/files#r20755730.
There was a problem hiding this comment.
Ah, good point. I would still fill every array key with a value (true is fine) to avoid having calls to array_key_exists()
|
Further conversations with @Ocramius has lead to the issue that private & protected properties can be unset, which the array casting will not catch, leading to the fatal error again. Trying different solutions around this, but most likely the only solution is use reflections in the property cache. This doesn't seem to degrade the speed as much as it could do. |
… unset properties which then don't get cast to array
…tName()` over direct `ReflectionProperty#$name` access
|
@BinaryKitten I've tweaked and merged this, thanks! |
…tAttributeSame` for strict checks. Minor CS, method naming and docblock fixes
…uced properties on generic userland classes
…osedly protected/private members
…t be hydrated by the `ObjectProperty` hydrator
…t be hydrated by the `ObjectProperty` hydrator
…t be extracted by the `ObjectProperty` hydrator
…sing `ReflectionProperty#getName()` over direct `ReflectionProperty#$name` access
…he` can be static
…ache`/`ObjectProperty#$skippedPropertiesCache`
…-should-only-hydrate-public-properties' Close zendframework/zendframework#6869
…-should-only-hydrate-public-properties' into develop Close zendframework/zendframework#6869 Forward port zendframework/zendframework#6869
According to the docs, the Zend\Stdlib\Hydrator\ObjectProperty hydrator hydrates "Any data key matching a publicly accessible property will be hydrated; any public properties will be used for extraction."
No Test was found directly for this Hydrator and the Hydrator itself would attempt to set private or protected properties resulting in a fatal error.
To this end, I have:
I hope that this meets guidelines for this.