nystudio107 / craft-transcoder

Transcode video & audio files to various formats, and provide video thumbnails
https://nystudio107.com/plugins/transcoder
Other
43 stars 12 forks source link

[FR]: Switch all exec calls to proc_open #25

Open rungta opened 4 years ago

rungta commented 4 years ago

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?)?

khalwat commented 4 years ago

@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 commented 4 years ago

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

rungta commented 4 years ago

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 notifications@github.com 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.