Skip to content

Commit

Permalink
bin-ext: remove shell_exec (fix #1356)
Browse files Browse the repository at this point in the history 
Signed-off-by: Varun Patil <[email protected]>
  • Loading branch information
@pulsejet
pulsejet committed Jan 4, 2025
1 parent 3a6b76c commit 873d2e1
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 44 deletions.
6 changes: 2 additions & 4 deletions lib/Controller/AdminController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,8 @@ public function getSystemStatus(): Http\Response
);

// Check for system perl
/** @psalm-suppress ForbiddenCode */
$status['perl'] = $this->getExecutableStatus(
trim(shell_exec('which perl') ?: '/bin/perl'),
trim(Util::execSafe(['which', 'perl'], 3000) ?: '/bin/perl'),
static fn (string $p) => BinExt::testSystemPerl($p),
);

Expand Down Expand Up @@ -140,10 +139,9 @@ public function getSystemStatus(): Http\Response
}

// Check for FFmpeg for preview generation
/** @psalm-suppress ForbiddenCode */
$status['ffmpeg_preview'] = $this->getExecutableStatus(
SystemConfig::get('preview_ffmpeg_path')
?: trim(shell_exec('which ffmpeg') ?: ''),
?: trim(Util::execSafe(['which', 'ffmpeg'], 3000) ?: ''),
static fn ($p) => BinExt::testFFmpeg($p, 'ffmpeg'),
);

Expand Down
6 changes: 3 additions & 3 deletions lib/Exif.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ public static function setExif(string $path, array $data): void
]);

