The result of calling getTimezone() on a Date object results in a 
DateTimeZone object with a reference to the dateobj->time->tz_info 
object which may get later destroyed.

This can cause unexpected script behaviour or interpreter crashes, test 
case in attachment (1).

When I fixed this the obvious way, per attachment 3, by adding a clone, 
I wondered where the cloned tzinfo structures would get destroyed and I 
can't see anywhere.

After looking at this further - so far as I can tell, the duplication of 
the tzinfo structures throughout this code is not actually necessary; 
when the structures are created they are referenced from the global 
tzcache and will hence last "forever" anyway.  The structures are not 
changed anywhere either, again AFAICT; though they aren't treated as 
const so maybe I'm missing something there.

So simply copying pointers around would simplify the code, fix leaks and 
fix the bug as well.  That's attachment (2).

What do you think?

Regards, Joe
Index: ext/date/tests/015.phpt
===================================================================
RCS file: ext/date/tests/015.phpt
diff -N ext/date/tests/015.phpt
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ ext/date/tests/015.phpt     8 Jul 2008 14:35:51 -0000
@@ -0,0 +1,22 @@
+--TEST--
+timezone object reference handling
+--INI--
+date.timezone=UTC
+--FILE--
+<?php
+$dto = new DateTime();
+$tzold = $dto->getTimezone();
+var_dump($tzold->getName());
+$dto->setTimezone(new DateTimeZone('US/Eastern'));
+var_dump($tzold->getName());
+var_dump($dto->getTimezone()->getName());
+unset($dto);
+var_dump($tzold->getName());
+echo "Done\n";
+?>
+--EXPECTF--
+unicode(3) "UTC"
+unicode(3) "UTC"
+unicode(10) "US/Eastern"
+unicode(3) "UTC"
+Done
Index: ext/date/php_date.c
===================================================================
RCS file: /repository/php-src/ext/date/php_date.c,v
retrieving revision 1.189
diff -u -r1.189 php_date.c
--- ext/date/php_date.c 7 Jun 2008 22:41:02 -0000       1.189
+++ ext/date/php_date.c 8 Jul 2008 14:21:35 -0000
@@ -1341,14 +1341,6 @@
        timelib_update_ts(t, tzi);
        ts = timelib_date_to_int(t, &error2);
 
-       /* if tz_info is not a copy, avoid double free */
-       if (now->tz_info != tzi && now->tz_info) {
-               timelib_tzinfo_dtor(now->tz_info);
-       }
-       if (t->tz_info != tzi) {
-               timelib_tzinfo_dtor(t->tz_info);
-       }
-
        timelib_time_dtor(now);
        timelib_time_dtor(t);
 
@@ -1779,7 +1771,7 @@
                newdateobj->time->tz_abbr = strdup(it_time->tz_abbr);
        }
        if (it_time->tz_info) {
-               newdateobj->time->tz_info = 
timelib_tzinfo_clone(it_time->tz_info);
+               newdateobj->time->tz_info = it_time->tz_info;
        }
        
        *data = &iterator->current;
@@ -1966,7 +1958,7 @@
                new_obj->time->tz_abbr = strdup(old_obj->time->tz_abbr);
        }
        if (old_obj->time->tz_info) {
-               new_obj->time->tz_info = 
timelib_tzinfo_clone(old_obj->time->tz_info);
+               new_obj->time->tz_info = old_obj->time->tz_info;
        }
        
        return new_ov;
@@ -2185,9 +2177,6 @@
        php_date_obj *intern = (php_date_obj *)object;
 
        if (intern->time) {
-               if (intern->time->tz_info) {
-                       timelib_tzinfo_dtor(intern->time->tz_info);
-               }
                timelib_time_dtor(intern->time);
        }
 
@@ -2220,16 +2209,10 @@
        php_period_obj *intern = (php_period_obj *)object;
 
        if (intern->start) {
-               if (intern->start->tz_info) {
-                       timelib_tzinfo_dtor(intern->start->tz_info);
-               }
                timelib_time_dtor(intern->start);
        }
 
        if (intern->end) {
-               if (intern->end->tz_info) {
-                       timelib_tzinfo_dtor(intern->end->tz_info);
-               }
                timelib_time_dtor(intern->end);
        }
 
@@ -2268,14 +2251,11 @@
        timelib_time   *now;
        timelib_tzinfo *tzi;
        timelib_error_container *err = NULL;
