Skip to content

Commit a2f8c78

Browse files
committed
Fix #74940: DateTimeZone loose comparison always true
Since `DateTimeZone` does not implement a `compare_objects` handler, nor has any properties, two `DateTimeZone` instances always compare as being equal, even if they designate totally different timezones. Even worse, after calling `var_dump()` on these objects, the actual comparison may yield a correct result. We therefore introduce a `compare_objects` handlers, which prevents different behavior before/after `var_dump()`, and which allows us to clearly define the intended semantics.
1 parent 6f8045c commit a2f8c78

File tree

3 files changed

+78
-43
lines changed

3 files changed

+78
-43
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ PHP NEWS
1313
- Date:
1414
. Fixed bug #79396 (DateTime hour incorrect during DST jump forward). (Nate
1515
Brunette)
16+
. Fixed bug #74940 (DateTimeZone loose comparison always true). (cmb)
1617

1718
- FPM:
1819
. Implement request #77062 (Allow numeric [UG]ID in FPM listen.{owner,group})

ext/date/php_date.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,8 @@ static zval *date_period_read_property(zval *object, zval *member, int type, voi
680680
static zval *date_period_write_property(zval *object, zval *member, zval *value, void **cache_slot);
681681
static zval *date_period_get_property_ptr_ptr(zval *object, zval *member, int type, void **cache_slot);
682682

683+
static int date_object_compare_timezone(zval *tz1, zval *tz2);
684+
683685
/* {{{ Module struct */
684686
zend_module_entry date_module_entry = {
685687
STANDARD_MODULE_HEADER_EX,
@@ -2152,6 +2154,7 @@ static void date_register_classes(void) /* {{{ */
21522154
date_object_handlers_timezone.offset = XtOffsetOf(php_timezone_obj, std);
21532155
date_object_handlers_timezone.free_obj = date_object_free_storage_timezone;
21542156
date_object_handlers_timezone.clone_obj = date_object_clone_timezone;
2157+
date_object_handlers_timezone.compare_objects = date_object_compare_timezone;
21552158
date_object_handlers_timezone.get_properties_for = date_object_get_properties_for_timezone;
21562159
date_object_handlers_timezone.get_gc = date_object_get_gc_timezone;
21572160
date_object_handlers_timezone.get_debug_info = date_object_get_debug_info_timezone;
@@ -2380,6 +2383,31 @@ static zend_object *date_object_clone_timezone(zval *this_ptr) /* {{{ */
23802383
return &new_obj->std;
23812384
} /* }}} */
23822385

2386+
static int date_object_compare_timezone(zval *tz1, zval *tz2) /* {{{ */
2387+
{
2388+
php_timezone_obj *o1, *o2;
2389+
2390+
o1 = Z_PHPTIMEZONE_P(tz1);
2391+
o2 = Z_PHPTIMEZONE_P(tz2);
2392+
2393+
ZEND_ASSERT(o1->initialized && o2->initialized);
2394+
2395+
if (o1->type != o2->type) {
2396+
php_error_docref(NULL, E_WARNING, "Trying to compare different kinds of DateTimeZone objects");
2397+
return 1;
2398+
}
2399+
2400+
switch (o1->type) {
2401+
case TIMELIB_ZONETYPE_OFFSET:
2402+
return o1->tzi.utc_offset == o2->tzi.utc_offset ? 0 : 1;
2403+
case TIMELIB_ZONETYPE_ABBR:
2404+
return strcmp(o1->tzi.z.abbr, o2->tzi.z.abbr) ? 1 : 0;
2405+
case TIMELIB_ZONETYPE_ID:
2406+
return strcmp(o1->tzi.tz->name, o2->tzi.tz->name) ? 1 : 0;
2407+
EMPTY_SWITCH_DEFAULT_CASE();
2408+
}
2409+
} /* }}} */
2410+
23832411
static void php_timezone_to_string(php_timezone_obj *tzobj, zval *zv)
23842412
{
23852413
switch (tzobj->type) {

ext/date/tests/DateTimeZone_compare_basic1.phpt

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,60 @@
11
--TEST--
2-
Test of compare object handler for DateTime objects
2+
Test of compare object handler for DateTimeZone objects
33
--FILE--
44
<?php
55

6-
// NB: DateTimeZone class does not define a customized compare class handler so standard object handler will be used
6+
$timezones = array(
7+
['+0200', '-0200'],
8+
['EST', 'PST'],
9+
['Europe/Amsterdam', 'Europe/Berlin']
10+
);
711

8-
echo "Simple test for DateTimeZone compare object handler\n";
9-
10-
//Set the default time zone
11-
date_default_timezone_set("Europe/London");
12-
13-
class DateTimeZoneExt1 extends DateTimeZone {
14-
}
15-
16-
class DateTimeZoneExt2 extends DateTimeZone{
17-
public $foo = "Hello";
18-
private $bar = 99;
12+
foreach ($timezones as [$timezone1, $timezone2]) {
13+
compare_timezones($timezone1, $timezone1);
14+
compare_timezones($timezone1, $timezone2);
1915
}
2016

21-
class DateTimeZoneExt3 extends DateTimeZoneExt2 {
17+
var_dump(new DateTimeZone('Europe/Berlin') == new DateTimeZone('CET'));
18+
19+
function compare_timezones($timezone1, $timezone2)
20+
{
21+
$tz1 = new DateTimeZone($timezone1);
22+
$tz2 = new DateTimeZone($timezone2);
23+
echo "compare $timezone1 with $timezone2\n";
24+
echo "< ";
25+
var_dump($tz1 < $tz2);
26+
echo "= ";
27+
var_dump($tz1 == $tz2);
28+
echo "> ";
29+
var_dump($tz1 > $tz2);
2230
}
2331

24-
$obj1 = new DateTimeZone("Europe/London");
25-
$obj2 = new DateTimeZoneExt1("Europe/London");
26-
$obj3 = new DateTimeZoneExt2("Europe/London");
27-
$obj4 = new DateTimeZoneExt3("Europe/London");
28-
29-
echo "\n-- All the following tests should compare equal --\n";
30-
var_dump($obj1 == $obj1);
31-
echo "\n-- All the following tests should compare NOT equal --\n";
32-
var_dump($obj1 == $obj2);
33-
var_dump($obj1 == $obj3);
34-
var_dump($obj1 == $obj4);
35-
var_dump($obj2 == $obj3);
36-
var_dump($obj2 == $obj4);
37-
var_dump($obj3 == $obj4);
38-
3932
?>
40-
===DONE===
41-
--EXPECT--
42-
Simple test for DateTimeZone compare object handler
43-
44-
-- All the following tests should compare equal --
45-
bool(true)
46-
47-
-- All the following tests should compare NOT equal --
48-
bool(false)
49-
bool(false)
50-
bool(false)
51-
bool(false)
52-
bool(false)
33+
--EXPECTF--
34+
compare +0200 with +0200
35+
< bool(false)
36+
= bool(true)
37+
> bool(false)
38+
compare +0200 with -0200
39+
< bool(false)
40+
= bool(false)
41+
> bool(false)
42+
compare EST with EST
43+
< bool(false)
44+
= bool(true)
45+
> bool(false)
46+
compare EST with PST
47+
< bool(false)
48+
= bool(false)
49+
> bool(false)
50+
compare Europe/Amsterdam with Europe/Amsterdam
51+
< bool(false)
52+
= bool(true)
53+
> bool(false)
54+
compare Europe/Amsterdam with Europe/Berlin
55+
< bool(false)
56+
= bool(false)
57+
> bool(false)
58+
59+
Warning: main(): Trying to compare different kinds of DateTimeZone objects in %s on line %d
5360
bool(false)
54-
===DONE===

0 commit comments

Comments
 (0)