Fixed bug #71018 (ReflectionProperty::setValue() behavior changed)

4 files changed, 53 insertions, 23 deletions

diff --git a/NEWS b/NEWS
index 3dcb4e72cd..776e98de72 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,8 @@ PHP                                                                        NEWS
   . Fixed stderr being written to stdout. (Bob)
 
 - Reflection:
+  . Fixed bug #71018 (ReflectionProperty::setValue() behavior changed).
+    (Laruence)
   . Fixed bug #70982 (setStaticPropertyValue behaviors inconsistently with
     5.6). (Laruence)
 
diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c
index 72df8b5901..80001e4997 100644
--- a/ext/reflection/php_reflection.c
+++ b/ext/reflection/php_reflection.c
@@ -5448,31 +5448,16 @@ ZEND_METHOD(reflection_property, setValue)
 		}
 		variable_ptr = &CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset];
 		if (variable_ptr != value) {
-			if (Z_ISREF_P(variable_ptr)) {
-				zval garbage;
+			zval garbage;
 
-				ZVAL_COPY_VALUE(&garbage, variable_ptr); /* old value should be destroyed */
+			ZVAL_DEREF(variable_ptr);
+			ZVAL_DEREF(value);
 
-				/* To check: can't *variable_ptr be some system variable like error_zval here? */
-				ZVAL_COPY_VALUE(variable_ptr, value);
-				if (Z_REFCOUNTED_P(value) && Z_REFCOUNT_P(value) > 0) {
-					zval_copy_ctor(variable_ptr);
-				}
-				zval_dtor(&garbage);
-			} else {
-				zval garbage;
+			ZVAL_COPY_VALUE(&garbage, variable_ptr);
 
-				ZVAL_COPY_VALUE(&garbage, variable_ptr);
-				/* if we assign referenced variable, we should separate it */
-				if (Z_REFCOUNTED_P(value)) {
-					Z_ADDREF_P(value);
-				}
-				if (Z_ISREF_P(value)) {
-					SEPARATE_ZVAL(value);
-				}
-				ZVAL_COPY_VALUE(variable_ptr, value);
-				zval_ptr_dtor(&garbage);
-			}
+			ZVAL_COPY(variable_ptr, value);
+
+			zval_ptr_dtor(&garbage);
 		}
 	} else {
 		const char *class_name, *prop_name;
diff --git a/ext/reflection/tests/ReflectionProperty_setAccessible.phpt b/ext/reflection/tests/ReflectionProperty_setAccessible.phpt
index 1e829b3a6c..cc184c1920 100644
--- a/ext/reflection/tests/ReflectionProperty_setAccessible.phpt
+++ b/ext/reflection/tests/ReflectionProperty_setAccessible.phpt
@@ -132,7 +132,7 @@ string(44) "Cannot access non-public member B::protected"
 string(50) "Cannot access non-public member B::protectedStatic"
 string(42) "Cannot access non-public member A::private"
 string(1) "a"
-string(1) "b"
+string(1) "f"
 string(1) "c"
 string(1) "e"
 string(1) "f"
diff --git a/ext/reflection/tests/bug71018.phpt b/ext/reflection/tests/bug71018.phpt
new file mode 100644
index 0000000000..00baa31d3c
--- /dev/null
+++ b/ext/reflection/tests/bug71018.phpt
@@ -0,0 +1,43 @@
+--TEST--
+Bug #71018 (ReflectionProperty::setValue() behavior changed)
+--FILE--
+<?php
+class T1 {
+    public static $data;
+
+    public static function getDataBySelf()
+    {
+        return self::$data;
+    }
+
+    public static function getDataByStatic()
+    {
+        return static::$data;
+    }
+}
+
+class T2 extends T1 {}
+
+$Prop1 = new ReflectionProperty(T1::class, 'data');
+$Prop2 = new ReflectionProperty(T2::class, 'data');
+
+// #1
+// prints: hello, hello in PHP5, but world, hello in PHP7 - not OK
+$Prop1->setValue(\T1::class, "world");
+$Prop2->setValue(\T2::class, 'hello');
+var_dump("T2::self = " . T2::getDataBySelf());
+var_dump("T2::static = " . T2::getDataByStatic());
+
+// #2
+// prints: hello, hello in both PHP5 and PHP7 - OK
+T1::$data = "world";
+T2::$data = 'hello';
+
+var_dump("T2::self = " . T2::getDataBySelf());
+var_dump("T2::static = " . T2::getDataByStatic());
+?>
+--EXPECT--
+string(16) "T2::self = hello"
+string(18) "T2::static = hello"
+string(16) "T2::self = hello"
+string(18) "T2::static = hello"