From a2f8c78183a5911bc5a923956ff531b6d27cff2a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 4 Mar 2020 19:20:10 +0100 Subject: [PATCH] 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. --- NEWS | 1 + ext/date/php_date.c | 28 ++++++ .../tests/DateTimeZone_compare_basic1.phpt | 92 ++++++++++--------- 3 files changed, 78 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index b087d3a847..40fd08f9b6 100644 --- 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 8c6dfbbe3e..02068b44cc 100644 --- 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 790bb6cf7f..b4e8d76cc2 100644 --- 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-- "; + 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=== -- 2.40.0