Add SplFixedArray::__set_state

Fixes bug 75268.

The design is that all string keys must come before all integer
keys. The string keys become properties and the integer keys become
values in the array.

Other serialization/unserialization methods should adopt this same
strategy as there are bugs:
https://3v4l.org/IMnI5

Migrate SplFixedArray to get_properties_for.

Extract spl_fixedarray_import, as I expect to reuse this code for
other cases as well, such as the serialization bug mentioned above.

Author: morrisonlevi

ext/spl/spl_fixedarray.c

@@ -195,23 +195,18 @@ static HashTable* spl_fixedarray_object_get_gc(zend_object *obj, zval **table, i
 	return ht;
 }
 
-static HashTable* spl_fixedarray_object_get_properties(zend_object *obj)
+static HashTable* spl_fixedarray_get_properties_for(zend_object *obj, zend_prop_purpose purpose)
 {
 	spl_fixedarray_object *intern = spl_fixed_array_from_obj(obj);
-	HashTable *ht = zend_std_get_properties(obj);
 
-	if (!spl_fixedarray_empty(&intern->array)) {
-		zend_long j = zend_hash_num_elements(ht);
+	/* Keep the values and properties separate*/
+	HashTable *ht = zend_array_dup(zend_std_get_properties(obj));
 
+	if (!spl_fixedarray_empty(&intern->array)) {
 		for (zend_long i = 0; i < intern->array.size; i++) {
-			zend_hash_index_update(ht, i, &intern->array.elements[i]);
+			zend_hash_index_add_new(ht, i, &intern->array.elements[i]);
 			Z_TRY_ADDREF(intern->array.elements[i]);
 		}
-		if (j > intern->array.size) {
-			for (zend_long i = intern->array.size; i < j; ++i) {
-				zend_hash_index_del(ht, i);
-			}
-		}
 	}
 
 	return ht;
@@ -628,7 +623,6 @@ PHP_METHOD(SplFixedArray, fromArray)
 	} else if (num > 0 && !save_indexes) {
 		zval *element;
 		zend_long i = 0;
-
 		spl_fixedarray_init(&array, num);
 
 		ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(data), element) {
@@ -754,6 +748,120 @@ PHP_METHOD(SplFixedArray, getIterator)
 	zend_create_internal_iterator_zval(return_value, ZEND_THIS);
 }
 
+/* Assumes the object has been created and the spl_fixedarray_object's
+ * array member is uninitialized or zero-initialized.
+ * The state can have string keys, but they must come prior to integers:
+ *
+ * 	SplFixedArray::__set_state(array(
+ * 	   'property' => 'value',
+ * 	   0 => 1,
+ * 	   1 => 2,
+ * 	   2 => 3,
+ * 	))
+ */
+static zend_result spl_fixedarray_import(zend_object *object, HashTable *ht)
+{
+	spl_fixedarray_object *intern = spl_fixed_array_from_obj(object);
+	spl_fixedarray *fixed = &intern->array;
+
+	spl_fixedarray_init(fixed, 0);
+
+	/* For performance we do a single pass over the input array and the
+	 * spl_fixedarray elems. This complicates the implementation, but part
+	 * of the reason for choosing SplFixedArray is for peformance, and
+	 * although that is primarily for memory we should still care about CPU.
+	 *
+	 * We need to track the number of string keys, which must come before
+	 * all the integer keys. This way the total size of the input minus the
+	 * number of string keys equals the number of integer keys, so we can
+	 * allocate the spl_fixedarray.elements and do this in a single pass.
+	 */
+
+	// The total size of the input array
+	const zend_long nht = zend_hash_num_elements(ht);
+	zend_long nstrings = 0; // The number of string keys
+	zend_long nints; // will be nht - nstrings
+
+	zend_string *str_idx = NULL; // current string key of input array
+	zend_ulong num_idx = 0; // current int key (valid iff str_idx == NULL)
+	zval *val = NULL; // current value of input array
+	zend_long max_idx = -1; // the largest index found so far
+	zval *begin = NULL; // pointer to beginning of fixedarray's storage
+	zval *end = NULL; // points one element passed the last element
+	const char *ex_msg = NULL; // message for the value exception
+
+	ZEND_HASH_FOREACH_KEY_VAL(ht, num_idx, str_idx, val) {
+		if (str_idx != NULL) {
+			if (UNEXPECTED(max_idx >= 0)) {
+				ex_msg = "must have all its string keys come before all integer keys";
+				goto value_error;
+			}
+
+			++nstrings;
+			object->handlers->write_property(object, str_idx, val, NULL);
+
+		} else if (UNEXPECTED((zend_long)num_idx != ++max_idx)) {
+			ex_msg = "did not have integer keys that start at 0 and increment sequentially";
+			goto value_error;
+
+		} else {
+			if (UNEXPECTED(max_idx == 0)) {
+				nints = nht - nstrings;
+				fixed->size = nints;
+				fixed->elements =
+					safe_emalloc(nints, sizeof(zval), 0);
+				begin = fixed->elements;
+				end = fixed->elements + nints;
+			}
+
+			ZEND_ASSERT(num_idx == max_idx);
+			ZEND_ASSERT(begin != end);
+
+			ZVAL_COPY(begin++, val);
+		}
+	} ZEND_HASH_FOREACH_END();
+
+	ZEND_ASSERT(begin == end);
+	return SUCCESS;
+
+value_error:
+	spl_fixedarray_dtor_range(fixed, 0, max_idx);
+	if (fixed->elements) {
+		efree(fixed->elements);
+	}
+
+	/* Zero-initialize so the object release is valid. */
+	spl_fixedarray_init(fixed, 0);
+	zend_object_release(object);
+
+	zend_argument_value_error(1, ex_msg);
+	return FAILURE;
+}
+
+PHP_METHOD(SplFixedArray, __set_state)
+{
+	zval *state = NULL;
+
+	ZEND_PARSE_PARAMETERS_START(1, 1)
+		Z_PARAM_ARRAY(state);
+	ZEND_PARSE_PARAMETERS_END();
+
+	HashTable *ht = Z_ARRVAL_P(state);
+	zend_class_entry *called_scope = zend_get_called_scope(execute_data);
+
+	zend_result result = object_init_ex(return_value, called_scope);
+	if (UNEXPECTED(result != SUCCESS)) {
+		ZVAL_NULL(return_value);
+		RETURN_THROWS();
+	}
+
+	zend_object *object = Z_OBJ_P(return_value);
+	if (UNEXPECTED(spl_fixedarray_import(object, ht) != SUCCESS)) {
+		ZVAL_NULL(return_value);
+		RETURN_THROWS();
+	}
+}
+
 static void spl_fixedarray_it_dtor(zend_object_iterator *iter)
 {
 	zval_ptr_dtor(&iter->data);
@@ -844,7 +952,8 @@ PHP_MINIT_FUNCTION(spl_fixedarray)
 	spl_handler_SplFixedArray.unset_dimension = spl_fixedarray_object_unset_dimension;
 	spl_handler_SplFixedArray.has_dimension   = spl_fixedarray_object_has_dimension;
 	spl_handler_SplFixedArray.count_elements  = spl_fixedarray_object_count_elements;
-	spl_handler_SplFixedArray.get_properties  = spl_fixedarray_object_get_properties;
+	spl_handler_SplFixedArray.get_properties_for
+		= spl_fixedarray_get_properties_for;
 	spl_handler_SplFixedArray.get_gc          = spl_fixedarray_object_get_gc;
 	spl_handler_SplFixedArray.dtor_obj        = zend_objects_destroy_object;
 	spl_handler_SplFixedArray.free_obj        = spl_fixedarray_object_free_storage;

ext/spl/spl_fixedarray.stub.php

@@ -49,4 +49,7 @@ public function offsetSet($index, mixed $value) {}
     public function offsetUnset($index) {}
 
     public function getIterator(): Iterator {}
+
+    /** @return SplFixedArray */
+    public static function __set_state(array $state) {}
 }

ext/spl/spl_fixedarray_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 831fe70055eb62135ae49321e5e5f3fe08c3d95f */
+ * Stub hash: 54028c566f2d868da1f91e5f4f627fbe6e823bb5 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplFixedArray___construct, 0, 0, 0)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, size, IS_LONG, 0, "0")
@@ -39,6 +39,10 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_SplFixedArray_getIterator, 0, 0, Iterator, 0)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplFixedArray___set_state, 0, 0, 1)
+	ZEND_ARG_TYPE_INFO(0, state, IS_ARRAY, 0)
+ZEND_END_ARG_INFO()
+
 
 ZEND_METHOD(SplFixedArray, __construct);
 ZEND_METHOD(SplFixedArray, __wakeup);
