GH-22905: avoid truncation on null bytes in reflection exceptions - #22922
GH-22905: avoid truncation on null bytes in reflection exceptions#22922DanielEScherzer wants to merge 2 commits into
Conversation
So that when the outputs are fixed the changes can be confirmed in tests
In addition to the fixes within the reflection extension, support for error strings with null bytes was added to `zend_throw_error()` and `zend_throw_exception_ex()`. The changes to `zend_throw_exception_ex()` are a partial backport of php#16684.
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
I wonder if for a lot of those methods it wouldn't be better to just reject names with nul bytes? As I don't know if anything other than properties can include such a byte?
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Fatal error: Uncaught ReflectionException: Class "foo%0bar" does not exist in %s:%d |
There was a problem hiding this comment.
Is it possible to have a class name with null bytes? If not then just using the Path ZPP specifier would be better
There was a problem hiding this comment.
Anonymous classes have a null byte in their name
There was a problem hiding this comment.
Ah that is true, but isn't it assumed that the part after the null byte is truncate? I remember we added the Interface to the name as a way to distinguish between different anonymous classes.
There was a problem hiding this comment.
I'm not sure. Right now anonymous class names are a concatenation of the parent class/interface if any, followed by @anonymous, a null byte, declaring file name, lineno, and a unique integer. ::class reports the entire name including the null byte and what follows.
There was a problem hiding this comment.
Hmmm maybe @nicolas-grekas can chime in as IIRC he asked for the parent class/interface to be added, so he might have more insight in what might be expected here.
|
|
||
| class Demo {} | ||
| $r = new ReflectionClass(Demo::class); | ||
| $r->getAttributes("foo\0bar", ReflectionAttribute::IS_INSTANCEOF); |
There was a problem hiding this comment.
I guess one could have an attribute with a NUL byte via eval?
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Fatal error: Uncaught ReflectionException: Method Demo::foo%0bar() does not exist in %s:%d |
There was a problem hiding this comment.
Is it possible to have a method with a null byte? If not using path ZPP might be better.
|
|
||
| class Demo {} | ||
| $r = new ReflectionClass(Demo::class); | ||
| $r->getStaticPropertyValue("foo\0bar"); |
There was a problem hiding this comment.
I'm not sure it's possible to have a static property to have a NUL byte?
I figured that kind of refactor shouldn't be done on the older branches - we can definitely switch to using the path ZPP on master after upmerging though |
3 participants
In addition to the fixes within the reflection extension, support for error strings with null bytes was added to
zend_throw_error()andzend_throw_exception_ex(). The changes tozend_throw_exception_ex()are a partial backport of #16684.Fixes #22905