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,31 @@ 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;
 
-	if (intern->ar_flags & SPL_ARRAY_STD_PROP_LIST) {
-		if (!intern->std.properties) {
-			rebuild_object_properties(&intern->std);
-		}
-		return intern->std.properties;
+	switch (purpose) {
+		case ZEND_PROP_PURPOSE_ARRAY_CAST:
+		case ZEND_PROP_PURPOSE_SERIALIZE:
+		case ZEND_PROP_PURPOSE_VAR_EXPORT:
+		case ZEND_PROP_PURPOSE_JSON:
+			if (!(intern->ar_flags & SPL_ARRAY_STD_PROP_LIST)) {
+				break;
+			}
+			/* break missing intentionally */
+		default:
+			return zend_std_get_properties_for(object, purpose);
 	}
 
-	return spl_array_get_hash_table(intern);
+	/* We are supposed to be the only owner of the internal hashtable,
+	 * so in the general case we'd have to zend_array_dup() here. However
+	 * the existing users only use the properties temporarily and read-only,
+	 * so returning without a copy is fine here (for now). */
+	ht = spl_array_get_hash_table(intern);
+	GC_ADDREF(ht);
+	return ht;
 } /* }}} */
 
 static HashTable* spl_array_get_debug_info(zval *obj, int *is_temp) /* {{{ */
@@ -2013,7 +2026,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)