ext/spl: Fix unnecessible value written when using null as key

Author: Girgias

ext/spl/spl_array.c

@@ -465,33 +465,22 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
 	spl_array_object *intern = spl_array_from_obj(object);
 	HashTable *ht;
 	spl_hash_key key;
-
-	/* Cannot append if backing value is an object */
-	if (offset == NULL && spl_array_is_object(intern)) {
-		zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name));
-		return;
-	}
-
-	if (check_inherited && intern->fptr_offset_set) {
-		zval tmp;
-
-		if (!offset) {
-			ZVAL_NULL(&tmp);
-			offset = &tmp;
-		}
-		zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value);
-		return;
-	}
+	uint32_t refcount = 0;
 
 	if (intern->nApplyCount > 0) {
 		zend_throw_error(NULL, "Modification of ArrayObject during sorting is prohibited");
 		return;
 	}
 
-	Z_TRY_ADDREF_P(value);
+	/* We are appending */
+	if (!offset) {
+		/* Cannot append if backing value is an object */
+		if (spl_array_is_object(intern)) {
+			zend_throw_error(NULL, "Cannot append properties to objects, use %s::offsetSet() instead", ZSTR_VAL(object->ce->name));
+			return;
+		}
 
-	uint32_t refcount = 0;
-	if (!offset || Z_TYPE_P(offset) == IS_NULL) {
+		Z_TRY_ADDREF_P(value);
 		ht = spl_array_get_hash_table(intern);
 		refcount = spl_array_set_refcount(intern->is_child, ht, 1);
 		zend_hash_next_index_insert(ht, value);
@@ -502,6 +491,19 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
 		return;
 	}
 
+	if (check_inherited && intern->fptr_offset_set) {
+		zval tmp;
+
+		if (!offset) {
+			ZVAL_NULL(&tmp);
+			offset = &tmp;
+		}
+		zend_call_method_with_2_params(object, object->ce, &intern->fptr_offset_set, "offsetSet", NULL, offset, value);
+		return;
+	}
+
+	Z_TRY_ADDREF_P(value);
+
 	if (get_hash_key(&key, intern, offset) == FAILURE) {
 		zend_illegal_container_offset(object->ce->name, offset, BP_VAR_W);
 		zval_ptr_dtor(value);
@@ -514,6 +516,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
 		zend_hash_update_ind(ht, key.key, value);
 		spl_hash_key_release(&key);
 	} else {
+		ZEND_ASSERT(!spl_array_is_object(intern));
 		zend_hash_index_update(ht, key.h, value);
 	}
 

ext/spl/tests/ArrayObject/custom_append.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Overwritting default append method
+--EXTENSIONS--
+spl
+--FILE--
+<?php
+
+class Dummy {}
+
+class OverWriteAppend extends ArrayObject
+{
+    private $data;
+
+    function __construct($v)
+    {
+        $this->data = [];
+        parent::__construct($v);
+    }
+
+    function append($value): void
+    {
+        $this->data[] = $value;
+    }
+}
+
+$d = new Dummy();
+$ao = new OverWriteAppend($d);
+
+$ao->append("value1");
+var_dump($ao);
+?>
+--EXPECT--
+object(OverWriteAppend)#2 (2) {
+  ["data":"OverWriteAppend":private]=>
+  array(1) {
+    [0]=>
+    string(6) "value1"
+  }
+  ["storage":"ArrayObject":private]=>
+  object(Dummy)#1 (0) {
+  }
+}

ext/spl/tests/ArrayObject/using_null_as_key.phpt

@@ -0,0 +1,23 @@
+--TEST--
+Using null as key must make the value accessible
+--EXTENSIONS--
+spl
+--FILE--
+<?php
+class MyClass {}
+$c = new MyClass();
+
+$ao = new ArrayObject($c);
+
+$ao[null] = "null key";
+var_dump($c);
+var_dump($c->{""});
+var_dump($ao[null]);
+?>
+--EXPECT--
+object(MyClass)#1 (1) {
+  [""]=>
+  string(8) "null key"
+}
+string(8) "null key"
+string(8) "null key"

ext/spl/tests/ArrayObject/using_null_for_offset_methods.phpt

@@ -0,0 +1,25 @@
+--TEST--
+Using null as offset must make the value accessible
+--EXTENSIONS--
+spl
+--FILE--
+<?php
+class MyClass {}
+$c = new MyClass();
+
+$ao = new ArrayObject($c);
+
+$ao->offsetSet(null, "null key");
+var_dump($ao->offsetExists(null));
+var_dump($ao->offsetGet(null));
+var_dump($c);
+var_dump($c->{""});
+?>
+--EXPECT--
+bool(true)
+string(8) "null key"
+object(MyClass)#1 (1) {
+  [""]=>
+  string(8) "null key"
+}
+string(8) "null key"

ext/spl/tests/bug33136.phpt

@@ -49,10 +49,8 @@ var_dump(count($arrayObj));
 --EXPECT--
 Initiate Obj
 Assign values
-Collection::offsetSet(NULL,foo)
 Collection::offsetGet(0)
 string(3) "foo"
-Collection::offsetSet(NULL,bar)
 Collection::offsetGet(0)
 string(3) "foo"
 Collection::offsetGet(1)