date: Fix unserialization of Time\Duration (#23629)

Author: TimWolla

NEWS

@@ -8,6 +8,9 @@ PHP                                                                        NEWS
   . Fix handling of references to typed properties during unserialization
     of various internal classes. (ndossche, timwolla)
 
+- Date:
+  . Fix unserialization of Time\Duration. (timwolla)
+
 - DOM:
   . Fixed use-after-free when re-constructing a DOMXPath whose php:function
     registrations are freed while still reachable from the cycle collector.

ext/date/tests/time/duration/gh23639.phpt

@@ -3,8 +3,6 @@ GH-23639 (object_properties_load allows creating readonly reference properties)
 --CREDITS--
 arnaud-lb
 ndossche
---XFAIL--
-Test can only succeed when GH-23629 is also merged
 --FILE--
 <?php
 

ext/date/tests/time/duration/serialize.phpt

@@ -0,0 +1,71 @@
+--TEST--
+Time\Duration: serialize()
+--FILE--
+<?php
+
+require __DIR__ . '/helper.inc';
+
+var_dump($serialized = serialize(Time\Duration::fromSeconds(1, 2)->negate()));
+echo f($unserialized = unserialize($serialized)), PHP_EOL;
+var_dump(serialize($unserialized));
+echo f($unserialized->add($unserialized)), PHP_EOL;
+
+try {
+    // $negative is not bool, but coercible.
+    echo f(unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";i:999;}')), PHP_EOL;
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    // $negative is not bool and not coercible.
+    unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";N;}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    // $seconds is negative.
+    unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:-1;s:11:"nanoseconds";i:1;s:8:"negative";b:0;}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    // Dynamic property.
+    unserialize('O:13:"Time\Duration":4:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";b:0;s:3:"foo";N;}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+
+try {
+    // Out of range nanoseconds
+    unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1000000000;s:8:"negative";b:0;}');
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    Time\Duration::fromSeconds(1, 1)
+        ->__unserialize([
+            'seconds' => 2,
+            'nanoseconds' => 2,
+            'negative' => true,
+        ]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+string(85) "O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:2;s:8:"negative";b:1;}"
+         -1.000000002
+string(85) "O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:2;s:8:"negative";b:1;}"
+         -2.000000004
+Exception: Invalid serialization data for Time\Duration object
+Exception: Invalid serialization data for Time\Duration object
+Exception: Invalid serialization data for Time\Duration object
+Exception: Invalid serialization data for Time\Duration object
+Exception: Invalid serialization data for Time\Duration object
+Exception: Invalid serialization data for Time\Duration object

ext/date/time.stub.php

@@ -21,6 +21,10 @@ private function __construct()
         {
         }
 
+        public function __unserialize(array $data): void
+        {
+        }
+
         public static function fromSeconds(int $seconds, int $nanoseconds = 0): Duration
         {
         }

ext/date/time_arginfo.h

@@ -77,20 +77,24 @@ static inline php_date_time_duration *create_duration_shell(zval *target)
 	return Z_DATE_TIME_DURATION_P(target);
 }
 
-ZEND_ATTRIBUTE_NODISCARD static inline zend_result sync_properties(php_date_time_duration *object)
+static inline bool duration_representable(const timelib_duration *duration)
 {
-	if (
+	return
 		/* Check if the duration would overflow the $seconds property. */
-		object->duration.seconds > ((uint64_t)ZEND_LONG_MAX)
+		duration->seconds <= ((uint64_t)ZEND_LONG_MAX)
 		/* This constraint is an explicit part of PHP's API: It is the maximum $seconds
 		 * value that allows storing the entire duration as a single int64_t counting
 		 * nanoseconds, which might be desirable in the future when userland `int` is
 		 * consistently 64 bits.
 		 *
 		 * While it is currently also enforced by timelib, this might change
 		 * in a future version of timelib, thus we also enforce it manually. */
-		|| object->duration.seconds > UINT64_C(9223372035)
-	) {
+		&& duration->seconds <= UINT64_C(9223372035);
+}
+
+ZEND_ATTRIBUTE_NODISCARD static inline zend_result sync_properties(php_date_time_duration *object)
+{
+	if (!duration_representable(&object->duration)) {
 		throw_out_of_range_exception();
 		return FAILURE;
 	}
@@ -149,6 +153,52 @@ PHP_METHOD(Time_Duration, __construct)
 	zend_throw_error(NULL, "Cannot directly construct Time\\Duration, use Time\\Duration::from*() methods instead");
 }
 
+PHP_METHOD(Time_Duration, __unserialize)
+{
+	php_date_time_duration *duration = Z_DATE_TIME_DURATION_P(ZEND_THIS);
+
+	HashTable *data;
+
+	ZEND_PARSE_PARAMETERS_START(1, 1)
+		Z_PARAM_ARRAY_HT(data);
+	ZEND_PARSE_PARAMETERS_END();
+
+	object_properties_load(&duration->std, data);
+	if (EG(exception)) {
+		goto fail;
+	}
+
+	zval *seconds = OBJ_PROP_NUM(&duration->std, 0);
+	zval *nanoseconds = OBJ_PROP_NUM(&duration->std, 1);
+	zval *negative = OBJ_PROP_NUM(&duration->std, 2);
+
+	/* Verify that both properties are positive, since the timelib_duration_ctor_static() takes unsigned. */
+	if (Z_LVAL_P(seconds) < 0 || Z_LVAL_P(nanoseconds) < 0) {
+		goto fail;
+	}
+
+	int error = timelib_duration_ctor_static(&duration->duration, Z_LVAL_P(seconds), Z_LVAL_P(nanoseconds), Z_TYPE_P(negative) == IS_TRUE);
+	if (error != TIMELIB_ERROR_NO_ERROR) {
+		throw_timelib_error(error);
+		goto fail;
+	}
+
+	if (!duration_representable(&duration->duration)) {
+		throw_out_of_range_exception();
+		goto fail;
+	}
+
+	return;
+
+ fail:
+
+	/* If an exception is already active (e.g. for unrepresentable durations) it will be wrapped for
+	 * uniform exceptions thrown from unserialization handlers, but to still provide additional
+	 * context for a human reader. */
+	zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(duration->std.ce->name));
+	RETURN_THROWS();
+}
+
 PHP_METHOD(Time_Duration, fromSeconds)
 {
 	zend_ulong seconds;