Fix #77291: magic methods inherited from a trait may be ignored

When adding methods from a trait, we must not assume that a method name with the same length as the name of the using class is either a PHP 4 style constructor, or not a magic method at all – it may well be another magic method. We mostly preserve the spirit of the optimization which caused this regression, and avoid string comparisons for all method names which can never be magic methods.
php · Dec 16, 2018 · 0061db5 · 0061db5
1 parent 54739c7
commit 0061db5
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 12 deletions.
diff --git a/NEWS b/NEWS
@@ -8,6 +8,8 @@ PHP                                                                        NEWS
     (Valentin V. Bartenev)
   . Fixed bug #76046 (PHP generates "FE_FREE" opcode on the wrong line).
     (Nikita)
+  . Fixed bug #77291 (magic methods inherited from a trait may be ignored).
+    (cmb)
 
 - CURL:
   . Fixed bug #77264 (curl_getinfo returning microseconds, not seconds).

diff --git a/Zend/tests/bug77291.phpt b/Zend/tests/bug77291.phpt
@@ -0,0 +1,42 @@
+--TEST--
+Bug #77291 (magic methods inherited from a trait may be ignored)
+--FILE--
+<?php
+
+trait AccessibleProperties
+{
+    public function __isset($property)
+    {
+        return property_exists($this, $property);
+    }
+
+    public function __get($property)
+    {
+        if (property_exists($this, $property)) {
+            return $this->$property;
+        }
+    }
+}
+
+class Foo4567 {
+    use AccessibleProperties;
+
+    protected $a = 'Some value';
+}
+
+class Foo45 {
+    use AccessibleProperties;
+
+    protected $a = 'Some value';
+}
+
+$foo = new Foo4567;
+var_dump(isset($foo->a));
+$foo = new Foo45;
+var_dump($foo->a);
+?>
+===DONE===
+--EXPECT--
+bool(true)
+string(10) "Some value"
+===DONE===
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -1126,18 +1126,7 @@ static zend_bool zend_traits_method_compatibility_check(zend_function *fn, zend_
 
 static void zend_add_magic_methods(zend_class_entry* ce, zend_string* mname, zend_function* fe) /* {{{ */
 {
-	if (ZSTR_LEN(ce->name) == ZSTR_LEN(mname)) {
-		zend_string *lowercase_name = zend_string_tolower(ce->name);
-		lowercase_name = zend_new_interned_string(lowercase_name);
-		if (!memcmp(ZSTR_VAL(mname), ZSTR_VAL(lowercase_name), ZSTR_LEN(mname))) {
-			if (ce->constructor  && (!ce->parent || ce->constructor != ce->parent->constructor)) {
-				zend_error_noreturn(E_COMPILE_ERROR, "%s has colliding constructor definitions coming from traits", ZSTR_VAL(ce->name));
-			}
-			ce->constructor = fe;
-			fe->common.fn_flags |= ZEND_ACC_CTOR;
-		}
-		zend_string_release_ex(lowercase_name, 0);
-	} else if (ZSTR_VAL(mname)[0] != '_' || ZSTR_VAL(mname)[1] != '_') {
+	if (ZSTR_LEN(ce->name) != ZSTR_LEN(mname) && (ZSTR_VAL(mname)[0] != '_' || ZSTR_VAL(mname)[1] != '_')) {
 		/* pass */
 	} else if (zend_string_equals_literal(mname, ZEND_CLONE_FUNC_NAME)) {
 		ce->clone = fe;
@@ -1168,6 +1157,17 @@ static void zend_add_magic_methods(zend_class_entry* ce, zend_string* mname, zen
 		ce->__tostring = fe;
 	} else if (zend_string_equals_literal(mname, ZEND_DEBUGINFO_FUNC_NAME)) {
 		ce->__debugInfo = fe;
+	} else if (ZSTR_LEN(ce->name) == ZSTR_LEN(mname)) {
+		zend_string *lowercase_name = zend_string_tolower(ce->name);
+		lowercase_name = zend_new_interned_string(lowercase_name);
+		if (!memcmp(ZSTR_VAL(mname), ZSTR_VAL(lowercase_name), ZSTR_LEN(mname))) {
+			if (ce->constructor  && (!ce->parent || ce->constructor != ce->parent->constructor)) {
+				zend_error_noreturn(E_COMPILE_ERROR, "%s has colliding constructor definitions coming from traits", ZSTR_VAL(ce->name));
+			}
+			ce->constructor = fe;
+			fe->common.fn_flags |= ZEND_ACC_CTOR;
+		}
+		zend_string_release_ex(lowercase_name, 0);
 	}
 }
 /* }}} */