Skip to content

Commit 408abe8

Browse files
NickSdotNickSdot
committed
Bypass the shell for test subprocesses
1 parent f4d24f0 commit 408abe8

3 files changed

Lines changed: 160 additions & 20 deletions

File tree

‎run-tests.php‎

Lines changed: 143 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ function main(): void
149149
$ignored_by_ext, $ini_overwrites, $colorize,
150150
$cli_opcache_enabled,
151151
$log_format, $no_clean, $no_file_cache,
152-
$pass_options, $php, $php_cgi, $preload,
152+
$pass_options, $pass_options_args, $php, $php_cgi, $preload,
153153
$result_tests_file, $slow_min_ms, $start_time,
154154
$temp_source, $temp_target, $test_cnt,
155155
$test_files, $test_idx, $test_results, $testfile,
@@ -329,6 +329,7 @@ function main(): void
329329
$failed_tests_file = false;
330330
$pass_option_n = false;
331331
$pass_options = '';
332+
$pass_options_args = [];
332333

333334
$output_file = INIT_DIR . '/php_test_results_' . date('Ymd_Hi') . '.txt';
334335

@@ -474,11 +475,13 @@ function main(): void
474475
case 'n':
475476
if (!$pass_option_n) {
476477
$pass_options .= ' -n';
478+
$pass_options_args[] = '-n';
477479
}
478480
$pass_option_n = true;
479481
break;
480482
case 'e':
481483
$pass_options .= ' -e';
484+
$pass_options_args[] = '-e';
482485
break;
483486
case '--preload':
484487
$preload = true;
@@ -684,8 +687,13 @@ function main(): void
684687
if ($conf_passed !== null) {
685688
if (IS_WINDOWS) {
686689
$pass_options .= " -c " . escapeshellarg($conf_passed);
690+
$pass_options_args[] = '-c';
691+
$pass_options_args[] = $conf_passed;
687692
} else {
688-
$pass_options .= " -c '" . realpath($conf_passed) . "'";
693+
$configurationFile = realpath($conf_passed);
694+
$pass_options .= " -c '" . $configurationFile . "'";
695+
$pass_options_args[] = '-c';
696+
$pass_options_args[] = (string) $configurationFile;
689697
}
690698
}
691699

