Skip to content

Commit

Permalink
Properly deal with internal attributes used on promoted properties.
Browse files Browse the repository at this point in the history
Closes GH-9661
  • Loading branch information
kooldev authored and iluuu1994 committed Nov 3, 2022
1 parent 8e49d7f commit fdd088f
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 30 deletions.
2 changes: 2 additions & 0 deletions NEWS
Expand Up @@ -6,6 +6,8 @@ PHP NEWS
. Fixed bug GH-9823 (Don’t reset func in zend_closure_internal_handler).
(Florian Sowade)
. Fixed potential NULL pointer dereference Windows shm*() functions. (cmb)
. Fix target validation for internal attributes with constructor property
promotion. (kooldev)

- FPM:
. Fixed bug GH-9754 (SaltStack (using Python subprocess) hangs when running
Expand Down
76 changes: 47 additions & 29 deletions Zend/zend_compile.c
Expand Up @@ -6589,8 +6589,9 @@ static bool zend_is_valid_default_value(zend_type type, zval *value)
return 0;
}

static void zend_compile_attributes(HashTable **attributes, zend_ast *ast, uint32_t offset, uint32_t target) /* {{{ */
{
static void zend_compile_attributes(
HashTable **attributes, zend_ast *ast, uint32_t offset, uint32_t target, uint32_t promoted
) /* {{{ */ {
zend_attribute *attr;
zend_internal_attribute *config;

Expand All @@ -6616,8 +6617,20 @@ static void zend_compile_attributes(HashTable **attributes, zend_ast *ast, uint3
}

zend_string *name = zend_resolve_class_name_ast(el->child[0]);
zend_string *lcname = zend_string_tolower_ex(name, false);
zend_ast_list *args = el->child[1] ? zend_ast_get_list(el->child[1]) : NULL;

config = zend_internal_attribute_get(lcname);
zend_string_release(lcname);

/* Exclude internal attributes that do not match on promoted properties. */
if (config && !(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) {
if (promoted & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL)) {
zend_string_release(name);
continue;
}
}

uint32_t flags = (CG(active_op_array)->fn_flags & ZEND_ACC_STRICT_TYPES)
? ZEND_ATTRIBUTE_STRICT_TYPES : 0;
attr = zend_add_attribute(
Expand Down Expand Up @@ -6662,31 +6675,33 @@ static void zend_compile_attributes(HashTable **attributes, zend_ast *ast, uint3
}
}

/* Validate attributes in a secondary loop (needed to detect repeated attributes). */
ZEND_HASH_PACKED_FOREACH_PTR(*attributes, attr) {
if (attr->offset != offset || NULL == (config = zend_internal_attribute_get(attr->lcname))) {
continue;
}
if (*attributes != NULL) {
/* Validate attributes in a secondary loop (needed to detect repeated attributes). */
ZEND_HASH_PACKED_FOREACH_PTR(*attributes, attr) {
if (attr->offset != offset || NULL == (config = zend_internal_attribute_get(attr->lcname))) {
continue;
}

if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) {
zend_string *location = zend_get_attribute_target_names(target);
zend_string *allowed = zend_get_attribute_target_names(config->flags);
if (!(target & (config->flags & ZEND_ATTRIBUTE_TARGET_ALL))) {
zend_string *location = zend_get_attribute_target_names(target);
zend_string *allowed = zend_get_attribute_target_names(config->flags);

zend_error_noreturn(E_ERROR, "Attribute \"%s\" cannot target %s (allowed targets: %s)",
ZSTR_VAL(attr->name), ZSTR_VAL(location), ZSTR_VAL(allowed)
);
}
zend_error_noreturn(E_ERROR, "Attribute \"%s\" cannot target %s (allowed targets: %s)",
ZSTR_VAL(attr->name), ZSTR_VAL(location), ZSTR_VAL(allowed)
);
}

if (!(config->flags & ZEND_ATTRIBUTE_IS_REPEATABLE)) {
if (zend_is_attribute_repeated(*attributes, attr)) {
zend_error_noreturn(E_ERROR, "Attribute \"%s\" must not be repeated", ZSTR_VAL(attr->name));
if (!(config->flags & ZEND_ATTRIBUTE_IS_REPEATABLE)) {
if (zend_is_attribute_repeated(*attributes, attr)) {
zend_error_noreturn(E_ERROR, "Attribute \"%s\" must not be repeated", ZSTR_VAL(attr->name));
}
}
}

if (config->validator != NULL) {
config->validator(attr, target, CG(active_class_entry));
}
} ZEND_HASH_FOREACH_END();
if (config->validator != NULL) {
config->validator(attr, target, CG(active_class_entry));
}
} ZEND_HASH_FOREACH_END();
}
}
/* }}} */

Expand Down Expand Up @@ -6821,7 +6836,10 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
arg_info->type = (zend_type) ZEND_TYPE_INIT_NONE(0);

if (attributes_ast) {
zend_compile_attributes(&op_array->attributes, attributes_ast, i + 1, ZEND_ATTRIBUTE_TARGET_PARAMETER);
zend_compile_attributes(
&op_array->attributes, attributes_ast, i + 1, ZEND_ATTRIBUTE_TARGET_PARAMETER,
property_flags ? ZEND_ATTRIBUTE_TARGET_PROPERTY : 0
);
}

if (type_ast) {
Expand Down Expand Up @@ -6927,7 +6945,7 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
scope, name, &default_value, property_flags | ZEND_ACC_PROMOTED, doc_comment, type);
if (attributes_ast) {
zend_compile_attributes(
&prop->attributes, attributes_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY);
&prop->attributes, attributes_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY, ZEND_ATTRIBUTE_TARGET_PARAMETER);
}
}
}
Expand Down Expand Up @@ -7364,7 +7382,7 @@ static void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel)
target = ZEND_ATTRIBUTE_TARGET_METHOD;
}

