Fix GH-19018: php_admin_value disable_functions ignored

[bukka]

This happens because disable_functions are not explicitly disabled. The fix explicitly calls zend_disable_functions when config is applied.

The WST test for this available here: https://github.com/wstool/wst-php-apache2handler/blob/6573dd55449504e71971c939bf1cca79412197b3/spec/instances/php-admin-value-disable-functions.yaml

[ndossche]

This looks a bit ugly. Why is this not done in the ini message handler instead?

[bukka]

What do you mean?

This is normally explicitly called in main after all functions are loaded in https://github.com/php/php-src/blob/php-8.3.28/main/main.c#L2266 . The problem with php_admin_value (here and in FPM) is that they are loaded after this so zend_disable_functions needs to be called explicitly. We can consider some refactoring for master but I think it should mirror FPM for bug fix.

[bukka]

FPM logic is here: https://github.com/php/php-src/blob/php-8.3.28/sapi/fpm/fpm/fpm_php.c#L121-L124

[ndossche]

Create a custom INI_MH function for disable_functions. Either move the call to zend_disable_functions to the message handler, or if that's not possible due to the message handler not being called always at the right time, then call zend_disable_functions conditionally in the message handler.

FPM logic is here: https://github.com/php/php-src/blob/php-8.3.28/sapi/fpm/fpm/fpm_php.c#L121-L124

This looks equally ugly.

We can consider some refactoring for master but I think it should mirror FPM for bug fix.

We are on master.

[bukka]

Create a custom INI_MH function for disable_functions. Either move the call to zend_disable_functions to the message handler, or if that's not possible due to the message handler not being called always at the right time, then call zend_disable_functions conditionally in the message handler.

But it's not really supposed to be called the first time (it won't do much harm but it will need to be called again when all extensions are loaded). What is in FPM and here is just non standard thing that is mostly not being used so a new INI_MH would be used really just for this edge case.

We are on master.

Sorry forgot to set base somehow. This is a bug fix targetting 8.3.

[bukka]

But it's not really supposed to be called the first time

Just to clarify I mean it's not supposed to be called in php.ini loading because not all functions will be load at that time so there would need to be logic to skip it at that point but just allow to set it later. I guess it can be done but seems more like refactoring as it would also require FPM changes so that should happen just in master IMO.

[ndossche]

Okay