@@ -1179,19 +1187,20 @@ function error_report(string $testname, string $logname, string $tested): void
11791187
* @return false|string
11801188
*/
11811189
function system_with_timeout(
1182-
string $commandline,
1190+
string|array $commandline,
11831191
?array $env = null,
11841192
?string $stdin = null,
11851193
bool $captureStdIn = true,
11861194
bool $captureStdOut = true,
1187-
bool $captureStdErr = true
1195+
bool $captureStdErr = true,
1196+
bool $mergeStdErr = false
11881197
) {
11891198
global $valgrind;
11901199

11911200
// when proc_open cmd is passed as a string (without bypass_shell=true option) the cmd goes thru shell
11921201
// and on Windows quotes are discarded, this is a fix to honor the quotes and allow values containing
11931202
// spaces like '"C:\Program Files\PHP\php.exe"' to be passed as 1 argument correctly
1194-
if (IS_WINDOWS) {
1203+
if (IS_WINDOWS && is_string($commandline)) {
11951204
$commandline = 'start "" /b /wait ' . $commandline . ' & exit';
11961205
}
11971206

@@ -1210,7 +1219,9 @@ function system_with_timeout(
12101219
$descriptorspec[1] = ['pipe', 'w'];
12111220
}
12121221
if ($captureStdErr) {
1213-
$descriptorspec[2] = ['pipe', 'w'];
1222+
$descriptorspec[2] = $mergeStdErr
1223+
? ['redirect', 1]
1224+
: ['pipe', 'w'];
12141225
}
12151226
$proc = proc_open($commandline, $descriptorspec, $pipes, TEST_PHP_SRCDIR, $bin_env, ['suppress_errors' => true]);
12161227

@@ -1558,6 +1569,49 @@ function has_only_fork_safe_ini_settings(TestFile $test): bool
15581569
return true;
15591570
}
15601571

1572+
function can_run_with_structured_test_command(TestFile $test): bool
1573+
{
1574+
global $preload, $valgrind;
1575+
1576+
return !$valgrind
1577+
&& !$preload
1578+
&& !$test->hasAnySections(
1579+
'ARGS',
1580+
'CAPTURE_STDIO',
1581+
'DEFLATE_POST',
1582+
'GZIP_POST',
1583+
'POST',
1584+
'POST_RAW',
1585+
'PUT',
1586+
);
1587+
}
1588+
1589+
function create_structured_test_command(
1590+
string $php,
1591+
array $sapiOptionArgs,
1592+
array $passOptionArgs,
1593+
array $iniSettings,
1594+
string $testFile,
1595+
int $numRepeats
1596+
): array {
1597+
$command = [
1598+
$php,
1599+
...$sapiOptionArgs,
1600+
...$passOptionArgs,
1601+
];
1602+
if ($numRepeats > 1) {
1603+
$command[] = '--repeat';
1604+
$command[] = (string) $numRepeats;
1605+
}
1606+
1607+
return [
1608+
...$command,
1609+
...settings2arguments($iniSettings),
1610+
'-f',
1611+
$testFile,
1612+
];
1613+
}
1614+
15611615
function test_fork_server_cache_key(
15621616
string|array $command,
15631617
array $env,
@@ -2197,7 +2251,7 @@ function skip_test(string $tested, string $tested_file, string $shortname, strin
21972251
function run_test(string $php, $file, array $env): string
21982252
{
21992253
global $log_format, $ini_overwrites, $PHP_FAILED_TESTS;
2200-
global $pass_options, $DETAILED, $IN_REDIRECT, $test_cnt, $test_idx;
2254+
global $pass_options, $pass_options_args, $DETAILED, $IN_REDIRECT, $test_cnt, $test_idx;
22012255
global $valgrind, $temp_source, $temp_target, $cfg, $environment;
22022256
global $no_clean;
22032257
global $SHOW_ONLY_GROUPS;
@@ -2219,6 +2273,9 @@ function run_test(string $php, $file, array $env): string
22192273
$skipCache = new SkipCache($enableSkipCache, $cfg['keep']['skip']);
22202274
}
22212275

2276+
$originalPhpExecutable = $php;
2277+
$phpExecutable = $php;
2278+
$sapiOptionArgs = [];
22222279
$php = escapeshellarg($php);
22232280
$orig_php = $php;
22242281

@@ -2290,6 +2347,8 @@ function run_test(string $php, $file, array $env): string
22902347
if (!$php_cgi) {
22912348
return skip_test($tested, $tested_file, $shortname, 'CGI not available');
22922349
}
2350+
$phpExecutable = $php_cgi;
2351+
$sapiOptionArgs[] = '-C';
22932352
$php = escapeshellarg($php_cgi) . ' -C ';
22942353
$uses_cgi = true;
22952354
if ($num_repeats > 1) {
@@ -2299,13 +2358,17 @@ function run_test(string $php, $file, array $env): string
22992358

23002359
/* For phpdbg tests, check if phpdbg sapi is available and if it is, use it. */
23012360
$extra_options = '';
2361+
$extraOptionArgs = [];
23022362
if ($test->hasSection('PHPDBG')) {
23032363
if (isset($phpdbg)) {
2364+
$phpExecutable = $phpdbg;
2365+
$sapiOptionArgs[] = '-qIb';
23042366
$php = escapeshellarg($phpdbg) . ' -qIb';
23052367

23062368
// Additional phpdbg command line options for sections that need to
23072369
// be run straight away. For example, EXTENSIONS, SKIPIF, CLEAN.
23082370
$extra_options = '-rr';
2371+
$extraOptionArgs[] = '-rr';
23092372
} else {
23102373
return skip_test($tested, $tested_file, $shortname, 'phpdbg not available');
23112374
}
@@ -2459,6 +2522,7 @@ function run_test(string $php, $file, array $env): string
24592522
//$ini_overwrites[] = 'setting=value';
24602523
settings2array($ini_overwrites, $ini_settings);
24612524

2525+
$orig_ini_settings_args = settings2arguments($ini_settings);
24622526
$orig_ini_settings = settings2params($ini_settings);
24632527

24642528
if ($file_cache !== null) {
@@ -2506,6 +2570,7 @@ function run_test(string $php, $file, array $env): string
25062570
}
25072571
}
25082572

2573+
$testIniSettings = $ini_settings;
25092574
$ini_settings = settings2params($ini_settings);
25102575

25112576
$env['TEST_PHP_EXTRA_ARGS'] = $pass_options . ' ' . $ini_settings;
@@ -2516,8 +2581,6 @@ function run_test(string $php, $file, array $env): string
25162581

25172582
if ($test->sectionNotEmpty('SKIPIF')) {
25182583
show_file_block('skip', $test->getSection('SKIPIF'));
2519-
$extra = !IS_WINDOWS ?
2520-
"unset REQUEST_METHOD; unset QUERY_STRING; unset PATH_TRANSLATED; unset SCRIPT_FILENAME; unset REQUEST_METHOD;" : "";
25212584

25222585
if ($valgrind) {
25232586
$env['USE_ZEND_ALLOC'] = '0';
@@ -2527,7 +2590,22 @@ function run_test(string $php, $file, array $env): string
25272590
$junit->startTimer($shortname);
25282591

25292592
$startTime = microtime(true);
2530-
$commandLine = "$extra $php $pass_options $extra_options -q $orig_ini_settings $no_file_cache -d display_errors=1 -d display_startup_errors=0";
2593+
$commandLine = [
2594+
$phpExecutable,
2595+
...$sapiOptionArgs,
2596+
...$pass_options_args,
2597+
...$extraOptionArgs,
2598+
'-q',
2599+
...$orig_ini_settings_args,
2600+
'-d',
2601+
'opcache.file_cache=',
2602+
'-d',
2603+
'opcache.file_cache_only=0',
2604+
'-d',
2605+
'display_errors=1',
2606+
'-d',
2607+
'display_startup_errors=0',
2608+
];
25312609
$output = $skipCache->checkSkip(
25322610
$commandLine,
25332611
$test->getSection('SKIPIF'),
@@ -2880,7 +2958,26 @@ function run_test(string $php, $file, array $env): string
28802958
);
28812959
}
28822960
if ($out === null) {
2883-
$out = system_with_timeout($cmd, $env, $stdin, $captureStdIn, $captureStdOut, $captureStdErr);
2961+
$useStructuredCommand = can_run_with_structured_test_command($test);
2962+
$testCommand = $useStructuredCommand
2963+
? create_structured_test_command(
2964+
$phpExecutable,
2965+
$sapiOptionArgs,
2966+
$pass_options_args,
2967+
$testIniSettings,
2968+
$test_file,
2969+
$num_repeats,
2970+
)
2971+
: $cmd;
2972+
$out = system_with_timeout(
2973+
$testCommand,
2974+
$env,
2975+
$stdin,
2976+
$captureStdIn,
2977+
$captureStdOut,
2978+
$captureStdErr,
2979+
$useStructuredCommand && $captureStdOut && $captureStdErr,
2980+
);
28842981
}
28852982

28862983
$junit->stopTimer($shortname);
@@ -2903,9 +3000,16 @@ function run_test(string $php, $file, array $env): string
29033000
save_text($test_clean, trim($test->getSection('CLEAN')), $temp_clean);
29043001

29053002
if (!$no_clean) {
2906-
$extra = !IS_WINDOWS ?
2907-
"unset REQUEST_METHOD; unset QUERY_STRING; unset PATH_TRANSLATED; unset SCRIPT_FILENAME; unset REQUEST_METHOD;" : "";
2908-
$cleanCommand = "$extra $orig_php $pass_options -q $orig_ini_settings $no_file_cache";
3003+
$cleanCommand = [
3004+
$originalPhpExecutable,
3005+
...$pass_options_args,
3006+
'-q',
3007+
...$orig_ini_settings_args,
3008+
'-d',
3009+
'opcache.file_cache=',
3010+
'-d',
3011+
'opcache.file_cache_only=0',
3012+
];
29093013
$cleanEnv = $env;
29103014
if (!IS_WINDOWS) {
29113015
unset(
@@ -2928,7 +3032,8 @@ function run_test(string $php, $file, array $env): string
29283032
);
29293033
}
29303034
if ($clean_output === null) {
2931-
$clean_output = system_with_timeout("$cleanCommand \"$test_clean\"", $cleanEnv);
3035+
$cleanCommand[] = $test_clean;
3036+
$clean_output = system_with_timeout($cleanCommand, $cleanEnv);
29323037
}
29333038
}
29343039

@@ -3440,6 +3545,20 @@ function settings2params(array $ini_settings): string
34403545
return $settings;
34413546
}
34423547

3548+
function settings2arguments(array $ini_settings): array
3549+
{
3550+
$arguments = [];
3551+
3552+
foreach ($ini_settings as $name => $value) {
3553+
foreach ((array) $value as $item) {
3554+
$arguments[] = '-d';
3555+
$arguments[] = "$name=$item";
3556+
}
3557+
}
3558+
3559+
return $arguments;
3560+
}
3561+
34433562
function compute_summary(): void
34443563
{
34453564
global $n_total, $test_results, $ignored_by_ext, $sum_results, $percent_results;
@@ -4020,7 +4139,7 @@ public function __construct(bool $enable, bool $keepFile)
40204139
}
40214140

40224141
public function checkSkip(
4023-
string $php,
4142+
string|array $command,
40244143
string $code,
40254144
string $checkFile,
40264145
string $tempFile,
@@ -4031,7 +4150,7 @@ public function checkSkip(
40314150
// Extension tests frequently use something like <?php require 'skipif.inc';
40324151
// for skip checks. This forces us to cache per directory to avoid pollution.
40334152
$dir = dirname($checkFile);
4034-
$key = "$php => $dir";
4153+
$key = (is_array($command) ? implode("\0", $command) : $command) . " => $dir";
40354154

40364155
if (isset($this->skips[$key][$code])) {
40374156
$this->hits++;
@@ -4045,7 +4164,7 @@ public function checkSkip(
40454164
$result = null;
40464165
if ($useForkServer) {
40474166
$result = system_with_test_fork_server(
4048-
$php,
4167+
$command,
40494168
$checkFile,
40504169
$env,
40514170
channel: 'skip',
@@ -4055,7 +4174,12 @@ public function checkSkip(
40554174
);
40564175
}
40574176
if ($result === null) {
4058-
$result = system_with_timeout("$php \"$checkFile\"", $env);
4177+
if (is_array($command)) {
4178+
$command[] = $checkFile;
4179+
} else {
4180+
$command .= " \"$checkFile\"";
4181+
}
4182+
$result = system_with_timeout($command, $env);
40594183
}
40604184
$result = trim($result);
40614185
if (strpos($result, 'nocache') === 0) {

‎tests/basic/req60524-win.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ if(PHP_OS_FAMILY !== "Windows")
1010
--FILE--
1111
<?php echo sys_get_temp_dir(); ?>
1212
--EXPECT--
13-
C:\\Windows
13+
C:\Windows

‎tests/run-test/clean_environment.phpt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
CLEAN does not inherit request environment variables on POSIX
3+
--FILE--
4+
<?php
5+
?>
6+
--CLEAN--
7+
<?php
8+
if (PHP_OS_FAMILY !== 'Windows') {
9+
foreach (['REQUEST_METHOD', 'QUERY_STRING', 'PATH_TRANSLATED', 'SCRIPT_FILENAME'] as $name) {
10+
if (getenv($name) !== false) {
11+
echo "$name was inherited\n";
12+
}
13+
}
14+
}
15+
?>
16+
--EXPECT--

0 commit comments

Comments
 (0)