zend_compile_attributes(&op_array->attributes, decl->child[4], 0, target);
zend_compile_attributes(&op_array->attributes, decl->child[4], 0, target, 0);
}

/* Do not leak the class scope into free standing functions, even if they are dynamically
Expand Down Expand Up @@ -7546,7 +7564,7 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f
info = zend_declare_typed_property(ce, name, &value_zv, flags, doc_comment, type);

if (attr_ast) {
zend_compile_attributes(&info->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY);
zend_compile_attributes(&info->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_PROPERTY, 0);
}
}
}
Expand Down Expand Up @@ -7607,7 +7625,7 @@ static void zend_compile_class_const_decl(zend_ast *ast, uint32_t flags, zend_as
c = zend_declare_class_constant_ex(ce, name, &value_zv, flags, doc_comment);

if (attr_ast) {
zend_compile_attributes(&c->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_CLASS_CONST);
zend_compile_attributes(&c->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_CLASS_CONST, 0);
}
}
}
Expand Down Expand Up @@ -7869,7 +7887,7 @@ static void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel)
CG(active_class_entry) = ce;

if (decl->child[3]) {
zend_compile_attributes(&ce->attributes, decl->child[3], 0, ZEND_ATTRIBUTE_TARGET_CLASS);
zend_compile_attributes(&ce->attributes, decl->child[3], 0, ZEND_ATTRIBUTE_TARGET_CLASS, 0);
}

if (implements_ast) {
Expand Down Expand Up @@ -8027,7 +8045,7 @@ static void zend_compile_enum_case(zend_ast *ast)

zend_ast *attr_ast = ast->child[3];
if (attr_ast) {
zend_compile_attributes(&c->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_CLASS_CONST);
zend_compile_attributes(&c->attributes, attr_ast, 0, ZEND_ATTRIBUTE_TARGET_CLASS_CONST, 0);
}
}

Expand Down
15 changes: 15 additions & 0 deletions ext/zend_test/test.c
Expand Up @@ -39,6 +39,7 @@ static zend_class_entry *zend_test_child_class;
static zend_class_entry *zend_test_trait;
static zend_class_entry *zend_test_attribute;
static zend_class_entry *zend_test_parameter_attribute;
static zend_class_entry *zend_test_property_attribute;
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;
static zend_class_entry *zend_test_forbid_dynamic_call;
Expand Down Expand Up @@ -587,6 +588,17 @@ static ZEND_METHOD(ZendTestParameterAttribute, __construct)
ZVAL_STR_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), 0), parameter);
}

static ZEND_METHOD(ZendTestPropertyAttribute, __construct)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

ZVAL_STR_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), 0), parameter);
}

static ZEND_METHOD(ZendTestClassWithMethodWithParameterAttribute, no_override)
{
zend_string *parameter;
Expand Down Expand Up @@ -688,6 +700,9 @@ PHP_MINIT_FUNCTION(zend_test)
ZVAL_PSTRING(&attr->args[0].value, "value1");
}

zend_test_property_attribute = register_class_ZendTestPropertyAttribute();
zend_mark_internal_attribute(zend_test_property_attribute);

zend_test_class_with_method_with_parameter_attribute = register_class_ZendTestClassWithMethodWithParameterAttribute();

{
Expand Down
7 changes: 7 additions & 0 deletions ext/zend_test/test.stub.php
Expand Up @@ -66,6 +66,13 @@ final class ZendTestParameterAttribute {
public function __construct(string $parameter) {}
}

#[Attribute(Attribute::TARGET_PROPERTY)]
final class ZendTestPropertyAttribute {
public string $parameter;

public function __construct(string $parameter) {}
}

class ZendTestClassWithMethodWithParameterAttribute {
final public function no_override(string $parameter): int {}
public function override(string $parameter): int {}
Expand Down
37 changes: 36 additions & 1 deletion ext/zend_test/test_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions ext/zend_test/tests/attribute-promotion-parameter-only.phpt
@@ -0,0 +1,31 @@
--TEST--
Attribute on promoted property may only target parameter
--EXTENSIONS--
zend_test
--FILE--
<?php

class AttrTest
{
public function __construct(
#[ZendTestParameterAttribute('foo')] public $param
) {}
}

$ref = new ReflectionClass(AttrTest::class);
$attr = $ref->getConstructor()->getParameters()[0]->getAttributes();

var_dump(count($attr));
var_dump($attr[0]->getName());
var_dump($attr[0]->newInstance()->parameter);

$attr = $ref->getProperty('param')->getAttributes();

var_dump(count($attr));

?>
--EXPECTF--
int(1)
string(26) "ZendTestParameterAttribute"
string(3) "foo"
int(0)
31 changes: 31 additions & 0 deletions ext/zend_test/tests/attribute-promotion-property-only.phpt
@@ -0,0 +1,31 @@
--TEST--
Attribute on promoted property may only target property
--EXTENSIONS--
zend_test
--FILE--
<?php

class AttrTest
{
public function __construct(
#[ZendTestPropertyAttribute('foo')] public $param
) {}
}

$ref = new ReflectionClass(AttrTest::class);
$attr = $ref->getConstructor()->getParameters()[0]->getAttributes();

var_dump(count($attr));

$attr = $ref->getProperty('param')->getAttributes();

var_dump(count($attr));
var_dump($attr[0]->getName());
var_dump($attr[0]->newInstance()->parameter);

?>
--EXPECTF--
int(0)
int(1)
string(25) "ZendTestPropertyAttribute"
string(3) "foo"

0 comments on commit fdd088f

Please sign in to comment.