Skip to content

Commit 7a6cba1

Browse files
committed
judgedaemon: Replace system and other command executions.
Introduce a safer and more readable variant that executes, logs and checks return values centrally.
1 parent cf2478c commit 7a6cba1

File tree

1 file changed

+88
-64
lines changed

1 file changed

+88
-64
lines changed

judge/judgedaemon.main.php

Lines changed: 88 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,41 @@ function read_judgehostlog(int $numLines = 20) : string
293293
return trim(ob_get_clean());
294294
}
295295

296+
define('DONT_CARE', new class {});
297+
/**
298+
* Execute a shell command given an array of strings forming the command.
299+
* The command and all its arguments are automatically escaped.
300+
*
301+
* @param array $command_parts The command and its arguments (e.g., ['ls', '-l', '/tmp/']).
302+
* @param mixed $retval The (integer) variable to store the command's exit code.
303+
* @param bool $log_nonzero_exitcode Whether non-zero exit codes should be logged.
304+
*
305+
* @return bool Returns true on success (exit code 0), false otherwise.
306+
*/
307+
function run_command_safe(array $command_parts, &$retval = DONT_CARE, $log_nonzero_exitcode = true): bool
308+
{
309+
if (empty($command_parts)) {
310+
logmsg(LOG_WARNING, "Need at least the command that should be called.");
311+
$retval = -1;
312+
return false;
313+
}
314+
315+
$command = implode(' ', array_map('dj_escapeshellarg', $command_parts));
316+
317+
logmsg(LOG_DEBUG, "Executing command: $command");
318+
system($command, $retval_local);
319+
if ($retval !== DONT_CARE) $retval = $retval_local;
320+
321+
if ($retval_local !== 0) {
322+
if ($log_nonzero_exitcode) {
323+
logmsg(LOG_WARNING, "Command failed with exit code $retval_local: $command");
324+
}
325+
return false;
326+
}
327+
328+
return true;
329+
}
330+
296331
// Fetches a new executable from database if not cached already, and runs build script to compile executable.
297332
// Returns an array with
298333
// - absolute path to run script
@@ -354,12 +389,10 @@ function fetch_executable_internal(
354389
$execrunjurypath = $execbuilddir . '/runjury';
355390
if (!is_dir($execdir) || !file_exists($execdeploypath) ||
356391
($combined_run_compare && file_get_contents(LIBJUDGEDIR . '/run-interactive.sh')!==file_get_contents($execrunpath))) {
357-
system('rm -rf ' . dj_escapeshellarg($execdir) . ' ' . dj_escapeshellarg($execbuilddir), $retval);
358-
if ($retval !== 0) {
392+
if (!run_command_safe(['rm', '-rf', $execdir, $execbuilddir])) {
359393
disable('judgehost', 'hostname', $myhost, "Deleting '$execdir' or '$execbuilddir' was unsuccessful.");
360394
}
361-
system('mkdir -p ' . dj_escapeshellarg($execbuilddir), $retval);
362-
if ($retval !== 0) {
395+
if (!run_command_safe(['mkdir', '-p', $execbuilddir])) {
363396
disable('judgehost', 'hostname', $myhost, "Could not create directory '$execbuilddir'");
364397
}
365398

@@ -469,8 +502,7 @@ function fetch_executable_internal(
469502
putenv('SCRIPTMEMLIMIT=' . djconfig_get_value('script_memory_limit'));
470503
putenv('SCRIPTFILELIMIT=' . djconfig_get_value('script_filesize_limit'));
471504

472-
system(LIBJUDGEDIR . '/build_executable.sh ' . dj_escapeshellarg($execdir), $retval);
473-
if ($retval !== 0) {
505+
if (!run_command_safe([LIBJUDGEDIR . '/build_executable.sh', $execdir])) {
474506
return [null, "Failed to build executable in $execdir.", "$execdir/build.log"];
475507
}
476508
chmod($execrunpath, 0755);
@@ -531,7 +563,11 @@ function fetch_executable_internal(
531563
exit(1);
532564
}
533565

534-
$myhost = trim(`hostname | cut -d . -f 1`);
566+
$hostname = gethostname();
567+
if ($hostname === false) {
568+
error("Could not determine hostname.");
569+
}
570+
$myhost = explode('.', $hostname)[0];
535571
if (isset($options['daemonid'])) {
536572
if (preg_match('/^\d+$/', $options['daemonid'])) {
537573
$myhost = $myhost . "-" . $options['daemonid'];
@@ -662,9 +698,8 @@ function fetch_executable_internal(
662698

663699
// Check basic prerequisites for chroot at judgehost startup
664700
logmsg(LOG_INFO, "🔏 Executing chroot script: '".CHROOT_SCRIPT." check'");
665-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' check', $retval);
666-
if ($retval!==0) {
667-
error("chroot validation check exited with exitcode $retval");
701+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'check'])) {
702+
error("chroot validation check failed");
668703
}
669704

670705
foreach ($endpoints as $id => $endpoint) {
@@ -756,8 +791,7 @@ function fetch_executable_internal(
756791
foreach ($candidateDirs as $d) {
757792
$cnt++;
758793
logmsg(LOG_INFO, " - deleting $d");
759-
system('rm -rf ' . dj_escapeshellarg($d), $retval);
760-
if ($retval !== 0) {
794+
if (!run_command_safe(['rm', '-rf', $d])) {
761795
logmsg(LOG_WARNING, "Deleting '$d' was unsuccessful.");
762796
}
763797
$after = disk_free_space(JUDGEDIR);
@@ -881,10 +915,7 @@ function fetch_executable_internal(
881915
$judgeTask['judgetaskid']
882916
);
883917

884-
$debug_cmd = implode(' ', array_map('dj_escapeshellarg',
885-
[$runpath, $workdir, $tmpfile]));
886-
system($debug_cmd, $retval);
887-
if ($retval !== 0) {
918+
if (!run_command_safe([$runpath, $workdir, $tmpfile])) {
888919
disable('run_script', 'run_script_id', $judgeTask['run_script_id'], "Running '$runpath' failed.");
889920
}
890921

@@ -953,8 +984,7 @@ function fetch_executable_internal(
953984
}
954985

955986

956-
system('mkdir -p ' . dj_escapeshellarg("$workdir/compile"), $retval);
957-
if ($retval !== 0) {
987+
if (!run_command_safe(['mkdir', '-p', "$workdir/compile"])) {
958988
error("Could not create '$workdir/compile'");
959989
}
960990

@@ -967,8 +997,7 @@ function fetch_executable_internal(
967997
if ($lastWorkdir !== $workdir) {
968998
// create chroot environment
969999
logmsg(LOG_INFO, " 🔒 Executing chroot script: '".CHROOT_SCRIPT." start'");
970-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' start', $retval);
971-
if ($retval!==0) {
1000+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'start'], $retval)) {
9721001
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
9731002
disable('judgehost', 'hostname', $myhost, "chroot script exited with exitcode $retval on $myhost");
9741003
continue;
@@ -1030,8 +1059,7 @@ function registerJudgehost(string $myhost): void
10301059

10311060
// Create directory where to test submissions
10321061
$workdirpath = JUDGEDIR . "/$myhost/endpoint-$endpointID";
1033-
system('mkdir -p ' . dj_escapeshellarg("$workdirpath/testcase"), $retval);
1034-
if ($retval !== 0) {
1062+
if (!run_command_safe(['mkdir', '-p', "$workdirpath/testcase"])) {
10351063
error("Could not create $workdirpath");
10361064
}
10371065
chmod("$workdirpath/testcase", 0700);
@@ -1109,8 +1137,7 @@ function cleanup_judging(string $workdir) : void
11091137

11101138
// destroy chroot environment
11111139
logmsg(LOG_INFO, " 🔓 Executing chroot script: '".CHROOT_SCRIPT." stop'");
1112-
system(LIBJUDGEDIR.'/'.CHROOT_SCRIPT.' stop', $retval);
1113-
if ($retval!==0) {
1140+
if (!run_command_safe([LIBJUDGEDIR.'/'.CHROOT_SCRIPT, 'stop'], $retval)) {
11141141
logmsg(LOG_ERR, "chroot script exited with exitcode $retval");
11151142
disable('judgehost', 'hostname', $myhost, "chroot script exited with exitcode $retval on $myhost");
11161143
// Just continue here: even though we might continue a current
@@ -1120,9 +1147,8 @@ function cleanup_judging(string $workdir) : void
11201147
}
11211148

11221149
// Evict all contents of the workdir from the kernel fs cache
1123-
system(LIBJUDGEDIR . '/evict ' . dj_escapeshellarg($workdir), $retval);
1124-
if ($retval!==0) {
1125-
warning("evict script exited with exitcode $retval");
1150+
if (!run_command_safe([LIBJUDGEDIR . '/evict', $workdir])) {
1151+
warning("evict script failed, continuing gracefully");
11261152
}
11271153
}
11281154

@@ -1131,7 +1157,7 @@ function compile(
11311157
string $workdir,
11321158
string $workdirpath,
11331159
array $compile_config,
1134-
string $cpuset_opt,
1160+
?string $daemonid,
11351161
int $output_storage_limit
11361162
): bool {
11371163
global $myhost, $EXITCODES;
@@ -1151,26 +1177,24 @@ function compile(
11511177
$args = 'hostname=' . urlencode($myhost);
11521178
foreach (['compiler', 'runner'] as $type) {
11531179
if (isset($version_verification[$type . '_version_command'])) {
1180+
if (file_exists($version_output_file)) {
1181+
unlink($version_output_file);
1182+
}
1183+
11541184
$vcscript_content = $version_verification[$type . '_version_command'];
11551185
$vcscript = tempnam(TMPDIR, 'version_check-');
11561186
file_put_contents($vcscript, $vcscript_content);
11571187
chmod($vcscript, 0755);
1158-
$version_cmd = LIBJUDGEDIR . "/version_check.sh " .
1159-
implode(' ', array_map('dj_escapeshellarg', [
1160-
$vcscript,
1161-
$workdir,
1162-
]));
11631188

1164-
if (file_exists($version_output_file)) {
1165-
unlink($version_output_file);
1166-
}
1167-
system($version_cmd, $retval);
1189+
run_command_safe([LIBJUDGEDIR . "/version_check.sh", $vcscript, $workdir], $retval);
1190+
11681191
$versions[$type] = trim(file_get_contents($version_output_file));
11691192
if ($retval !== 0) {
11701193
$versions[$type] =
11711194
"Getting $type version failed with exit code $retval\n"
11721195
. $versions[$type];
11731196
}
1197+
11741198
unlink($vcscript);
11751199
}
11761200
if (isset($versions[$type])) {
@@ -1248,16 +1272,14 @@ function compile(
12481272
}
12491273

12501274
// Compile the program.
1251-
$compile_cmd = LIBJUDGEDIR . "/compile.sh $cpuset_opt " .
1252-
implode(' ', array_map('dj_escapeshellarg', array_merge([
1253-
$execrunpath,
1254-
$workdir,
1255-
], $files)));
1256-
logmsg(LOG_DEBUG, "Compile command: ".$compile_cmd);
1257-
system($compile_cmd, $retval);
1258-
if ($retval!==0) {
1259-
warning("compile script exited with exitcode $retval");
1275+
$compile_command_parts = [LIBJUDGEDIR . '/compile.sh'];
1276+
if (isset($daemonid)) {
1277+
$compile_command_parts[] = '-n';
1278+
$compile_command_parts[] = $daemonid;
12601279
}
1280+
array_push($compile_command_parts, $execrunpath, $workdir, ...$files);
1281+
// Note that the $retval is handled further down after reading/writing metadata.
1282+
run_command_safe($compile_command_parts, $retval, log_nonzero_exitcode: false);
12611283

12621284
$compile_output = '';
12631285
if (is_readable($workdir . '/compile.out')) {
@@ -1358,7 +1380,7 @@ function judge(array $judgeTask): bool
13581380
}
13591381

13601382
$workdir = judging_directory($workdirpath, $judgeTask);
1361-
$compile_success = compile($judgeTask, $workdir, $workdirpath, $compile_config, $cpuset_opt, $output_storage_limit);
1383+
$compile_success = compile($judgeTask, $workdir, $workdirpath, $compile_config, $options['daemonid'] ?? null, $output_storage_limit);
13621384
if (!$compile_success) {
13631385
return false;
13641386
}
@@ -1460,37 +1482,39 @@ function judge(array $judgeTask): bool
14601482
// after the first (note that $passCnt starts at 1).
14611483
if ($passCnt > 1) {
14621484
$prevPassdir = $testcasedir . '/' . ($passCnt - 1) . '/feedback';
1463-
system('cp -R ' . dj_escapeshellarg($prevPassdir) . ' ' . dj_escapeshellarg($passdir . '/'));
1464-
system('rm ' . dj_escapeshellarg($passdir . '/feedback/nextpass.in'));
1485+
run_command_safe(['cp', '-R', $prevPassdir, $passdir . '/']);
1486+
run_command_safe(['rm', $passdir . '/feedback/nextpass.in']);
14651487
}
14661488

14671489
// Copy program with all possible additional files to testcase
14681490
// dir. Use hardlinks to preserve space with big executables.
14691491
$programdir = $passdir . '/execdir';
1470-
system('mkdir -p ' . dj_escapeshellarg($programdir), $retval);
1471-
if ($retval!==0) {
1492+
if (!run_command_safe(['mkdir', '-p', $programdir])) {
14721493
error("Could not create directory '$programdir'");
14731494
}
14741495

14751496
foreach (glob("$workdir/compile/*") as $compile_file) {
1476-
system('cp -PRl ' . dj_escapeshellarg($compile_file) . ' ' . dj_escapeshellarg($programdir), $retval);
1477-
if ($retval!==0) {
1497+
if (!run_command_safe(['cp', '-PRl', $compile_file, $programdir])) {
14781498
error("Could not copy program to '$programdir'");
14791499
}
14801500
}
14811501

14821502
$timelimit_str = implode(':', $timelimit['cpu']) . ',' . implode(':', $timelimit['wall']);
1483-
$test_run_cmd = LIBJUDGEDIR . "/testcase_run.sh $cpuset_opt " .
1484-
implode(' ', array_map('dj_escapeshellarg', [
1485-
$input,
1486-
$output,
1487-
$timelimit_str,
1488-
$passdir,
1489-
$run_runpath,
1490-
$compare_runpath,
1491-
$compare_config['compare_args']
1492-
]));
1493-
system($test_run_cmd, $retval);
1503+
$run_command_parts = [LIBJUDGEDIR . '/testcase_run.sh'];
1504+
if (isset($options['daemonid'])) {
1505+
$run_command_parts[] = '-n';
1506+
$run_command_parts[] = $options['daemonid'];
1507+
}
1508+
array_push($run_command_parts,
1509+
$input,
1510+
$output,
1511+
$timelimit_str,
1512+
$passdir,
1513+
$run_runpath,
1514+
$compare_runpath,
1515+
$compare_config['compare_args']
1516+
);
1517+
run_command_safe($run_command_parts, $retval, log_nonzero_exitcode: false);
14941518

14951519
// What does the exitcode mean?
14961520
if (!isset($EXITCODES[$retval])) {

0 commit comments

Comments
 (0)