-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix GH-16188: Handle references after fetching Exception properties #16196
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
Conversation
Zend/zend_exceptions.c
Outdated
|
||
ZEND_PARSE_PARAMETERS_NONE(); | ||
|
||
ZVAL_COPY(return_value, GET_PROPERTY_SILENT(ZEND_THIS, ZEND_STR_PREVIOUS)); | ||
prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_PREVIOUS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the change from silent to non-silent intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Zend/zend_exceptions.c
Outdated
ZVAL_DEREF(prop); | ||
ZVAL_COPY(return_value, prop); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have ZVAL_COPY_DEREF, although I see pre-existing places with this pattern too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
break; | ||
} | ||
} | ||
zend_string_release_ex(fname, 0); | ||
|
||
exception = ZEND_THIS; | ||
/* Reset apply counts */ | ||
while (exception && Z_TYPE_P(exception) == IS_OBJECT && (base_ce = i_get_exception_base(Z_OBJ_P(exception))) && instanceof_function(Z_OBJCE_P(exception), base_ce)) { | ||
while (Z_TYPE_P(exception) == IS_OBJECT && (base_ce = i_get_exception_base(Z_OBJ_P(exception))) && instanceof_function(Z_OBJCE_P(exception), base_ce)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of cleanup: we have a similar exception
NULL-check on line 670. But that doesn't need to be in this PR.
* PHP-8.2: NEWS for GH-16196 Handle references properties of the Exception class
Fixes GH-16188.
Exception
assumes that the result ofzend_read_property()
have the right type, relying on property type hints. However the property value may also be a reference so we have to deref.This was already done for
protected
properties as it's possible to assign them directly in sub-class, but not forprivate
ones.