From 337deb1e9f490c1c333b55eb29b4cbb4d0500f36 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 13 Oct 2021 12:43:08 +0200 Subject: [PATCH 1/4] Fix #81500: Interval serialization regression since 7.3.14 / 7.4.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While it may not be desired, `DateInterval::$f` supports negative values, at least with regard to calculations. We still need to guard from assigning double values which are out of range for signed 64bit integers (which would be undefined behavior). While we could restrict this only to values which are actually out of range, the marker for invalid µs is `-1000000`, so we cannot accept smaller numbers. To retain symmetry, we also reject values greater than `1000000`. We must not do that only for unserialization, but also when the property is set in the first place. --- ext/date/php_date.c | 9 +++++++-- ext/date/tests/bug81500.phpt | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 ext/date/tests/bug81500.phpt diff --git a/ext/date/php_date.c b/ext/date/php_date.c index 189e4355f5d4..3940618c2410 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -4331,7 +4331,12 @@ static zval *date_interval_write_property(zval *object, zval *member, zval *valu SET_VALUE_FROM_STRUCT(i, "i"); SET_VALUE_FROM_STRUCT(s, "s"); if (strcmp(Z_STRVAL_P(member), "f") == 0) { - obj->diff->us = zval_get_double(value) * 1000000; + double val = zval_get_double(value) * 1000000; + if (val > -1000000 && val < 1000000) { + obj->diff->us = val; + } else { + obj->diff->us = -1000000; + } break; } SET_VALUE_FROM_STRUCT(invert, "invert"); @@ -4471,7 +4476,7 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter (*intobj)->diff->us = -1000000; if (z_arg) { double val = zval_get_double(z_arg) * 1000000; - if (val >= 0 && val < 1000000) { + if (val > -1000000 && val < 1000000) { (*intobj)->diff->us = val; } } diff --git a/ext/date/tests/bug81500.phpt b/ext/date/tests/bug81500.phpt new file mode 100644 index 000000000000..0c60c204a34d --- /dev/null +++ b/ext/date/tests/bug81500.phpt @@ -0,0 +1,16 @@ +--TEST-- +Bug #81500 (Interval serialization regression since 7.3.14 / 7.4.2) +--FILE-- +f = -0.000001; +var_dump($interval->s, $interval->f); + +$interval = unserialize(serialize($interval)); +var_dump($interval->s, $interval->f); +?> +--EXPECT-- +int(1) +float(-1.0E-6) +int(1) +float(-1.0E-6) From 9ec00855de76b5065ae2c0d54d7ac062232cf58d Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 15 Oct 2021 15:06:21 +0200 Subject: [PATCH 2/4] Use zend_dval_to_lval() to take care of overflow If overflow would occur, the value is set to `0`, what is standard PHP behavior anyway. This way we can avoid setting the invalid marker, which doesn't work as expected anyway. We need to adapt some of the existing tests wrt. this behavior. In particular, we check for an arbitrary value in bug79015.phpt, to cater to differences between 32bit and 64bit architectures. --- ext/date/php_date.c | 13 ++----------- ext/date/tests/bug53437_var5.phpt | 2 +- ext/date/tests/bug73091.phpt | 2 +- ext/date/tests/bug79015.phpt | 2 +- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/ext/date/php_date.c b/ext/date/php_date.c index 3940618c2410..e1a427c5ca4c 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -4331,12 +4331,7 @@ static zval *date_interval_write_property(zval *object, zval *member, zval *valu SET_VALUE_FROM_STRUCT(i, "i"); SET_VALUE_FROM_STRUCT(s, "s"); if (strcmp(Z_STRVAL_P(member), "f") == 0) { - double val = zval_get_double(value) * 1000000; - if (val > -1000000 && val < 1000000) { - obj->diff->us = val; - } else { - obj->diff->us = -1000000; - } + obj->diff->us = zend_dval_to_lval(zval_get_double(value) * 1000000.0); break; } SET_VALUE_FROM_STRUCT(invert, "invert"); @@ -4473,12 +4468,8 @@ static int php_date_interval_initialize_from_hash(zval **return_value, php_inter PHP_DATE_INTERVAL_READ_PROPERTY("s", s, timelib_sll, -1) { zval *z_arg = zend_hash_str_find(myht, "f", sizeof("f") - 1); - (*intobj)->diff->us = -1000000; if (z_arg) { - double val = zval_get_double(z_arg) * 1000000; - if (val > -1000000 && val < 1000000) { - (*intobj)->diff->us = val; - } + (*intobj)->diff->us = zend_dval_to_lval(zval_get_double(z_arg) * 1000000.0); } } PHP_DATE_INTERVAL_READ_PROPERTY("weekday", weekday, int, -1) diff --git a/ext/date/tests/bug53437_var5.phpt b/ext/date/tests/bug53437_var5.phpt index 38783b15456b..abd7313051e6 100644 --- a/ext/date/tests/bug53437_var5.phpt +++ b/ext/date/tests/bug53437_var5.phpt @@ -44,6 +44,6 @@ object(DateInterval)#%d (16) { ["have_special_relative"]=> int(0) ["f"]=> - float(-1) + float(0) } ==DONE== diff --git a/ext/date/tests/bug73091.phpt b/ext/date/tests/bug73091.phpt index 14f161afe746..918170d823fb 100644 --- a/ext/date/tests/bug73091.phpt +++ b/ext/date/tests/bug73091.phpt @@ -28,7 +28,7 @@ object(DateInterval)#%d (16) { ["s"]=> int(-1) ["f"]=> - float(-1) + float(0) ["weekday"]=> int(-1) ["weekday_behavior"]=> diff --git a/ext/date/tests/bug79015.phpt b/ext/date/tests/bug79015.phpt index 5ebb13832b79..9d752dee7221 100644 --- a/ext/date/tests/bug79015.phpt +++ b/ext/date/tests/bug79015.phpt @@ -20,7 +20,7 @@ object(DateInterval)#%d (16) { ["s"]=> int(0) ["f"]=> - float(-1) + float(%f) ["weekday"]=> int(0) ["weekday_behavior"]=> From da1ade8c8bdbe485c4d063fb0e67bfdcff3eb7d6 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 15 Oct 2021 16:54:42 +0200 Subject: [PATCH 3/4] fix another affected test case --- ext/standard/tests/serialize/bug69425.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/serialize/bug69425.phpt b/ext/standard/tests/serialize/bug69425.phpt index c7f426578981..f44aae91cbb1 100644 --- a/ext/standard/tests/serialize/bug69425.phpt +++ b/ext/standard/tests/serialize/bug69425.phpt @@ -40,7 +40,7 @@ array(2) { ["s"]=> int(-1) ["f"]=> - float(-1) + float(0) ["weekday"]=> int(-1) ["weekday_behavior"]=> From 2b7c03910419219e2abf332d9fd46511b3674bfb Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 15 Oct 2021 17:35:16 +0200 Subject: [PATCH 4/4] fix another test case --- ext/date/tests/bug53437_var3.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/date/tests/bug53437_var3.phpt b/ext/date/tests/bug53437_var3.phpt index 6738f14be792..cf0d07451c32 100644 --- a/ext/date/tests/bug53437_var3.phpt +++ b/ext/date/tests/bug53437_var3.phpt @@ -44,6 +44,6 @@ object(DateInterval)#%d (16) { ["have_special_relative"]=> int(0) ["f"]=> - float(-1) + float(0) } ==DONE==