Re: Re: Problems with the fix for the BC break introduced in 5.4.29 and 5.5.13

Hi! > Another topic would be to that internal classes (not implementing > Serializable) can still be instantiated without the constructor call > through the unserialize method (using the "O:" format), and we can't > prohibit that, as it is/can be perfectly normal to unserialize an > internal class previously properly instantiated and serialize, but we > don't have the knowledge in the class to tell what is a properly > serialized string, and what isn't for that given class. Ah, I was missing this part. I thought those objects go through C, but now I see they can go through O also. This means we can't really ban internal classes in O:. But can we see if the serializer is zend_user_serialize (in which case it's probably OK to allow internal class or user class) or some other function (in which case allowing internal class is probably not a good idea)? Do we still have problems or segfaults in this case? > I think that it would be a nice if we could add more validation for the > internal classes __wakeup()/unserialize() methods for data which > presence is mandatory for the class to be able to work, which would ofc. Some classes just don't allow being serialized/unserialized at all, and use serialize handlers to do that. Trying to run them through unserialize in a roundabout way is very dangerous since they would assume nobody does that. > bother the life of the phpunit/doctrine users/devs, but it would only > prevent the mocking those classes which would be unstable/dangerous > previously when instantiated without the constructor call. I guess this is where I'm trying to get - banning unserializing of the classes which are unsafe (i.e. O: in internal class while having serialize handler) while not banning cases that we can allow. So I wonder here if additional check for zend_user_serialize would help? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227