run-tests.php does not escape path when building cmd

[mvorisek]

Description

https://github.com/php/php-src/blob/php-8.2.2/run-tests.php#L2397

the $php variable is set like: https://github.com/php/php-src/blob/php-8.2.2/run-tests.php#L518

and when it is used to build a full cmd string, it must be escaped, currently when the path contains spaces, the testing fails, especially on Windows, where the path is very often in format like: C:\Program Files\PHP\php.exe

PHP Version

any

Operating System

any

[nielsdos]

Seems like we can use escapeshellarg to solve this issue I think. But we have to be careful about these lines tho:

php-src/run-tests.php

Line 1873 in b20c0e9

$php = $php_cgi . ' -C ';

&

php-src/run-tests.php

Line 1884 in b20c0e9

$php = $phpdbg . ' -qIb';

. So we only must do it on the paths, and not blindly on the $php variable.

[nielsdos]

I tried this, and it mostly works. But I ran into the following issue:
Some tests (for example sapi/cli/tests/021.phpt) use a hashbang line to invoke PHP from the CLI. The problem here is that it is not possible to escape a path in the hashbang line, so those tests will fail. The only workaround I found is to use a symlink and then override the TEST_PHP_EXECUTABLE environment variable (& friends) in order to use the symlink. But I'm not so sure if the maintainers will like this solution... Another idea might be to simply skip those problematic tests.

[mvorisek]

isn't shebang ignored when a script is run like php xxx.php?

[nielsdos]

isn't shebang ignored when a script is run like php xxx.php?

Yes, but I'm pretty sure that test does want to test how the hashbang interacts with PHP.

[mvorisek]

based on conda/conda#186 we can probably skip the test if TEST_PHP_EXECUTABLE (in https://github.com/php/php-src/blob/master/sapi/cli/tests/021.phpt#L17) contain space

[nielsdos]

Yeah I was thinking that as well, sounds reasonable. I'll try that shortly (currently working on another issue report).