From 26253b39934d4202c7eff8fcbb8c929b864c4325 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 17 Jun 2026 11:05:40 -0700 Subject: [PATCH 1/2] Add regression tests for interfaces added via attribute validators --- ext/zend_test/test.c | 97 +++++++++++++++++++ ext/zend_test/test.stub.php | 3 + ext/zend_test/test_arginfo.h | 24 ++++- .../tests/attribute-adds-interface-01.phpt | 18 ++++ .../tests/attribute-adds-interface-02.phpt | 20 ++++ .../tests/attribute-adds-interface-03.phpt | 24 +++++ 6 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 ext/zend_test/tests/attribute-adds-interface-01.phpt create mode 100644 ext/zend_test/tests/attribute-adds-interface-02.phpt create mode 100644 ext/zend_test/tests/attribute-adds-interface-03.phpt 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..75a12de47143 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-02.phpt @@ -0,0 +1,20 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement same interface) +--XFAIL-- +Currently leaks +--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..6f9904227bd2 --- /dev/null +++ b/ext/zend_test/tests/attribute-adds-interface-03.phpt @@ -0,0 +1,24 @@ +--TEST-- +Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement different interface) +--XFAIL-- +Currently leaks and overwrites the interface added by the attribute +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +array(2) { + ["_ZendTestInterface"]=> + string(18) "_ZendTestInterface" + ["MyInterface"]=> + string(11) "MyInterface" +} From 094b72f817ec1da5b600c1a245625aa5e601dfea Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 17 Jun 2026 11:42:09 -0700 Subject: [PATCH 2/2] GH-22354: support interfaces added by attribute validators In `zend_compile_implements()`, when there were no interfaces already added, keep the existing code of just adding the newly declared interfaces. But, when some interfaces are already present, don't overwrite them. Instead, add to the existing list. In case the developer tries to add an interface that was already added by an attribute, skip the manual interface addition to avoid errors about trying to apply the same interface multiple times. But, only skip the *first* manual interface addition of the same interface, if there are multiple such additions then there really was an attempt to apply the same interface multiple times. Fixes #22354 --- NEWS | 4 + Zend/zend_compile.c | 90 +++++++++++++++++-- .../tests/attribute-adds-interface-02.phpt | 2 - .../tests/attribute-adds-interface-03.phpt | 2 - .../tests/attribute-adds-interface-04.phpt | 15 ++++ .../tests/attribute-adds-interface-05.phpt | 17 ++++ 6 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 ext/zend_test/tests/attribute-adds-interface-04.phpt create mode 100644 ext/zend_test/tests/attribute-adds-interface-05.phpt 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/tests/attribute-adds-interface-02.phpt b/ext/zend_test/tests/attribute-adds-interface-02.phpt index 75a12de47143..e2d0f63d9de8 100644 --- a/ext/zend_test/tests/attribute-adds-interface-02.phpt +++ b/ext/zend_test/tests/attribute-adds-interface-02.phpt @@ -1,7 +1,5 @@ --TEST-- Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement same interface) ---XFAIL-- -Currently leaks --EXTENSIONS-- zend_test --FILE-- diff --git a/ext/zend_test/tests/attribute-adds-interface-03.phpt b/ext/zend_test/tests/attribute-adds-interface-03.phpt index 6f9904227bd2..982af8b236b6 100644 --- a/ext/zend_test/tests/attribute-adds-interface-03.phpt +++ b/ext/zend_test/tests/attribute-adds-interface-03.phpt @@ -1,7 +1,5 @@ --TEST-- Verify that #[ZendTestAttributeAddsInterface] adding an interface doesn't leak (manually implement different interface) ---XFAIL-- -Currently leaks and overwrites the interface added by the attribute --EXTENSIONS-- zend_test --FILE-- 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