Fix ini values not getting restored after request has finished

[manuelm]

This fixes two separate cases of ini values not getting restored:

• The max_memory_limit handler directly overwrites the global memory_limit value.
• The fpm sapi does not restore any ini values passed through fcgi environment (PHP_VALUE header).

Both cases should be passed through zend_alter_ini, so modified_ini_directives can keep track.

[iluuu1994]

The OnChangeMaxMemoryLimit part makes sense to me. I did not know per-host ini settings were a thing, and could change OnChangeMaxMemoryLimit after OnChangeMemoryLimit.

As for the FPM part, I'll let that review up to Jakub, as I don't know much about FPM.

[iluuu1994]

@bukka Can you please verify the sapi/fpm/fpm/fpm_main.c part? Alternatively, we may split the PR so I can merge the fix in main/main.c in the meantime.

@manuelm Weren't there tests before? Were they dropped purposefully?

[bukka]

Yeah I got it on my list and should be able to check in the next few days.

[manuelm]

@manuelm Weren't there tests before? Were they dropped purposefully?

My bad. I somehow missed them in the rebase. They are back now.

[iluuu1994]

Argh... Referenced wrong commit in a different PR. Sorry about that.

[bukka]

So I went through this and the FPM is actually a very well known thing that we have 3 bug reports open:

• https://bugs.php.net/bug.php?id=63965 - php-fpm site-specific settings go global
• https://bugs.php.net/bug.php?id=53611 - fastcgi_param PHP_VALUE pollutes other sites
• https://bugs.php.net/bug.php?id=61867 - user_ini_filename ignored for cached user_config (might need a check if this will be addressed by the above)

Those are all valid concerns and I agree that this should be design like that. However I'm quite worried about BC impact. The thing is that there are likely setup relying on the setting being kept between requests. This is really not something I want to deal with.

Another thing is that we also have requests to completely disable the FCGI env INI settinsg so what I would like to introduce instead is an option to control it. It means it should specify how this is going to behave and have couple of options. The default should stay as it is but after some years we can consider changing it so people can try it before the migration and mainly there will be option to revert it back if it breaks their use case.

It means the FPM part should be removed from this PR as it is no acceptable in the current form.

[bukka]

In addition this FPM change is also wrong because it needs to keep using fpm_php_apply_defines_ex to allow special handling for extension and disabled_functions.

[manuelm]

I've removed the FPM related commit.

The code is actually about ~10 years old, and I've been maintaining it since. I had hoped I could contribute it upstream.