Uploaded image for project: 'Doctrine Common'
  1. Doctrine Common
  2. DCOM-175

Proxies return private properties in __sleep, which is not supported by PHP.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.5.0
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      __sleep should not return private parent property names (see http://php.net/__sleep) this raises notices, and also results in the value of the property being 'N' (null) instead of keeping its value.

      I am unfortunately stuck having to serialize proxies in my revision tracking, as doctrine seems to be currently ignoring the fetch="EAGER" I set on the related properties.

      Proxies should use the Serializable interface, and not __sleep, or not return parent property names which are private, it serves no purpose, and is not supported by PHP itself anyway.

      Also, if you keep __sleep, but do not return the parent property names, then it will only include the items you return, so it would be better to simply drop the __sleep method, I cannot actually see any useful purpose it serves.

        Activity

        bittarman Ryan Mauger created issue -
        Hide
        bittarman Ryan Mauger added a comment -

        just updated the issue body, realised that I worded something badly.

        Show
        bittarman Ryan Mauger added a comment - just updated the issue body, realised that I worded something badly.
        bittarman Ryan Mauger made changes -
        Field Original Value New Value
        Description __sleep should not return private property names (see http://php.net/__sleep) this raises notices, and also results in the value of the property being 'N' (null) instead of keeping its value.

        I am unfortunately stuck having to serialize proxies in my revision tracking, as doctrine seems to be currently ignoring the fetch="EAGER" I set on the related properties.

        Proxies should use the Serializable interface, and not __sleep, or not return property names which are private, it serves no purpose, and is not supported by PHP itself anyway.
        __sleep should not return private parent property names (see http://php.net/__sleep) this raises notices, and also results in the value of the property being 'N' (null) instead of keeping its value.

        I am unfortunately stuck having to serialize proxies in my revision tracking, as doctrine seems to be currently ignoring the fetch="EAGER" I set on the related properties.

        Proxies should use the Serializable interface, and not __sleep, or not return parent property names which are private, it serves no purpose, and is not supported by PHP itself anyway.

        Also, if you keep __sleep, but do not return the parent property names, then it will only include the items you return, so it would be better to simply drop the __sleep method, I cannot actually see any useful purpose it serves.
        beberlei Benjamin Eberlei made changes -
        Workflow jira [ 13562 ] jira-feedback [ 14036 ]
        beberlei Benjamin Eberlei made changes -
        Workflow jira-feedback [ 14036 ] jira-feedback2 [ 15900 ]
        beberlei Benjamin Eberlei made changes -
        Workflow jira-feedback2 [ 15900 ] jira-feedback3 [ 18155 ]
        Hide
        ocramius Marco Pivetta added a comment -

        Ryan Mauger I think that's a limitation we have. We use `__sleep` to avoid serializing fields like the initializers and the persisters of course.

        Even by implementing serializable, it would only work if the end user implemented it in the parent class.

        Tempted to mark it as "can't fix"

        Show
        ocramius Marco Pivetta added a comment - Ryan Mauger I think that's a limitation we have. We use `__sleep` to avoid serializing fields like the initializers and the persisters of course. Even by implementing serializable, it would only work if the end user implemented it in the parent class. Tempted to mark it as "can't fix"
        ocramius Marco Pivetta made changes -
        Assignee Benjamin Eberlei [ beberlei ] Marco Pivetta [ ocramius ]
        Hide
        ocramius Marco Pivetta added a comment - - edited

        I think there's a solution by having something like following:

        class SomeGeneratedProxyName extends RealName implements \Serializable
        {
            public function unserialize($data)
            {
                $reflectionClass = new ReflectionClass($this);
                foreach ($reflectionClass->getProperties(ReflectionProperty::IS_PRIVATE) as $privateProp) {
                    $privateProp->setAccessible(true);
                    $privateProp->setValue($this, $data[$privateProp->getName()]);
                }
                // ... other props ...
            }
        
            public function serialize()
            {
                $data = array();
                $reflectionClass = new ReflectionClass($this);
                foreach ($reflectionClass->getProperties(ReflectionProperty::IS_PRIVATE) as $privateProp) {
                    $privateProp->setAccessible(true);
                    $data[$privateProp->getName()] = $privateProp->getValue($this);
                }
                // ... other props ...
                return $data;
            }
        }
        
        Show
        ocramius Marco Pivetta added a comment - - edited I think there's a solution by having something like following: class SomeGeneratedProxyName extends RealName implements \Serializable { public function unserialize($data) { $reflectionClass = new ReflectionClass($ this ); foreach ($reflectionClass->getProperties(ReflectionProperty::IS_PRIVATE) as $privateProp) { $privateProp->setAccessible( true ); $privateProp->setValue($ this , $data[$privateProp->getName()]); } // ... other props ... } public function serialize() { $data = array(); $reflectionClass = new ReflectionClass($ this ); foreach ($reflectionClass->getProperties(ReflectionProperty::IS_PRIVATE) as $privateProp) { $privateProp->setAccessible( true ); $data[$privateProp->getName()] = $privateProp->getValue($ this ); } // ... other props ... return $data; } }
        ocramius Marco Pivetta made changes -
        Project Doctrine 2 - ORM [ 10032 ] Doctrine Common [ 10043 ]
        Key DDC-1727 DCOM-175
        Affects Version/s 2.4 [ 10327 ]
        Affects Version/s 2.2.1 [ 10194 ]
        Component/s ORM [ 10012 ]
        ocramius Marco Pivetta made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        ocramius Marco Pivetta added a comment - - edited

        Ryan Mauger I started implementing this one at https://github.com/Ocramius/common/compare/hotfix;DCOM-175 and so far it looks promising.

        The only doubts so far are with handling cases like following:

        class MyEntity implements Serializable
        {
            public function serialize()
            {
                // [...]
            }
        
            public function unserialize($serialized)
            {
                // [...]
            }
        
            public function __sleep()
            {
                // [...]
            }
        
            public function __wakeup()
            {
                // [...]
            }
        }
        

        So far I didn't get to write tests that demonstrate the exact behaviour for this case, but it looks like when `Serializable` is implemented, `_sleep` and `_wakeup` are ignored. Any thoughts on this?

        Show
        ocramius Marco Pivetta added a comment - - edited Ryan Mauger I started implementing this one at https://github.com/Ocramius/common/compare/hotfix;DCOM-175 and so far it looks promising. The only doubts so far are with handling cases like following: class MyEntity implements Serializable { public function serialize() { // [...] } public function unserialize($serialized) { // [...] } public function __sleep() { // [...] } public function __wakeup() { // [...] } } So far I didn't get to write tests that demonstrate the exact behaviour for this case, but it looks like when `Serializable` is implemented, `_ sleep` and ` _wakeup` are ignored. Any thoughts on this?
        Hide
        ocramius Marco Pivetta added a comment -

        A possible solution is to use something like http://eval.in/16806, and thus exploiting the ability of php to retrieve an object's private properties by using the special

        chr(0) . 'Foo' . chr(0) . 'bar'
        

        trick.

        This can be abstracted by using

        array_keys((array) $object);

        , which retrieves also those special keys

        Show
        ocramius Marco Pivetta added a comment - A possible solution is to use something like http://eval.in/16806 , and thus exploiting the ability of php to retrieve an object's private properties by using the special chr(0) . 'Foo' . chr(0) . 'bar' trick. This can be abstracted by using array_keys((array) $object); , which retrieves also those special keys
        Hide
        ocramius Marco Pivetta added a comment -
        Show
        ocramius Marco Pivetta added a comment - Provided a fix at https://github.com/doctrine/common/pull/284
        Show
        ocramius Marco Pivetta added a comment - Solved at https://github.com/doctrine/common/commit/9c6b8615a988117dec83bdbb0fad80afef23eab8
        ocramius Marco Pivetta made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        beberlei Benjamin Eberlei made changes -
        Fix Version/s 2.4 [ 10327 ]
        ocramius Marco Pivetta made changes -
        Fix Version/s 2.5.0 [ 10526 ]
        ocramius Marco Pivetta made changes -
        Fix Version/s 2.4 [ 10327 ]

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={expand=changesets[0:20].revisions[0:29],reviews, query=DCOM-175}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            ocramius Marco Pivetta
            Reporter:
            bittarman Ryan Mauger
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: