Don't overload get_properties for ArrayObject

Instead overload get_properties_for for a few specific cases such
as array casts. This resolves the issue where ArrayObject
get_properties may violate engine invariants in some cases.

Author: nikic

ext/reflection/tests/bug61388.phpt

@@ -14,12 +14,6 @@ print_r($reflObj->getProperties(ReflectionProperty::IS_PUBLIC));
 --EXPECT--
 Array
 (
-    [0] => ReflectionProperty Object
-        (
-            [name] => test
-            [class] => ArrayObject
-        )
-
 )
 Array
 (

ext/spl/spl_array.c

@@ -807,18 +807,40 @@ SPL_METHOD(Array, getArrayCopy)
 	RETURN_ARR(zend_array_dup(spl_array_get_hash_table(intern)));
 } /* }}} */
 
-static HashTable *spl_array_get_properties(zval *object) /* {{{ */
+static HashTable *spl_array_get_properties_for(zval *object, zend_prop_purpose purpose) /* {{{ */
 {
 	spl_array_object *intern = Z_SPLARRAY_P(object);
+	HashTable *ht;
+	zend_bool dup;
 
 	if (intern->ar_flags & SPL_ARRAY_STD_PROP_LIST) {
-		if (!intern->std.properties) {
-			rebuild_object_properties(&intern->std);
-		}
-		return intern->std.properties;
+		return zend_std_get_properties_for(object, purpose);
+	}
+
+	/* We are supposed to be the only owner of the internal hashtable.
+	 * The "dup" flag decides whether this is a "long-term" use where
+	 * we need to duplicate, or a "temporary" one, where we can expect
+	 * that no operations on the ArrayObject will be performed in the
+	 * meantime. */
+	switch (purpose) {
+		case ZEND_PROP_PURPOSE_ARRAY_CAST:
+			dup = 1;
+			break;
+		case ZEND_PROP_PURPOSE_VAR_EXPORT:
+		case ZEND_PROP_PURPOSE_JSON:
+			dup = 0;
+			break;
+		default:
+			return zend_std_get_properties_for(object, purpose);
 	}
 
-	return spl_array_get_hash_table(intern);
+	ht = spl_array_get_hash_table(intern);
+	if (dup) {
+		ht = zend_array_dup(ht);
+	} else {
+		GC_ADDREF(ht);
+	}
+	return ht;
 } /* }}} */
 
 static HashTable* spl_array_get_debug_info(zval *obj, int *is_temp) /* {{{ */
@@ -2013,7 +2035,7 @@ PHP_MINIT_FUNCTION(spl_array)
 	spl_handler_ArrayObject.has_dimension = spl_array_has_dimension;
 	spl_handler_ArrayObject.count_elements = spl_array_object_count_elements;
 
-	spl_handler_ArrayObject.get_properties = spl_array_get_properties;
+	spl_handler_ArrayObject.get_properties_for = spl_array_get_properties_for;
 	spl_handler_ArrayObject.get_debug_info = spl_array_get_debug_info;
 	spl_handler_ArrayObject.get_gc = spl_array_get_gc;
 	spl_handler_ArrayObject.read_property = spl_array_read_property;

ext/spl/tests/ArrayObject_get_object_vars.phpt

@@ -0,0 +1,30 @@
+--TEST--
+get_object_vars() on ArrayObject works on the properties of the ArrayObject itself
+--FILE--
+<?php
+
+class Test {
+    public $test;
+    public $test2;
+}
+
+class AO extends ArrayObject {
+    private $test;
+
+    public function getObjectVars() {
+        return get_object_vars($this);
+    }
+}
+
+$ao = new AO(new Test);
+var_dump(get_object_vars($ao));
+var_dump($ao->getObjectVars());
+
+?>
+--EXPECT--
+array(0) {
+}
+array(1) {
+  ["test"]=>
+  NULL
+}

ext/spl/tests/array_017.phpt

@@ -139,13 +139,17 @@ array(3) {
   ["Flags"]=>
   int(0)
   ["OVars"]=>
-  array(3) {
-    [0]=>
-    int(1)
-    ["a"]=>
-    int(25)
+  array(5) {
     ["pub1"]=>
-    int(42)
+    int(1)
+    ["pro1"]=>
+    int(2)
+    ["pri1"]=>
+    int(3)
+    ["imp1"]=>
+    int(4)
+    ["dyn1"]=>
+    int(5)
   }
   ["$this"]=>
   object(ArrayObjectEx)#%d (6) {
@@ -178,13 +182,17 @@ array(3) {
   ["Flags"]=>
   int(0)
   ["OVars"]=>
-  array(3) {
-    [0]=>
+  array(5) {
+    ["pub2"]=>
     int(1)
-    ["a"]=>
-    int(25)
-    ["pub1"]=>
-    int(42)
+    ["pro2"]=>
+    int(2)
+    ["pri2"]=>
+    int(3)
+    ["imp2"]=>
+    int(4)
+    ["dyn2"]=>
+    int(5)
   }
   ["$this"]=>
   object(ArrayIteratorEx)#%d (6) {
@@ -242,13 +250,17 @@ array(3) {
   ["Flags"]=>
   int(0)
   ["OVars"]=>
-  array(3) {
-    [0]=>
+  array(5) {
+    ["pub2"]=>
     int(1)
-    ["a"]=>
-    int(25)
-    ["pub1"]=>
-    int(42)
+    ["pro2"]=>
+    int(2)
+    ["pri2"]=>
+    int(3)
+    ["imp2"]=>
+    int(4)
+    ["dyn2"]=>
+    int(5)
   }
   ["$this"]=>
   object(ArrayIteratorEx)#%d (6) {
@@ -541,14 +553,16 @@ array(3) {
   ["Flags"]=>
   int(0)
   ["OVars"]=>
-  array(4) {
-    ["pub1"]=>
+  array(5) {
+    ["pub2"]=>
     int(1)
-    ["pro1"]=>
+    ["pro2"]=>
     int(2)
-    ["imp1"]=>
+    ["pri2"]=>
+    int(3)
+    ["imp2"]=>
     int(4)
-    ["dyn1"]=>
+    ["dyn2"]=>
     int(5)
   }
   ["$this"]=>
@@ -598,14 +612,16 @@ array(3) {
   ["Flags"]=>
   int(0)
   ["OVars"]=>
-  array(4) {
-    ["pub1"]=>
+  array(5) {
+    ["pub2"]=>
     int(1)
-    ["pro1"]=>
+    ["pro2"]=>
     int(2)
-    ["imp1"]=>
+    ["pri2"]=>
+    int(3)
+    ["imp2"]=>
     int(4)
-    ["dyn1"]=>
+    ["dyn2"]=>
     int(5)
   }
   ["$this"]=>

ext/spl/tests/bug61347.phpt

@@ -12,7 +12,7 @@ var_dump(isset($b['no_exists'])); //false
 var_dump(empty($b['b'])); //true
 var_dump(empty($b[37])); //true
 
-var_dump(array_key_exists('b', $b)); //true
+var_dump(array_key_exists('b', $b)); //false
 var_dump($b['b']);
 
 $a = array('b' => '', 37 => false);
@@ -31,7 +31,7 @@ bool(false)
 bool(false)
 bool(true)
 bool(true)
-bool(true)
+bool(false)
 NULL
 bool(true)
 bool(true)