try {
$stdout = Util::execSafe($cmd, $raw, self::EXIFTOOL_TIMEOUT);
$stdout = Util::execSafe($cmd, self::EXIFTOOL_TIMEOUT, $raw) ?? 'error: cmd fail';
} catch (\Exception $ex) {
error_log("Timeout reading from exiftool: [{$path}]");

Expand Down Expand Up @@ -400,7 +400,7 @@ public static function getBinaryExifProp(string $path, string $prop): string
$cmd = array_merge(self::getExiftool(), [$prop, '-n', '-b', $path]);

try {
return Util::execSafe($cmd, null, self::EXIFTOOL_TIMEOUT);
return Util::execSafe($cmd, self::EXIFTOOL_TIMEOUT) ?? '';
} catch (\Exception $ex) {
error_log("Timeout reading from exiftool: [{$path}]");

Expand Down Expand Up @@ -475,7 +475,7 @@ private static function getExifFromLocalPathWithSeparateProc(string $path, array
$cmd = array_merge(self::getExiftool(), self::EXIFTOOL_ARGS, $extraArgs, [$path]);

try {
$stdout = Util::execSafe($cmd, null, self::EXIFTOOL_TIMEOUT);
$stdout = Util::execSafe($cmd, self::EXIFTOOL_TIMEOUT) ?? '';
} catch (\Exception $ex) {
error_log("Timeout reading from exiftool: [{$path}]");

Expand Down
61 changes: 29 additions & 32 deletions lib/Service/BinExt.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace OCA\Memories\Service;

use OCA\Memories\Settings\SystemConfig;
use OCA\Memories\Util;

class BinExt
{
Expand Down Expand Up @@ -67,12 +68,11 @@ public static function getName(string $name, string $version = ''): string
/** Test configured exiftool binary */
public static function testExiftool(): string
{
$cmd = implode(' ', array_merge(self::getExiftool(), ['-ver']));
$cmd = array_merge(self::getExiftool(), ['-ver']);

/** @psalm-suppress ForbiddenCode */
$out = shell_exec($cmd);
$out = Util::execSafe($cmd, 3000);
if (!$out) {
throw new \Exception("failed to run exiftool: {$cmd}");
throw new \Exception('failed to run exiftool: '.implode(' ', $cmd));
}

// Check version
Expand Down Expand Up @@ -263,8 +263,8 @@ public static function startGoVod(): ?string
$tmpPath = $env['tempdir'];

// (Re-)create temp dir
/** @psalm-suppress ForbiddenCode */
shell_exec("rm -rf '{$tmpPath}' && mkdir -p '{$tmpPath}' && chmod 755 '{$tmpPath}'");
Util::execSafe(['rm', '-rf', $tmpPath], 3000);
mkdir($tmpPath, 0755, true);

// Check temp directory exists
if (!is_dir($tmpPath)) {
Expand All @@ -285,8 +285,13 @@ public static function startGoVod(): ?string
self::pkill(self::getName('go-vod'));

// Start transcoder
/** @psalm-suppress ForbiddenCode */
shell_exec("nohup {$transcoder} {$configFile} >> '{$logFile}' 2>&1 & > /dev/null");
// We need init to own this process, there's no easy way to do this
$pipes = [];
proc_open(['sh', '-c', "nohup {$transcoder} {$configFile} &"], [
0 => ['file', '/dev/null', 'r'],
1 => ['file', $logFile, 'a'],
2 => ['file', $logFile, 'a'],
], $pipes);

// wait for 500ms
usleep(500000);
Expand Down Expand Up @@ -414,12 +419,8 @@ public static function detectFFmpeg(): ?string

if (empty($ffmpegPath) || !file_exists($ffmpegPath) || empty($ffprobePath) || !file_exists($ffprobePath)) {
// Use PATH environment variable to find ffmpeg

/** @psalm-suppress ForbiddenCode */
$ffmpegPath = shell_exec('which ffmpeg');

/** @psalm-suppress ForbiddenCode */
$ffprobePath = shell_exec('which ffprobe');
$ffmpegPath = Util::execSafe(['which', 'ffmpeg'], 3000);
$ffprobePath = Util::execSafe(['which', 'ffprobe'], 3000);
if (!$ffmpegPath || !$ffprobePath) {
return null;
}
Expand All @@ -443,8 +444,7 @@ public static function detectFFmpeg(): ?string

public static function testFFmpeg(string $path, string $name): string
{
/** @psalm-suppress ForbiddenCode */
$version = shell_exec("{$path} -version") ?: '';
$version = Util::execSafe([$path, '-version'], 3000) ?: '';
if (!preg_match("/{$name} version \\S*/", $version, $matches)) {
throw new \Exception("failed to detect version, found {$version}");
}
Expand All @@ -454,13 +454,11 @@ public static function testFFmpeg(string $path, string $name): string

public static function testSystemPerl(string $path): string
{
/** @psalm-suppress ForbiddenCode */
if (($out = shell_exec("{$path} -e 'print \"OK\";'")) !== 'OK') {
if (($out = Util::execSafe([$path, '-e', 'print "OK";'], 3000)) !== 'OK') {
throw new \Exception('Failed to run test perl script: '.(string) $out);
}

/** @psalm-suppress ForbiddenCode */
return shell_exec("{$path} -e 'print $^V;'") ?: 'unknown version';
return Util::execSafe([$path, '-e', 'print $^V;'], 3000) ?: 'unknown version';
}

/**
Expand All @@ -480,28 +478,27 @@ public static function pkill(string $name): void
$name = substr($name, 0, 12);

// check if ps or busybox is available
$ps = 'ps';
$ps = ['ps'];

/** @psalm-suppress ForbiddenCode */
if (!shell_exec('which ps')) {
if (!shell_exec('which busybox')) {
if (!Util::execSafe(['which', 'ps'], 1000)) {
if (!Util::execSafe(['which', 'busybox'], 1000)) {
return;
}

$ps = 'busybox ps';
$ps = ['busybox', 'ps'];
}

// get pids using ps as array
/** @psalm-suppress ForbiddenCode */
$pids = shell_exec("{$ps} -eao pid,comm | grep {$name} | awk '{print $1}'");
if (null === $pids || empty($pids)) {
$procs = Util::execSafe(array_merge($ps, ['-eao', 'pid,comm']), 1000) ?: '';
$procs = explode("\n", $procs);

$matches = array_filter($procs, static fn ($l) => str_contains($l, $name));
$pids = array_map(static fn ($l) => (int) explode(' ', trim($l))[0], $matches);
if (empty($pids)) {
return;
}
$pids = array_filter(explode("\n", $pids));

// kill all pids
foreach ($pids as $pid) {
posix_kill((int) $pid, 9); // SIGKILL
posix_kill($pid, 9); // SIGKILL
}
}
}
30 changes: 25 additions & 5 deletions lib/Util.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,13 @@ public static function getArch(): ?string
*/
public static function getLibc(): ?string
{
/** @psalm-suppress ForbiddenCode */
$ldd = strtolower(shell_exec('ldd --version 2>&1') ?: 'unknown');
// glibc -> stdout, musl -> stderr
$output = self::execSafe2(['ldd', '--version'], 3000, null, true, true);

// check in either
$ldd = strtolower($output[0] ?? '').
strtolower($output[1] ?? '');

if (str_contains($ldd, 'musl')) {
return 'musl';
}
Expand Down Expand Up @@ -457,14 +462,20 @@ public static function registerInterruptHandler(string $name, callable $callback
* Execute a command safely.
*
* @param string[] $cmd command to execute
* @param string $stdin standard input
* @param int $timeout milliseconds
* @param ?string $stdin standard input
*
* @return string standard output
*
* @throws \Exception on error
*/
public static function execSafe(array $cmd, ?string $stdin, int $timeout): string
public static function execSafe(array $cmd, int $timeout, ?string $stdin = null): ?string
{
return self::execSafe2($cmd, $timeout, $stdin, true, false)[0];
}

/** Exec safe with extra options */
public static function execSafe2(array $cmd, int $timeout, ?string $stdin, bool $rstdout, bool $rstderr): array
{
$pipes = [];
$proc = proc_open($cmd, [
Expand All @@ -480,7 +491,16 @@ public static function execSafe(array $cmd, ?string $stdin, int $timeout): strin
fclose($pipes[0]);

try {
return self::readOrTimeout($pipes[1], $timeout);
$output = [null, null];

if ($rstdout) {
$output[0] = self::readOrTimeout($pipes[1], $timeout);
}
if ($rstderr) {
$output[1] = self::readOrTimeout($pipes[2], $timeout);
}

return $output;
} catch (\Exception $ex) {
throw $ex;
} finally {
Expand Down

0 comments on commit 873d2e1

Please sign in to comment.