@@ -52,6 +56,7 @@ ZEND_METHOD(SplFixedArray, offsetGet);
 ZEND_METHOD(SplFixedArray, offsetSet);
 ZEND_METHOD(SplFixedArray, offsetUnset);
 ZEND_METHOD(SplFixedArray, getIterator);
+ZEND_METHOD(SplFixedArray, __set_state);
 
 
 static const zend_function_entry class_SplFixedArray_methods[] = {
@@ -67,5 +72,6 @@ static const zend_function_entry class_SplFixedArray_methods[] = {
 	ZEND_ME(SplFixedArray, offsetSet, arginfo_class_SplFixedArray_offsetSet, ZEND_ACC_PUBLIC)
 	ZEND_ME(SplFixedArray, offsetUnset, arginfo_class_SplFixedArray_offsetUnset, ZEND_ACC_PUBLIC)
 	ZEND_ME(SplFixedArray, getIterator, arginfo_class_SplFixedArray_getIterator, ZEND_ACC_PUBLIC)
+	ZEND_ME(SplFixedArray, __set_state, arginfo_class_SplFixedArray___set_state, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
 	ZEND_FE_END
 };

ext/spl/tests/ArrayObject_overloaded_object_incompatible.phpt

@@ -1,5 +1,7 @@
 --TEST--
 Objects with overloaded get_properties are incompatible with ArrayObject
+--XFAIL--
+SplFixedArray has migrated to get_properties_for; find new test case
 --FILE--
 <?php
 

ext/spl/tests/spl_fixedarray___set_state.phpt

@@ -0,0 +1,60 @@
+--TEST--
+SplFixedArray: __set_state
+--FILE--
+<?php
+
+$fixed = SplFixedArray::__set_state([1, 2, 3]);
+var_export($fixed);
+echo "\n---\n";
+
+// dynamic properties
+$fixed->undefined = 'undef';
+var_export($fixed);
+echo "\n---\n";
+
+// inheritance
+class Vec
+    extends SplFixedArray
+{
+    public $type;
+}
+$vec = new Vec(3);
+$vec[0] = 1;
+$vec[1] = 2;
+$vec[2] = 3;
+$vec->type = 'int';
+var_export($vec);
+echo "\n---\n";
+
+$vec->undefined = 'undef';
+var_export($vec);
+
+?>
+--EXPECT--
+SplFixedArray::__set_state(array(
+   0 => 1,
+   1 => 2,
+   2 => 3,
+))
+---
+SplFixedArray::__set_state(array(
+   'undefined' => 'undef',
+   0 => 1,
+   1 => 2,
+   2 => 3,
+))
+---
+Vec::__set_state(array(
+   'type' => 'int',
+   0 => 1,
+   1 => 2,
+   2 => 3,
+))
+---
+Vec::__set_state(array(
+   'type' => 'int',
+   'undefined' => 'undef',
+   0 => 1,
+   1 => 2,
+   2 => 3,
+))

ext/spl/tests/spl_fixedarray___set_state_errors.phpt

@@ -0,0 +1,36 @@
+--TEST--
+SplFixedArray: __set_state
+--FILE--
+<?php
+
+try {
+    $fixed = SplFixedArray::__set_state([1 => 2]);
+} catch (ValueError $error) {
+    var_dump($error->getMessage());
+}
+
+try {
+    $fixed = SplFixedArray::__set_state([0 => 1, 1 => 2, 3 => 4]);
+} catch (ValueError $error) {
+    var_dump($error->getMessage());
+}
+
+try {
+    $fixed = SplFixedArray::__set_state([-1 => 2]);
+} catch (ValueError $error) {
+    var_dump($error->getMessage());
+}
+
+echo "-----\n";
+
+try {
+    $fixed = SplFixedArray::__set_state([0 => 1, 'type' => 'undefined']);
+} catch (ValueError $error) {
+    var_dump($error->getMessage());
+}
+--EXPECTF--
+string(%d) "SplFixedArray::__set_state(): Argument #1 ($state) did not have integer keys that start at 0 and increment sequentially"
+string(%d) "SplFixedArray::__set_state(): Argument #1 ($state) did not have integer keys that start at 0 and increment sequentially"
+string(%d) "SplFixedArray::__set_state(): Argument #1 ($state) did not have integer keys that start at 0 and increment sequentially"
+-----
+string(%d) "SplFixedArray::__set_state(): Argument #1 ($state) must have all its string keys come before all integer keys"

ext/spl/tests/spl_fixedarray_export_import.phpt

@@ -0,0 +1,49 @@
+--TEST--
+SplFixedArray: __set_state export and import
+--FILE--
+<?php
+
+function compare_export($var, string $case) {
+    $exported = var_export($var, true);
+
+    $imported = eval("return {$exported};");
+    $reexported = var_export($imported, true);
+
+    if ($exported == $reexported) {
+        echo "Pass {$case}.\n";
+    } else {
+        echo "Fail {$case}.\nExpected:\n{$exported}\nActual:\n{$reexported}\n";
+    }
+}
+
+// simple
+$fixed = SplFixedArray::__set_state([1, 2, 3]);
+compare_export($fixed, 'simple');
+
+// dynamic properties
+$fixed->undefined = 'undef';
+compare_export($fixed, 'dynamic properties');
+
+// inheritance
+class Vec
+    extends SplFixedArray
+{
+    public $type;
+}
+$vec = new Vec(3);
+$vec[0] = 1;
+$vec[1] = 2;
+$vec[2] = 3;
+$vec->type = 'int';
+
+compare_export($vec, 'inheritance');
+
+// dynamic properties and inheritance
+$vec->undefined = 'undef';
+compare_export($vec, 'dynamic properties and inheritance');
+
+--EXPECT--
+Pass simple.
+Pass dynamic properties.
+Pass inheritance.
+Pass dynamic properties and inheritance.