Skip to content

Improve magic __get and property type inconsistency error message #9436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/tests/type_declarations/typed_properties_030.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ unset($foo->bar); # ok
var_dump($foo->bar); # not okay, __get is nasty
?>
--EXPECTF--
Fatal error: Uncaught TypeError: Cannot assign string to property Foo::$bar of type int in %s:%d
Fatal error: Uncaught TypeError: Value of type string returned from __get is inconsistent with property Foo::$bar of type int in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/type_declarations/typed_properties_040.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var_dump($foo->bar);
--EXPECTF--
string(3) "bar"

Fatal error: Uncaught TypeError: Cannot assign null to property Foo::$bar of type int in %s:%d
Fatal error: Uncaught TypeError: Value of type null returned from __get is inconsistent with property Foo::$bar of type int in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/type_declarations/typed_properties_074.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ object(Test)#1 (1) {
["val"]=>
uninitialized(int)
}
Cannot assign string to property Test::$val of type int
Value of type string returned from __get is inconsistent with property Test::$val of type int
object(Test)#1 (1) {
["prop"]=>
&string(1) "x"
Expand Down
32 changes: 30 additions & 2 deletions Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,24 @@ ZEND_COLD zend_never_inline void zend_verify_property_type_error(zend_property_i
zend_string_release(type_str);
}

ZEND_COLD zend_never_inline void zend_magic_get_property_type_inconsistency_error(zend_property_info *info, zval *property)
{
zend_string *type_str;

/* we _may_ land here in case reading already errored and runtime cache thus has not been updated (i.e. it contains a valid but unrelated info) */
if (EG(exception)) {
return;
}

type_str = zend_type_to_string(info->type);
zend_type_error("Value of type %s returned from __get is inconsistent with property %s::$%s of type %s",
zend_zval_type_name(property),
ZSTR_VAL(info->ce->name),
zend_get_unmangled_property_name(info->name),
ZSTR_VAL(type_str));
zend_string_release(type_str);
}

ZEND_COLD void zend_match_unhandled_error(zval *value)
{
smart_str msg = {0};
Expand Down Expand Up @@ -3560,7 +3578,7 @@ ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, ze
return variable_ptr;
}

ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref_ex(zend_property_info *prop_info, zval *orig_val, bool strict, zend_verify_prop_assignable_by_ref_context context) {
zval *val = orig_val;
if (Z_ISREF_P(val) && ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(val))) {
int result;
Expand Down Expand Up @@ -3591,10 +3609,20 @@ ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_inf
}
}

zend_verify_property_type_error(prop_info, val);
if (EXPECTED(context == ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT)) {
zend_verify_property_type_error(prop_info, val);
} else {
ZEND_ASSERT(context == ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET);
zend_magic_get_property_type_inconsistency_error(prop_info, val);
}

return 0;
}

ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
return zend_verify_prop_assignable_by_ref_ex(prop_info, orig_val, strict, ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT);
}

ZEND_API void ZEND_FASTCALL zend_ref_add_type_source(zend_property_info_source_list *source_list, zend_property_info *prop)
{
zend_property_info_list *list;
Expand Down
7 changes: 7 additions & 0 deletions Zend/zend_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ ZEND_COLD void ZEND_FASTCALL zend_param_must_be_ref(const zend_function *func, u
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_use_resource_as_offset(const zval *dim);

ZEND_API bool ZEND_FASTCALL zend_verify_ref_assignable_zval(zend_reference *ref, zval *zv, bool strict);

typedef enum {
ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_ASSIGNMENT,
ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET,
} zend_verify_prop_assignable_by_ref_context;
ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref_ex(zend_property_info *prop_info, zval *orig_val, bool strict, zend_verify_prop_assignable_by_ref_context context);
ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict);

ZEND_API ZEND_COLD void zend_throw_ref_type_error_zval(zend_property_info *prop, zval *zv);
Expand Down Expand Up @@ -449,6 +455,7 @@ ZEND_API int ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *call);

ZEND_API bool zend_verify_property_type(zend_property_info *info, zval *property, bool strict);
ZEND_COLD void zend_verify_property_type_error(zend_property_info *info, zval *property);
ZEND_COLD void zend_magic_get_property_type_inconsistency_error(zend_property_info *info, zval *property);

#define ZEND_REF_ADD_TYPE_SOURCE(ref, source) \
zend_ref_add_type_source(&ZEND_REF_TYPE_SOURCES(ref), source)
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_object_handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
}

if (UNEXPECTED(prop_info)) {
zend_verify_prop_assignable_by_ref(prop_info, retval, (zobj->ce->__get->common.fn_flags & ZEND_ACC_STRICT_TYPES) != 0);
zend_verify_prop_assignable_by_ref_ex(prop_info, retval, (zobj->ce->__get->common.fn_flags & ZEND_ACC_STRICT_TYPES) != 0, ZEND_VERIFY_PROP_ASSIGNABLE_BY_REF_CONTEXT_MAGIC_GET);
}

OBJ_RELEASE(zobj);
Expand Down