Add API functions for zend_exception #5157
Conversation
morrisonlevi
force-pushed
the
levim/exception-api
branch
2 times, most recently
from
b53bd3e
to
971ffb3
Compare
| ZVAL_DEREF(prop); | ||
| if (Z_TYPE_P(prop) != IS_LONG) { | ||
| if (!silent) { | ||
| zend_type_error("Exception %s was not a long", ZSTR_VAL(ZSTR_KNOWN(id))); |
There was a problem hiding this comment.
Might it make more sense to switch these to use typed properties, so this is enforced with the now built-in mechanism? Seems like something we can do in PHP 8...
There was a problem hiding this comment.
Definitely, it makes no sense to check types during property reading. The wrong values must not be written there at all.
There was a problem hiding this comment.
Can typed properties be subverted via reflection?
There was a problem hiding this comment.
The problem is certainly PDOException here where code is a string. Same could be true for any other userland exception which just writes to $this->code. Changing code to a typed string is a serious BC break.
morrisonlevi
force-pushed
the
levim/exception-api
branch
from
971ffb3
to
bdd44dd
Compare
- zend_exception_get_code - zend_exception_get_file - zend_exception_get_line - zend_exception_get_message - zend_exception_get_trace - zend_exception_get_trace_as_string. This accepts an arg for controlling whether the arguments will be included.
morrisonlevi
force-pushed
the
levim/exception-api
branch
from
bdd44dd
to
0f65960
Compare
|
I have rebased on master and removed the Regarding using typed properties: can it be subverted via reflection? If not, are there any examples of typed properties in core already? |
| ZEND_API zend_long *zend_exception_get_line(zend_object *exception, zend_bool silent); | ||
| ZEND_API zend_string *zend_exception_get_message(zend_object *exception, zend_bool silent); | ||
| ZEND_API HashTable *zend_exception_get_trace(zend_object *exception, zend_bool silent); | ||
| ZEND_API zend_string *zend_exception_get_trace_as_string(zend_object *exception, zend_bool include_args, zend_bool silent); |
There was a problem hiding this comment.
Can we have an argument here and in zend_exception_get_trace that limits the number of returned items to $n?
There was a problem hiding this comment.
Limiting the stack depth seems like a good idea.
|
This PR doesn't change the status quo -- there was a runtime error before and it has one now. I'm wondering if we can agree to just merge it in. If we get time before 8 is released we can do the follow-up improvements:
Any opposition? |
|
I am +1 for merging this. |
|
Ran out of steam for 8.0; may revisit for 8.1. |
Adds:
zend_exception_get_codezend_exception_get_filezend_exception_get_linezend_exception_get_messagezend_exception_get_tracezend_exception_get_trace_as_string. This accepts an arg forcontrolling whether the arguments will be included.
Also adds
zend_obj_read_propertyandzend_obj_read_property_ex. These are just likezend_read_property/zend_read_property_exexcept they take azend_object *instead of azval *.I intended these to mostly be used by extensions, but with a quick look did find some places where they were useful in core too.