craft-transcoder icon indicating copy to clipboard operation
craft-transcoder copied to clipboard

[FR]: Switch all exec calls to proc_open

Open rungta opened this issue 6 years ago • 3 comments

Could the exec calls in the code base be switched to proc_open for wider compatibility with systems where exec is disabled (and better security?)?

rungta avatar Nov 05 '19 18:11 rungta

@rungta I think really what should happen is they should all be switched over to use the ::executeShellCommand() which does exactly that:

    protected function executeShellCommand(string $command): string
    {
        // Create the shell command
        $shellCommand = new ShellCommand();
        $shellCommand->setCommand($command);

        // If we don't have proc_open, maybe we've got exec
        if (!\function_exists('proc_open') && \function_exists('exec')) {
            $shellCommand->useExec = true;
        }

The good news is that there are only 3 places where exec() is used directly, so this shouldn't be bad to do.

khalwat avatar Nov 08 '19 22:11 khalwat

I take it back, it's not a simple swap-out because exec() returns an array, and the shellCommand returns a string.

khalwat avatar Nov 09 '19 14:11 khalwat

Yeah, I assumed there must’ve been a good reason why it wasn’t already making use of executeShellCommand. My experience with both these functions is fairly limited though, so not sure of the intricacies.

On 09-Nov-2019, at 19:57, Andrew Welch [email protected] wrote:

I take it back, it's not a simple swap-out because exec() returns an array, and the shellCommand returns a string.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

rungta avatar Nov 09 '19 18:11 rungta