-       int free_tzi = 0, type = TIMELIB_ZONETYPE_ID, new_dst;
+       int type = TIMELIB_ZONETYPE_ID, new_dst;
        char *new_abbr;
        timelib_sll     new_offset;
        
        if (dateobj->time) {
-               if (dateobj->time->tz_info) {
-                       timelib_tzinfo_dtor(dateobj->time->tz_info);
-               }
                timelib_time_dtor(dateobj->time);
        }
        if (format) {
@@ -2303,8 +2283,7 @@
                tzobj = (php_timezone_obj *) 
zend_object_store_get_object(timezone_object TSRMLS_CC);
                switch (tzobj->type) {
                        case TIMELIB_ZONETYPE_ID:
-                               tzi = timelib_tzinfo_clone(tzobj->tzi.tz);
-                               free_tzi = 1;
+                               tzi = tzobj->tzi.tz;
                                break;
                        case TIMELIB_ZONETYPE_OFFSET:
                                new_offset = tzobj->tzi.utc_offset;
@@ -2317,8 +2296,7 @@
                }
                type = tzobj->type;
        } else if (dateobj->time->tz_info) {
-               tzi = timelib_tzinfo_clone(dateobj->time->tz_info);
-               free_tzi = 1;
+               tzi = dateobj->time->tz_info;
        } else {
                tzi = get_timezone_info(TSRMLS_C);
        }
@@ -2345,12 +2323,6 @@
 
        dateobj->time->have_relative = 0;
 
-       if (type == TIMELIB_ZONETYPE_ID && now->tz_info != tzi) {
-               timelib_tzinfo_dtor(now->tz_info);
-       }
-       if (free_tzi) {
-               timelib_tzinfo_dtor(tzi);
-       }
        timelib_time_dtor(now);
 
        return 1;
@@ -2860,10 +2832,7 @@
        if (tzobj->type != TIMELIB_ZONETYPE_ID) {
                php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can only do this 
for zones with ID for now");
        }
-       if (dateobj->time->tz_info) {
-               timelib_tzinfo_dtor(dateobj->time->tz_info);
-       }
-       timelib_set_timezone(dateobj->time, 
timelib_tzinfo_clone(tzobj->tzi.tz));
+       timelib_set_timezone(dateobj->time, tzobj->tzi.tz);
        timelib_unixtime2local(dateobj->time, dateobj->time->sse);
 }
 /* }}} */
@@ -3613,7 +3582,7 @@
                        clone->tz_abbr = strdup(dateobj->time->tz_abbr);
                }
                if (dateobj->time->tz_info) {
-                       clone->tz_info = 
timelib_tzinfo_clone(dateobj->time->tz_info);
+                       clone->tz_info = dateobj->time->tz_info;
                }
                dpobj->start = clone;
 
@@ -3629,7 +3598,7 @@
                                clone->tz_abbr = strdup(dateobj->time->tz_abbr);
                        }
                        if (dateobj->time->tz_info) {
-                               clone->tz_info = 
timelib_tzinfo_clone(dateobj->time->tz_info);
+                               clone->tz_info = dateobj->time->tz_info;
                        }
                        dpobj->end = clone;
                }
Index: ext/date/tests/015.phpt
===================================================================
RCS file: ext/date/tests/015.phpt
diff -N ext/date/tests/015.phpt
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ ext/date/tests/015.phpt     8 Jul 2008 14:21:35 -0000
@@ -0,0 +1,19 @@
+--TEST--
+timezone object reference handling
+--INI--
+date.timezone=UTC
+--FILE--
+<?php
+$dto = new DateTime();
+$tzold = $dto->getTimezone();
+var_dump($tzold->getName());
+$dto->setTimezone(new DateTimeZone('US/Eastern'));
+var_dump($tzold->getName());
+var_dump($dto->getTimezone()->getName());
+echo "Done\n";
+?>
+--EXPECTF--
+unicode(3) "UTC"
+unicode(3) "UTC"
+unicode(10) "US/Eastern"
+Done
Index: php_date.c
===================================================================
RCS file: /repository/php-src/ext/date/php_date.c,v
retrieving revision 1.189
diff -u -r1.189 php_date.c
--- php_date.c  7 Jun 2008 22:41:02 -0000       1.189
+++ php_date.c  8 Jul 2008 14:31:05 -0000
@@ -2824,7 +2824,7 @@
                tzobj->type = dateobj->time->zone_type;
                switch (dateobj->time->zone_type) {
                        case TIMELIB_ZONETYPE_ID:
-                               tzobj->tzi.tz = dateobj->time->tz_info;
+                               tzobj->tzi.tz = 
timelib_tzinfo_clone(dateobj->time->tz_info);
                                break;
                        case TIMELIB_ZONETYPE_OFFSET:
                                tzobj->tzi.utc_offset = dateobj->time->z;

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

Reply via email to