Skip to content

Commit 2f27e0b

Browse files
authored
Fix missing variance check for abstract set with asymmetric type (#15157)
Fixes GH-15140
1 parent 11094d5 commit 2f27e0b

File tree

3 files changed

+68
-15
lines changed

3 files changed

+68
-15
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ PHP NEWS
66
. Updated build system scripts config.guess to 2024-07-27 and config.sub to
77
2024-05-27. (Peter Kokot)
88
. Fixed bug GH-15240 (Infinite recursion in trait hook). (ilutov)
9+
. Fixed bug GH-15140 (Missing variance check for abstract set with asymmetric
10+
type). (ilutov)
911

1012
- Date:
1113
. Constants SUNFUNCS_RET_TIMESTAMP, SUNFUNCS_RET_STRING, and SUNFUNCS_RET_DOUBLE
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-15140: Asymmetric set type cannot be satisfied by plain property
3+
--FILE--
4+
<?php
5+
6+
interface I {
7+
public string $prop {
8+
set(int|string $value);
9+
}
10+
}
11+
class C implements I {
12+
public string $prop;
13+
}
14+
15+
?>
16+
--EXPECTF--
17+
Fatal error: Set type of C::$prop must be supertype of string|int (as in interface I) in %s on line %d

Zend/zend_inheritance.c

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,7 +1242,7 @@ static void do_inherit_method(zend_string *key, zend_function *parent, zend_clas
12421242
}
12431243
/* }}} */
12441244

1245-
static inheritance_status property_types_compatible(
1245+
static inheritance_status full_property_types_compatible(
12461246
const zend_property_info *parent_info, const zend_property_info *child_info,
12471247
prop_variance variance) {
12481248
if (ZEND_TYPE_PURE_MASK(parent_info->type) == ZEND_TYPE_PURE_MASK(child_info->type)
@@ -1271,7 +1271,7 @@ static inheritance_status property_types_compatible(
12711271
return INHERITANCE_UNRESOLVED;
12721272
}
12731273

1274-
static void emit_incompatible_property_error(
1274+
static ZEND_COLD void emit_incompatible_property_error(
12751275
const zend_property_info *child, const zend_property_info *parent, prop_variance variance) {
12761276
zend_string *type_str = zend_type_to_string_resolved(parent->type, parent->ce);
12771277
zend_error_noreturn(E_COMPILE_ERROR,
@@ -1284,6 +1284,48 @@ static void emit_incompatible_property_error(
12841284
ZSTR_VAL(parent->ce->name));
12851285
}
12861286

1287+
static ZEND_COLD void emit_set_hook_type_error(const zend_property_info *child, const zend_property_info *parent)
1288+
{
1289+
zend_type set_type = parent->hooks[ZEND_PROPERTY_HOOK_SET]->common.arg_info[0].type;
1290+
zend_string *type_str = zend_type_to_string_resolved(set_type, parent->ce);
1291+
zend_error_noreturn(E_COMPILE_ERROR,
1292+
"Set type of %s::$%s must be supertype of %s (as in %s %s)",
1293+
ZSTR_VAL(child->ce->name),
1294+
zend_get_unmangled_property_name(child->name),
1295+
ZSTR_VAL(type_str),
1296+
zend_get_object_type_case(parent->ce, false),
1297+
ZSTR_VAL(parent->ce->name));
1298+
}
1299+
1300+
static inheritance_status verify_property_type_compatibility(
1301+
const zend_property_info *parent_info,
1302+
const zend_property_info *child_info,
1303+
prop_variance variance,
1304+
bool throw_on_error,
1305+
bool throw_on_unresolved
1306+
) {
1307+
inheritance_status result = full_property_types_compatible(parent_info, child_info, variance);
1308+
if ((result == INHERITANCE_ERROR && throw_on_error) || (result == INHERITANCE_UNRESOLVED && throw_on_unresolved)) {
1309+
emit_incompatible_property_error(child_info, parent_info, variance);
1310+
}
1311+
if (result != INHERITANCE_SUCCESS) {
1312+
return result;
1313+
}
1314+
if (parent_info->flags & ZEND_ACC_ABSTRACT) {
1315+
ZEND_ASSERT(parent_info->hooks);
1316+
if (parent_info->hooks[ZEND_PROPERTY_HOOK_SET]
1317+
&& (!child_info->hooks || !child_info->hooks[ZEND_PROPERTY_HOOK_SET])) {
1318+
zend_type set_type = parent_info->hooks[ZEND_PROPERTY_HOOK_SET]->common.arg_info[0].type;
1319+
inheritance_status result = zend_perform_covariant_type_check(
1320+
parent_info->ce, set_type, child_info->ce, child_info->type);
1321+
if ((result == INHERITANCE_ERROR && throw_on_error) || (result == INHERITANCE_UNRESOLVED && throw_on_unresolved)) {
1322+
emit_set_hook_type_error(child_info, parent_info);
1323+
}
1324+
}
1325+
}
1326+
return INHERITANCE_SUCCESS;
1327+
}
1328+
12871329
static bool property_has_operation(zend_property_info *prop_info, zend_property_hook_kind kind)
12881330
{
12891331
return (!(prop_info->flags & ZEND_ACC_VIRTUAL)
@@ -1427,11 +1469,8 @@ static void do_inherit_property(zend_property_info *parent_info, zend_string *ke
14271469

14281470
prop_variance variance = prop_get_variance(parent_info);
14291471
if (ZEND_TYPE_IS_SET(parent_info->type)) {
1430-
inheritance_status status = property_types_compatible(
1431-
parent_info, child_info, variance);
1432-
if (status == INHERITANCE_ERROR) {
1433-
emit_incompatible_property_error(child_info, parent_info, variance);
1434-
}
1472+
inheritance_status status = verify_property_type_compatibility(
1473+
parent_info, child_info, variance, true, false);
14351474
if (status == INHERITANCE_UNRESOLVED) {
14361475
add_property_compatibility_obligation(ce, child_info, parent_info, variance);
14371476
}
@@ -2747,7 +2786,7 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent
27472786
}
27482787

27492788
if ((colliding_prop->flags & flags_mask) == (flags & flags_mask) &&
2750-
property_types_compatible(property_info, colliding_prop, PROP_INVARIANT) == INHERITANCE_SUCCESS
2789+
verify_property_type_compatibility(property_info, colliding_prop, PROP_INVARIANT, false, false) == INHERITANCE_SUCCESS
27512790
) {
27522791
/* the flags are identical, thus, the properties may be compatible */
27532792
zval *op1, *op2;
@@ -3099,11 +3138,7 @@ static void check_variance_obligation(variance_obligation *obligation) {
30993138
}
31003139
/* Either the compatibility check was successful or only threw a warning. */
31013140
} else if (obligation->type == OBLIGATION_PROPERTY_COMPATIBILITY) {
3102-
inheritance_status status =
3103-
property_types_compatible(obligation->parent_prop, obligation->child_prop, obligation->variance);
3104-
if (status != INHERITANCE_SUCCESS) {
3105-
emit_incompatible_property_error(obligation->child_prop, obligation->parent_prop, obligation->variance);
3106-
}
3141+
verify_property_type_compatibility(obligation->parent_prop, obligation->child_prop, obligation->variance, true, true);
31073142
} else if (obligation->type == OBLIGATION_CLASS_CONSTANT_COMPATIBILITY) {
31083143
inheritance_status status =
31093144
class_constant_types_compatible(obligation->parent_const, obligation->child_const);
@@ -3645,8 +3680,7 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e
36453680
if (zv) {
36463681
zend_property_info *child_info = Z_PTR_P(zv);
36473682
if (ZEND_TYPE_IS_SET(child_info->type)) {
3648-
inheritance_status status = property_types_compatible(parent_info, child_info, prop_get_variance(parent_info));
3649-
ZEND_ASSERT(status != INHERITANCE_WARNING);
3683+
inheritance_status status = verify_property_type_compatibility(parent_info, child_info, prop_get_variance(parent_info), false, false);
36503684
if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
36513685
return status;
36523686
}

0 commit comments

Comments
 (0)