core: Remove disable_classes INI setting #12043
Conversation
|
I think it's way too late for this being added to 8.3. I think it would be better to deprecate it first before the removal. Even though I agree with the reasoning I think there needs to be more time for users to migrate rather than just suddenly remove it after the feature freeze. Ideally I would suggest this goes through the usual deprecations RFC. |
|
I think this feature could work better if we threw an exception on instantiation of the forbidden class, rather than constructing a stripped down version of it. This would make functions that instantiate these objects abort on object construction, rather than unexpectedly work, or break in mysterious ways. That is, as long as the code checks for exceptions after the objects construction. Anyway, I agree that this feature is fragile, especially when it comes to disabling important classes, so I'm not against phasing it out. |
|
I think, disable_classes may be removed in master after PHP-8.3 split. Of course, it's better to create a mini RFC. |
|
Considering this was noticed late, and I didn't really have a lot of time, I'll write an RFC for 8.4. |
As described in the email to the PHP internals list [1] this feature is fundamentally broken and pointless. Only internal classes can be disable which brings the following observation. On a minimal build of PHP, with only the mandatory extensions enabled, there are 148 classes/interfaces/traits defined. [2] Other than the SPL ones (and even then), disabling any of these classes will cause issues within the engine. Moreover, the SPL ones are not a security concern. Therefore, any other class that can be disabled must come from an extension that can be disabled altogether. And "disabling" a class from an extension without disabling said extension will render it useless anyway. If a hosting provided is concerned about an extension, then it should not enable it in the first place. Not break it ad hoc. Considering the above, I cannot see how this functionality was ever useful. This is in stark contrast to the disable_functions INI setting, which can be used to selectively remove functionality of an extension without breaking it overall. What makes this setting particularly broken is that it does not unregister the class, it only overwrites the create CE handler to emit a warning and purge the properties and function hashtables. This leads to various use after free, segfaults, and broken expectations for the engine and extensions which define said classes. On top of that, it is possible to actually instantiate such a class (and even classes which actually disallow this like ext/imap) in userland, and pass it to function that are typed against said class without raising a TypeError. However, when trying to do anything with said object stuff is going to explode in countless ways. [1] https://news-web.php.net/php.internals/120896 [2] https://gist.github.com/Girgias/63d55ba1e50b580412b004046daed02b
Girgias
force-pushed
the
remove-disable-classes
branch
from
d61c299 to
5bf5753
Compare
As described in the email to the PHP internals list [1] this feature is fundamentally broken and pointless.
Only internal classes can be disable which brings the following observation. On a minimal build of PHP, with only the mandatory extensions enabled, there are 148 classes/interfaces/traits defined. [2]
Other than the SPL ones (and even then), disabling any of these classes will cause issues within the engine.
Moreover, the SPL ones are not a security concern.
Therefore, any other class that can be disabled must come from an extension that can be disabled altogether. And "disabling" a class from an extension without disabling said extension will render it useless anyway.
If a hosting provided is concerned about an extension, then it should not enable it in the first place. Not break it ad hoc.
Considering the above, I cannot see how this functionality was ever useful.
This is in stark contrast to the disable_functions INI setting, which can be used to selectively remove functionality of an extension without breaking it overall.
What makes this setting particularly broken is that it does not unregister the class, it only overwrites the create CE handler to emit a warning and purge the properties and function hashtables. This leads to various use after free, segfaults, and broken expectations for the engine and extensions which define said classes. On top of that, it is possible to actually instantiate such a class (and even classes which actually disallow this like ext/imap) in userland, and pass it to function that are typed against said class without raising a TypeError. However, when trying to do anything with said object stuff is going to explode in countless ways.
[1] https://news-web.php.net/php.internals/120896
[2] https://gist.github.com/Girgias/63d55ba1e50b580412b004046daed02b