Fix for bug in file handling refactor.

[jlbprof]

While testing the cPanel usage of PHP-FPM, we stumbled on this bug.
Without the fix, the zend_string is corrupted and getting odd filenames

When using FPM we kept getting "No input file specified".

I work for cPanel and we use PHP extensively.

[cmb69]

Thanks for the fix. I wonder why that hasn't been reported so far, or has it?

[nikic]

@cmb69 https://bugs.php.net/bug.php?id=81447 looks related.

Fix is clearly right, but we should probably have a test for this functionality.

[jlbprof]

I am not familiar with your testing suite. Especially a targeted test. In our environment we stand up an fpm pool and submit a request. Without the fix, it fails with "No input file specified", with the fix, the request is properly processed. But that is more of an integration level test, not sure how I would write a targeted/small test.

[jlbprof]

Let me investigate how you test things.

[nikic]

FPM tests are located in https://github.com/php/php-src/tree/master/sapi/fpm/tests, though TBH I don't immediately see how to write the test for this, maybe @bukka can help.

[jlbprof]

I am not sure I can test this. I looked at sapi/fpm/tests/tester.inc and "class Tester" is not being tripped up or all those tests would have failed. Sorry I will leave the testing to you.

[bukka]

Can you provide FPM config and some script that is failing with that. It should be possible to recreate it but need a bit more info...

[jlbprof]

I will get you whatever info you need.

In our environment, username cptest1, domain cptest1.tld

We stand up Apache, but NGINX would work as well.

The critical info for the Apache vhost config

We proxy pass to the fpm master process.

    <IfModule proxy_fcgi_module>
        <FilesMatch \.(phtml|php[0-9]*)$>
            SetHandler proxy:unix:/opt/cpanel/ea-php81/root/usr/var/run/php-fpm/8b54c1cb37a8a00661225ba80ac5f7254aa
        </FilesMatch>
    </IfModule>

The config for the fpm pool:

root@10-2-64-92 php-fpm.d]# cat cptest1.tld.conf 
[cptest1_tld]
catch_workers_output = yes
chdir = /home/cptest1
group = "cptest1"
listen = /opt/cpanel/ea-php81/root/usr/var/run/php-fpm/8b54c1cb37a8a00661225ba80ac5f7254aaa91ab.sock
listen.group = "nobody"
listen.mode = 0660
listen.owner = "cptest1"
php_admin_flag[allow_url_fopen] = on
php_admin_flag[log_errors] = on
php_admin_value[disable_functions] = exec,passthru,shell_exec,system
php_admin_value[doc_root] = "/home/cptest1/public_html"
php_admin_value[error_log] = /home/cptest1/logs/cptest1_tld.php.error.log
php_admin_value[short_open_tag] = on
php_value[error_reporting] = E_ALL & ~E_NOTICE
ping.path = /ping
pm = ondemand
pm.max_children = 5
pm.max_requests = 20
pm.max_spare_servers = 5
pm.min_spare_servers = 1
pm.process_idle_timeout = 10
pm.start_servers = 0
pm.status_path = /status
security.limit_extensions = .phtml .php .php3 .php4 .php5 .php6 .php7 .php8
user = "cptest1"

Then to get the error

curl http://cptest1.tld/index.php

The contents of index.php are not relevant, and you should add the record to /etc/hosts.

If you need more info let me know.

Julian

[nikic]

This was my attempt to create a test, but it didn't work: https://gist.github.com/nikic/401a1b46c944908f47bbd0eb7932f159

Not sure how to make the tester actually make use of the doc root.

[bukka]

I will try to find some time on this over the weekend and see if I can come with a test. If not, we can merge it without it...

[bukka]

New PR created with this fix and a test: #7673 . So closing this (the same commit is still used in that PR so credit will be there :) ).

@nikic Looking at your test it was almost there. The main missing bit was that the SCRIPT_FILENAME still needs to be the full path so I had to add a special setting for that. It's also probably better not to add an extra script as this can be just set to __DIR__ and the generated script can be used instead which is a bit cleaner.