Fixes GH-8152: add __serialise and __unserialise methods to DateTime and DateTimeZone #8422
Conversation
| @@ -3407,19 +3501,19 @@ static zend_result timezone_initialize(php_timezone_obj *tzobj, const char *tz, | |||
| if (strlen(tz) != tz_len) { | |||
| php_error_docref(NULL, E_WARNING, "Timezone must not contain null bytes"); | |||
There was a problem hiding this comment.
Is there a test to see if a crafted serialisation will trigger this warning?
There was a problem hiding this comment.
I don't think so, but will check and add one if needed.
| switch (purpose) { | ||
| case ZEND_PROP_PURPOSE_DEBUG: | ||
| case ZEND_PROP_PURPOSE_SERIALIZE: | ||
| case ZEND_PROP_PURPOSE_VAR_EXPORT: | ||
| case ZEND_PROP_PURPOSE_JSON: | ||
| case ZEND_PROP_PURPOSE_ARRAY_CAST: | ||
| break; | ||
| default: | ||
| return zend_std_get_properties_for(object, purpose); | ||
| } | ||
|
|
There was a problem hiding this comment.
An idea for later, it seems similar code to this is present in other functions, maybe extract this into it's own function?
There was a problem hiding this comment.
It is something you need in nearly every serialise handler, not only in the Date/Time ones. It's a little tricky to extract, as it has both a break (do nothing) and has to return something. I'll make a note and see if this can be done better at some point.
ext/date/php_date.c
Outdated
| dateobj = Z_PHPDATE_P(object); | ||
|
|
||
| array_init(return_value); | ||
| myht = Z_ARRVAL_P(return_value); | ||
| date_object_to_hash(dateobj, myht); |
There was a problem hiding this comment.
This can crash if the __constructor was not called:
class D extends DateTime { public function __construct() {} }
serialize(new D());
There was a problem hiding this comment.
I don't think it can, as the __serialize handler for "D" does not call the __serialize handler for "DateTime".
However, the following does crash:
<?php
class D extends DateTime {
public function __construct() {}
public function __serialize() : array {
return parent::__serialize();
}
}
echo serialize(new D());
Will add a guard, good catch!
|
This works nicely with symfony/var-exporter, thanks! |
97db8b3 to
6f2c501
Compare
…icolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [VarExporter] Fix exporting DateTime objects on PHP 8.2 | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Needs php/php-src#8422 Commits ------- a09e2b4 [VarExporter] Fix exporting DateTime objects on PHP 8.2
This adds the __serialize() and unserialize() methods to DateTime, DateTimeImmutable, and DateTimeZone for PHP 8.2 and later.
This specific PR does not touch on DateInterval and DatePeriod yet, because these cases are more complicated. I will make a separate PR, which might need to end up breaking BC (albeit for functionality that didn't work as expected anyway).
/cc @nicolas-grekas