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.

diff --git a/NEWS b/NEWS
index b087d3a847e1828605a1957b07f50a91eed2c515..40fd08f9b6b845c952819a9f8fad79c333cf4f8a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,7 @@ PHP                                                                        NEWS
 - Date:
   . Fixed bug #79396 (DateTime hour incorrect during DST jump forward). (Nate
     Brunette)
+  . Fixed bug #74940 (DateTimeZone loose comparison always true). (cmb)
 
 - FPM:
   . Implement request #77062 (Allow numeric [UG]ID in FPM listen.{owner,group})

diff --git a/ext/date/php_date.c b/ext/date/php_date.c
index 8c6dfbbe3e510f51b7ffce877f129c3f3401a4ba..02068b44ccb75e5fb4ef16bfe513facb2870524f 100644 (file)
--- a/ext/date/php_date.c
+++ b/ext/date/php_date.c
@@ -680,6 +680,8 @@ static zval *date_period_read_property(zval *object, zval *member, int type, voi
 static zval *date_period_write_property(zval *object, zval *member, zval *value, void **cache_slot);
 static zval *date_period_get_property_ptr_ptr(zval *object, zval *member, int type, void **cache_slot);
 
+static int date_object_compare_timezone(zval *tz1, zval *tz2);
+
 /* {{{ Module struct */
 zend_module_entry date_module_entry = {
        STANDARD_MODULE_HEADER_EX,
@@ -2152,6 +2154,7 @@ static void date_register_classes(void) /* {{{ */
        date_object_handlers_timezone.offset = XtOffsetOf(php_timezone_obj, std);
        date_object_handlers_timezone.free_obj = date_object_free_storage_timezone;
        date_object_handlers_timezone.clone_obj = date_object_clone_timezone;
+       date_object_handlers_timezone.compare_objects = date_object_compare_timezone;
        date_object_handlers_timezone.get_properties_for = date_object_get_properties_for_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;
@@ -2380,6 +2383,31 @@ static zend_object *date_object_clone_timezone(zval *this_ptr) /* {{{ */
        return &new_obj->std;
 } /* }}} */
 
+static int date_object_compare_timezone(zval *tz1, zval *tz2) /* {{{ */
+{
+       php_timezone_obj *o1, *o2;
+
+       o1 = Z_PHPTIMEZONE_P(tz1);
+       o2 = Z_PHPTIMEZONE_P(tz2);
+
+       ZEND_ASSERT(o1->initialized && o2->initialized);
+
+       if (o1->type != o2->type) {
+               php_error_docref(NULL, E_WARNING, "Trying to compare different kinds of DateTimeZone objects");
+               return 1;
+       }
+
+       switch (o1->type) {
+               case TIMELIB_ZONETYPE_OFFSET:
+                       return o1->tzi.utc_offset == o2->tzi.utc_offset ? 0 : 1;
+               case TIMELIB_ZONETYPE_ABBR:
+                       return strcmp(o1->tzi.z.abbr, o2->tzi.z.abbr) ? 1 : 0;
+               case TIMELIB_ZONETYPE_ID:
+                       return strcmp(o1->tzi.tz->name, o2->tzi.tz->name) ? 1 : 0;
+               EMPTY_SWITCH_DEFAULT_CASE();
+       }
+} /* }}} */
+
 static void php_timezone_to_string(php_timezone_obj *tzobj, zval *zv)
 {
        switch (tzobj->type) {

diff --git a/ext/date/tests/DateTimeZone_compare_basic1.phpt b/ext/date/tests/DateTimeZone_compare_basic1.phpt
index 790bb6cf7f050fc2872da89c56e330f181c38b5a..b4e8d76cc2deb4d140daf153f87e169c9fedaffa 100644 (file)
--- a/ext/date/tests/DateTimeZone_compare_basic1.phpt
+++ b/ext/date/tests/DateTimeZone_compare_basic1.phpt
@@ -1,54 +1,60 @@
 --TEST--
-Test of compare object handler for DateTime objects
+Test of compare object handler for DateTimeZone objects
 --FILE--
 <?php
 
-// NB: DateTimeZone class does not define a customized compare class handler so standard object handler will be used
+$timezones = array(
+    ['+0200', '-0200'],
+    ['EST', 'PST'],
+    ['Europe/Amsterdam', 'Europe/Berlin']
+);
 
-echo "Simple test for DateTimeZone compare object handler\n";
-
-//Set the default time zone
-date_default_timezone_set("Europe/London");
-
-class DateTimeZoneExt1 extends DateTimeZone {
-}
-
-class DateTimeZoneExt2 extends DateTimeZone{
-       public $foo = "Hello";
-       private $bar = 99;
+foreach ($timezones as [$timezone1, $timezone2]) {
+    compare_timezones($timezone1, $timezone1);
+    compare_timezones($timezone1, $timezone2);
 }
 
-class DateTimeZoneExt3 extends DateTimeZoneExt2 {
+var_dump(new DateTimeZone('Europe/Berlin') == new DateTimeZone('CET'));
+
+function compare_timezones($timezone1, $timezone2)
+{
+    $tz1 = new DateTimeZone($timezone1);
+    $tz2 = new DateTimeZone($timezone2);
+    echo "compare $timezone1 with $timezone2\n";
+    echo "< ";
+    var_dump($tz1 < $tz2);
+    echo "= ";
+    var_dump($tz1 == $tz2);
+    echo "> ";
+    var_dump($tz1 > $tz2);
 }
 
-$obj1 = new DateTimeZone("Europe/London");
-$obj2 = new DateTimeZoneExt1("Europe/London");
-$obj3 = new DateTimeZoneExt2("Europe/London");
-$obj4 = new DateTimeZoneExt3("Europe/London");
-
-echo "\n-- All the following tests should compare equal --\n";
-var_dump($obj1 == $obj1);
-echo "\n-- All the following tests should compare NOT equal --\n";
-var_dump($obj1 == $obj2);
-var_dump($obj1 == $obj3);
-var_dump($obj1 == $obj4);
-var_dump($obj2 == $obj3);
-var_dump($obj2 == $obj4);
-var_dump($obj3 == $obj4);
-
 ?>
-===DONE===
---EXPECT--
-Simple test for DateTimeZone compare object handler
-
--- All the following tests should compare equal --
-bool(true)
-
--- All the following tests should compare NOT equal --
-bool(false)
-bool(false)
-bool(false)
-bool(false)
-bool(false)
+--EXPECTF--
+compare +0200 with +0200
+< bool(false)
+= bool(true)
+> bool(false)
+compare +0200 with -0200
+< bool(false)
+= bool(false)
+> bool(false)
+compare EST with EST
+< bool(false)
+= bool(true)
+> bool(false)
+compare EST with PST
+< bool(false)
+= bool(false)
+> bool(false)
+compare Europe/Amsterdam with Europe/Amsterdam
+< bool(false)
+= bool(true)
+> bool(false)
+compare Europe/Amsterdam with Europe/Berlin
+< bool(false)
+= bool(false)
+> bool(false)
+
+Warning: main(): Trying to compare different kinds of DateTimeZone objects in %s on line %d
 bool(false)
-===DONE===