diff --git a/NEWS b/NEWS index 98f5bf7e718d..5a43235e09bc 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.24 +- Core: + . Fixed bug GH-22354 (zend_compile_implements() assumes that the class entry + has no interfaces already). (DanielEScherzer) + - Reflection: . Fixed bug GH-22324 (Ignore leading namespace separator in ReflectionParameter::__construct()). (jorgsowa) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 42c136d6bf36..b26537eeb1b0 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8963,17 +8963,97 @@ static void zend_compile_implements(zend_ast *ast) /* {{{ */ zend_class_name *interface_names; uint32_t i; - interface_names = emalloc(sizeof(zend_class_name) * list->children); + // Attribute validators run before `implements` parts are compiled, an + // internal attribute from an extension might have added an interface + // already so we cannot assume that the list of interfaces is empty. + // See GH-22354. The attribute validator might have also added an interface + // that the user is trying to manually implement, skip those since otherwise + // there are errors about trying to implement an interface that was already + // implemented. + if (EXPECTED(ce->num_interfaces == 0)) { + interface_names = emalloc(sizeof(zend_class_name) * list->children); + for (i = 0; i < list->children; ++i) { + zend_ast *class_ast = list->child[i]; + interface_names[i].name = + zend_resolve_const_class_name_reference(class_ast, "interface name"); + interface_names[i].lc_name = zend_string_tolower(interface_names[i].name); + } + + ce->num_interfaces = list->children; + ce->interface_names = interface_names; + return; + } + // Figure out which interfaces in the list should be skipped; first, resolve + // the names + // BUT, only skip the *first* usage of an interface in the manual `implements` + // part, if an interface is added by an attribute but also manually declared + // twice it should still be warned about + zend_string **to_add_names = safe_emalloc(list->children, sizeof(*to_add_names), 0); + zend_string **skipped_names = safe_emalloc(list->children, sizeof(*skipped_names), 0); + uint32_t to_add_count = 0; + uint32_t skipped_count = 0; for (i = 0; i < list->children; ++i) { zend_ast *class_ast = list->child[i]; - interface_names[i].name = - zend_resolve_const_class_name_reference(class_ast, "interface name"); - interface_names[i].lc_name = zend_string_tolower(interface_names[i].name); + zend_string *name = zend_resolve_const_class_name_reference(class_ast, "interface name"); + bool already_added = false; + for (uint32_t idx = 0; idx < ce->num_interfaces; ++idx) { + if (zend_string_equals_ci(name, ce->interface_names[idx].name)) { + already_added = true; + break; + } + } + if (already_added) { + // Did we already skip this interface name once? + bool already_skipped = false; + for (uint32_t idx = 0; idx < skipped_count; ++idx) { + if (zend_string_equals_ci(name, skipped_names[idx])) { + already_skipped = true; + break; + } + } + if (already_skipped) { + to_add_names[i] = name; + to_add_count++; + } else { + to_add_names[i] = NULL; + skipped_names[i] = name; + skipped_count++; + } + } else { + to_add_names[i] = name; + to_add_count++; + } + } + ZEND_ASSERT(skipped_count + to_add_count == list->children); + + // Free the skipped names + for (uint32_t idx = 0; idx < skipped_count; ++idx) { + zend_string_release(skipped_names[idx]); + } + efree(skipped_names); + + const uint32_t initial_count = ce->num_interfaces; + interface_names = safe_erealloc(ce->interface_names, (initial_count + to_add_count), sizeof(*interface_names), 0); + + uint32_t added_count = 0; + for (i = 0; i < list->children; ++i) { + zend_string *name = to_add_names[i]; + if (name == NULL) { + // This was one of the names that was already added by a validator + continue; + } + interface_names[initial_count + added_count].name = name; + interface_names[initial_count + added_count].lc_name = zend_string_tolower(name); + // To make it clear that the to_add_names no longer owns the reference + to_add_names[i] = NULL; + added_count += 1; } + ZEND_ASSERT(added_count == to_add_count); - ce->num_interfaces = list->children; + ce->num_interfaces = initial_count + added_count; ce->interface_names = interface_names; + efree(to_add_names); } /* }}} */ diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 576bacd5e4a9..92a4bd988e16 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -30,6 +30,7 @@ #include "zend_attributes.h" #include "zend_enum.h" #include "zend_interfaces.h" +#include "zend_inheritance.h" #include "zend_weakrefs.h" #include "Zend/Optimizer/zend_optimizer.h" #include "Zend/zend_alloc.h" @@ -61,6 +62,7 @@ static zend_class_entry *zend_test_attribute; static zend_class_entry *zend_test_repeatable_attribute; static zend_class_entry *zend_test_parameter_attribute; static zend_class_entry *zend_test_property_attribute; +static zend_class_entry *zend_test_attribute_add_interface; static zend_class_entry *zend_test_attribute_with_arguments; static zend_class_entry *zend_test_class_with_method_with_parameter_attribute; static zend_class_entry *zend_test_child_class_with_method_with_parameter_attribute; @@ -913,6 +915,95 @@ void zend_attribute_validate_zendtestattribute(zend_attribute *attr, uint32_t ta } } +/** + * Recursive handler to check if a class implements the custom casting + * interface, either directly or via inheritance. + * + * Technically this shouldn't be needed since attribute validators run before + * interfaces are added and classes are linked, but better safe than sorry + */ +static bool check_class_has_interface(zend_class_entry *scope) { + // Might be *linked* (so already have class entries) but not *resolved*, + // since that waits until the inherited parent class is resolved + if (scope->ce_flags & ZEND_ACC_LINKED) { + for (uint32_t iii = 0; iii < scope->num_interfaces; iii++) { + if (scope->interfaces[iii] == zend_test_interface) { + return true; + } + } + } else { + for (uint32_t iii = 0; iii < scope->num_interfaces; iii++) { + if (zend_string_equals_literal( + scope->interface_names[iii].lc_name, + "_zendtestinterface" + )) { + // Interface was added manually be the developer + return true; + } + } + } + zend_class_entry *parent = NULL; + if (scope->ce_flags & ZEND_ACC_LINKED) { + if (scope->parent == NULL) { + return false; + } + parent = scope->parent; + } else if (scope->parent_name == NULL) { + return false; + } else { + parent = zend_lookup_class_ex( + scope->parent_name, + NULL, + ZEND_FETCH_CLASS_ALLOW_UNLINKED + ); + } + if (parent == NULL) { + // Invalid class to extend? Leave that up to normal PHP to deal with + return false; + } + return check_class_has_interface(parent); +} + +void zend_attribute_validate_add_interface(zend_attribute *attr, uint32_t target, zend_class_entry *scope) +{ + if (target != ZEND_ATTRIBUTE_TARGET_CLASS) { + return; + } + if (scope->ce_flags & (ZEND_ACC_ENUM|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT)) { + zend_error_noreturn(E_ERROR, "Only classes can be marked with #[ZendTestAttributeAddsInterface]"); + } + if (check_class_has_interface(scope)) { + return; + } + if (scope->ce_flags & ZEND_ACC_LINKED) { + // There is already a method to add interfaces + zend_do_implement_interface(scope, zend_test_interface); + return; + } + + // Add the interface automatically to the list + const uint32_t interfaceIdx = scope->num_interfaces; + scope->num_interfaces++; + + zend_class_name *newInterfaceSet = safe_erealloc( + scope->interface_names, + scope->num_interfaces, + sizeof(*newInterfaceSet), + 0 + ); + newInterfaceSet[interfaceIdx].name = zend_string_init( + "_ZendTestInterface", + strlen("_ZendTestInterface"), + 0 + ); + newInterfaceSet[interfaceIdx].lc_name = zend_string_init( + "_zendtestinterface", + strlen("_zendtestinterface"), + 0 + ); + scope->interface_names = newInterfaceSet; +} + static ZEND_METHOD(_ZendTestClass, __toString) { ZEND_PARSE_PARAMETERS_NONE(); @@ -1296,6 +1387,12 @@ PHP_MINIT_FUNCTION(zend_test) zend_test_property_attribute = register_class_ZendTestPropertyAttribute(); zend_mark_internal_attribute(zend_test_property_attribute); + zend_test_attribute_add_interface = register_class_ZendTestAttributeAddsInterface(); + { + zend_internal_attribute *attr = zend_mark_internal_attribute(zend_test_attribute_add_interface); + attr->validator = zend_attribute_validate_add_interface; + } + zend_test_attribute_with_arguments = register_class_ZendTestAttributeWithArguments(); zend_mark_internal_attribute(zend_test_attribute_with_arguments); diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 9116245c30f4..a954015307a6 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -154,6 +154,9 @@ final class ZendTestPropertyAttribute { public function __construct(string $parameter) {} } + #[Attribute(Attribute::TARGET_CLASS)] + final class ZendTestAttributeAddsInterface {} + class ZendTestClassWithMethodWithParameterAttribute { final public function no_override( #[ZendTestParameterAttribute("value2")] diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 039757207e69..4e81903ac8b6 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: bf65e1dd1eeeeec46687a76a7ea6554cd1971dfc */ + * Stub hash: 90fded8eabc29d73f7c83ae91ea7685fe286545f */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -1032,6 +1032,28 @@ static zend_class_entry *register_class_ZendTestPropertyAttribute(void) return class_entry; } +static zend_class_entry *register_class_ZendTestAttributeAddsInterface(void) +{ + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, "ZendTestAttributeAddsInterface", NULL); +#if (PHP_VERSION_ID >= 80400) + class_entry = zend_register_internal_class_with_flags(&ce, NULL, ZEND_ACC_FINAL); +#else + class_entry = zend_register_internal_class_ex(&ce, NULL); + class_entry->ce_flags |= ZEND_ACC_FINAL; +#endif + + zend_string *attribute_name_Attribute_class_ZendTestAttributeAddsInterface_0 = zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); + zend_attribute *attribute_Attribute_class_ZendTestAttributeAddsInterface_0 = zend_add_class_attribute(class_entry, attribute_name_Attribute_class_ZendTestAttributeAddsInterface_0, 1); + zend_string_release(attribute_name_Attribute_class_ZendTestAttributeAddsInterface_0); + zval attribute_Attribute_class_ZendTestAttributeAddsInterface_0_arg0; + ZVAL_LONG(&attribute_Attribute_class_ZendTestAttributeAddsInterface_0_arg0, ZEND_ATTRIBUTE_TARGET_CLASS); + ZVAL_COPY_VALUE(&attribute_Attribute_class_ZendTestAttributeAddsInterface_0->args[0].value, &attribute_Attribute_class_ZendTestAttributeAddsInterface_0_arg0); + + return class_entry; +} + static zend_class_entry *register_class_ZendTestClassWithMethodWithParameterAttribute(void) { zend_class_entry ce, *class_entry; diff --git a/ext/zend_test/tests/attribute-adds-interface-01.phpt b/ext/zend_test/tests/attribute-adds-interface-01.phpt new file mode 100644 index 000000000000..0d4e31be9895 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-01.phpt @@ -0,0 +1,18 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (no manual implements) +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +array(1) { + ["_ZendTestInterface"]=> + string(18) "_ZendTestInterface" +} diff --git a/ext/zend_test/tests/attribute-adds-interface-02.phpt b/ext/zend_test/tests/attribute-adds-interface-02.phpt new file mode 100644 index 000000000000..e2d0f63d9de8 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-02.phpt @@ -0,0 +1,18 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement same interface) +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +array(1) { + ["_ZendTestInterface"]=> + string(18) "_ZendTestInterface" +} diff --git a/ext/zend_test/tests/attribute-adds-interface-03.phpt b/ext/zend_test/tests/attribute-adds-interface-03.phpt new file mode 100644 index 000000000000..982af8b236b6 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-03.phpt @@ -0,0 +1,22 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement different interface) +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +array(2) { + ["_ZendTestInterface"]=> + string(18) "_ZendTestInterface" + ["MyInterface"]=> + string(11) "MyInterface" +} diff --git a/ext/zend_test/tests/attribute-adds-interface-04.phpt b/ext/zend_test/tests/attribute-adds-interface-04.phpt new file mode 100644 index 000000000000..8bb282533fe7 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-04.phpt @@ -0,0 +1,15 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] and manually implementing same interface repeatedly errors +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECTF-- +Fatal error: Class Demo cannot implement previously implemented interface _ZendTestInterface in %s on line %d diff --git a/ext/zend_test/tests/attribute-adds-interface-05.phpt b/ext/zend_test/tests/attribute-adds-interface-05.phpt new file mode 100644 index 000000000000..c667acd8c592 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-05.phpt @@ -0,0 +1,17 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] and manually implementing a different interface repeatedly errors +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECTF-- +Fatal error: Class Demo cannot implement previously implemented interface MyInterface in %s on line %d