Skip to content

Commit

Permalink
Fixed bug #68406 calling var_dump on a DateTimeZone object modifies it
Browse files Browse the repository at this point in the history
  • Loading branch information
jhdxr authored and krakjoe committed Feb 12, 2018
1 parent f706937 commit 070211b
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 16 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ PHP NEWS
. Fixed bug #75857 (Timezone gets truncated when formatted). (carusogabriel)
. Fixed bug #75928 (Argument 2 for `DateTimeZone::listIdentifiers()` should
accept `null`). (Pedro Lacerda)
. Fixed bug #68406 (calling var_dump on a DateTimeZone object modifies it).
(jhdxr)

- PGSQL:
. Fixed #75838 (Memory leak in pg_escape_bytea()). (ard_1 at mail dot ru)
Expand Down
41 changes: 41 additions & 0 deletions ext/date/php_date.c
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ static HashTable *date_object_get_gc_period(zval *object, zval **table, int *n);
static HashTable *date_object_get_properties_period(zval *object);
static HashTable *date_object_get_properties_timezone(zval *object);
static HashTable *date_object_get_gc_timezone(zval *object, zval **table, int *n);
static HashTable *date_object_get_debug_info_timezone(zval *object, int *is_temp);

zval *date_interval_read_property(zval *object, zval *member, int type, void **cache_slot, zval *rv);
void date_interval_write_property(zval *object, zval *member, zval *value, void **cache_slot);
Expand Down Expand Up @@ -2100,6 +2101,7 @@ static void date_register_classes(void) /* {{{ */
date_object_handlers_timezone.clone_obj = date_object_clone_timezone;
date_object_handlers_timezone.get_properties = date_object_get_properties_timezone;
date_object_handlers_timezone.get_gc = date_object_get_gc_timezone;
date_object_handlers_timezone.get_debug_info = date_object_get_debug_info_timezone;

#define REGISTER_TIMEZONE_CLASS_CONST_STRING(const_name, value) \
zend_declare_class_constant_long(date_ce_timezone, const_name, sizeof(const_name)-1, value);
Expand Down Expand Up @@ -2377,6 +2379,45 @@ static HashTable *date_object_get_properties_timezone(zval *object) /* {{{ */
return props;
} /* }}} */

static HashTable *date_object_get_debug_info_timezone(zval *object, int *is_temp) /* {{{ */
{
HashTable *ht, *props;
zval zv;
php_timezone_obj *tzobj;

tzobj = Z_PHPTIMEZONE_P(object);
props = zend_std_get_properties(object);

*is_temp = 1;
ht = zend_array_dup(props);

ZVAL_LONG(&zv, tzobj->type);
zend_hash_str_update(ht, "timezone_type", sizeof("timezone_type")-1, &zv);

switch (tzobj->type) {
case TIMELIB_ZONETYPE_ID:
ZVAL_STRING(&zv, tzobj->tzi.tz->name);
break;
case TIMELIB_ZONETYPE_OFFSET: {
zend_string *tmpstr = zend_string_alloc(sizeof("UTC+05:00")-1, 0);

ZSTR_LEN(tmpstr) = snprintf(ZSTR_VAL(tmpstr), sizeof("+05:00"), "%c%02d:%02d",
tzobj->tzi.utc_offset > 0 ? '-' : '+',
abs(tzobj->tzi.utc_offset / 60),
abs((tzobj->tzi.utc_offset % 60)));

ZVAL_NEW_STR(&zv, tmpstr);
}
break;
case TIMELIB_ZONETYPE_ABBR:
ZVAL_STRING(&zv, tzobj->tzi.z.abbr);
break;
}
zend_hash_str_update(ht, "timezone", sizeof("timezone")-1, &zv);

return ht;
} /* }}} */

static inline zend_object *date_object_new_interval_ex(zend_class_entry *class_type, int init_props) /* {{{ */
{
php_interval_obj *intern;
Expand Down
32 changes: 16 additions & 16 deletions ext/date/tests/DateTimeZone_clone_basic3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -39,34 +39,30 @@ object(DateTimeZone)#%d (2) {

-- Add some properties --
object(DateTimeZone)#%d (4) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/London"
["property1"]=>
int(99)
["property2"]=>
string(5) "Hello"
}

-- clone it --
object(DateTimeZone)#%d (4) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/London"
}

-- clone it --
object(DateTimeZone)#%d (4) {
["property1"]=>
int(99)
["property2"]=>
string(5) "Hello"
}

-- Add some more properties --
object(DateTimeZone)#%d (6) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/London"
}

-- Add some more properties --
object(DateTimeZone)#%d (6) {
["property1"]=>
int(99)
["property2"]=>
Expand All @@ -75,14 +71,14 @@ object(DateTimeZone)#%d (6) {
bool(true)
["property4"]=>
float(10.5)
}

-- clone it --
object(DateTimeZone)#%d (6) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/London"
}

-- clone it --
object(DateTimeZone)#%d (6) {
["property1"]=>
int(99)
["property2"]=>
Expand All @@ -91,5 +87,9 @@ object(DateTimeZone)#%d (6) {
bool(true)
["property4"]=>
float(10.5)
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/London"
}
===DONE===
34 changes: 34 additions & 0 deletions ext/date/tests/bug68406.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
Bug #68406 calling var_dump on a DateTimeZone object modifies it
--INI--
date.timezone=UTC
--FILE--
<?php

$tz1 = new DateTimeZone('Europe/Berlin');
$tz2 = new DateTimeZone('Europe/Berlin');

$d = new DateTime('2014-12-24 13:00:00', $tz1);
var_dump($d->getTimezone(), $tz2);

if($tz2 == $d->getTimezone()) {
echo "yes";
}
else {
echo "no";
}

--EXPECT--
object(DateTimeZone)#4 (2) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/Berlin"
}
object(DateTimeZone)#2 (2) {
["timezone_type"]=>
int(3)
["timezone"]=>
string(13) "Europe/Berlin"
}
yes

2 comments on commit 070211b

@nikic
Copy link
Member

@nikic nikic commented on 070211b Feb 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change should really be master only. This bug has existed for so long, I'm sure there are people relying on it. I remember seeing some rather dubious code on StackOverflow...

@jhdxr
Copy link
Member Author

@jhdxr jhdxr commented on 070211b Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this is a bug since day 1, and I also run into serveral questions about this, but they are asking why and how to avoid this side effect. I'm curious how people will using this "feature". calling var_dump and retrieve $tz->timezone_type? this value doesn't make sense for userland IMO.

Please sign in to comment.