[PHP-DEV] Merge #62852 and #53437 into 5.3 and 5.4

2013-03-18 Thread Anatol Belski
Hi,

#62852 and #53437 introduced fixes for the date extension crashing when
unserializing objects. They was applied to 5.5. If there are no
objections, I would backport those to 5.3 and 5.4.

Regards

Anatol

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Merge #62852 and #53437 into 5.3 and 5.4

2013-03-18 Thread Johannes Schlüter
Hi,

commit f8b91d9ac (fix for #62852) looks fine to me.

Commit 0ee71  for #53437 is a bit too much for 5.3 in my opinion - what happens 
if users have custom work-arounds in extended classes with custom __wakeup or 
__set_State methods? Will there be a change of behavihor? I think there should 
be an error message instead of a crash in 5.3. Aside from that: I don't think 
that E_ERROR is a good error level for the caes, either use E_RECOVERABLE or 
even just E_WARNING.

johannes

Anatol Belski  wrote:

>Hi,
>
>#62852 and #53437 introduced fixes for the date extension crashing when
>unserializing objects. They was applied to 5.5. If there are no
>objections, I would backport those to 5.3 and 5.4.
>
>Regards
>
>Anatol
>
>-- 
>PHP Internals - PHP Runtime Development Mailing List
>To unsubscribe, visit: http://www.php.net/unsub.php
>


Re: [PHP-DEV] Merge #62852 and #53437 into 5.3 and 5.4

2013-03-18 Thread Anatol Belski
Johannes,

I completely agree about the E_ERROR - in general. An error should be
recoverable as much as possible. In this concrete case as I've reworking a
patch descending from 2010 and that E_ERROR was there, I just let it go.

The situation is that doing an E_WARNING possibly leaves user with an
invalid object, which will crash on arbitrary places - in method calls,
var_dump or iterator. In 5.5 it is even possible to throw exception in
__wakeup as unserialize cares about it there. But anyway - any recoverable
error would need even more intervention into that code which could
consequently bring further behavior changes.

For #53437 as workaround maybe one could initialize the objects in
DatePeriod (as there are multiple) to some freezed value and put a
warning? That would be more permissive and bring minimal behavior changes.
I could give it a try if you mean it's acceptable. Any solution is better
than a PHP crash I mean :)

Other workaround you've possibly in mind?

Regards

Anatol

On Mon, March 18, 2013 18:03, Johannes Schlüter wrote:
> Hi,
>
>
> commit f8b91d9ac (fix for #62852) looks fine to me.
>
> Commit 0ee71  for #53437 is a bit too much for 5.3 in my opinion - what
> happens if users have custom work-arounds in extended classes with custom
> __wakeup or __set_State methods? Will there be a change of behavihor? I
> think there should be an error message instead of a crash in 5.3. Aside
> from that: I don't think that E_ERROR is a good error level for the caes,
> either use E_RECOVERABLE or even just E_WARNING.
>
> johannes
>
> Anatol Belski  wrote:
>
>
>> Hi,
>>
>>
>> #62852 and #53437 introduced fixes for the date extension crashing when
>>  unserializing objects. They was applied to 5.5. If there are no
>> objections, I would backport those to 5.3 and 5.4.
>>
>> Regards
>>
>>
>> Anatol
>>
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>
>


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php