Skip to content

Make quoting of cmd execution functions consistent#5102

Closed
cmb69 wants to merge 1 commit intophp:masterfrom
cmb69:cmb/consistent-exec
Closed

Make quoting of cmd execution functions consistent#5102
cmb69 wants to merge 1 commit intophp:masterfrom
cmb69:cmb/consistent-exec

Conversation

@cmb69
Copy link
Copy Markdown
Member

@cmb69 cmb69 commented Jan 21, 2020

While the $command passed to proc_open() had to be wrapped in
double-quotes manually, that was implicitly done for all other
program execution functions. This could easily introduce bugs and
even security issues when switching from one to another program
execution function.

Furthermore we ensure that the additional quotes are always
unwrapped regardless of what is passed as $command by passing
the /s flag to cmd.exe. As it was, shell_exec('path with spaces/program.exe') did execute program.exe, but adding an
argument (shell_exec('path with spaces/program.exe -h)) failed
to execute program.exe, because cmd.exe stripped the additional
quotes.

While these changes obviously can cause BC breaks, we feel that in
the long run the benefits of having consistent behavior for all
program execution functions outweighs the drawbacks of potentially
breaking some code now.

While the `$command` passed to `proc_open()` had to be wrapped in
double-quotes manually, that was implicitly done for all other
program execution functions.  This could easily introduce bugs and
even security issues when switching from one to another program
execution function.

Furthermore we ensure that the additional quotes are always
unwrapped regardless of what is passed as `$command` by passing
the `/s` flag to cmd.exe.  As it was, `shell_exec('path with
spaces/program.exe')` did execute program.exe, but adding an
argument (`shell_exec('path with spaces/program.exe -h)`) failed
to execute program.exe, because cmd.exe stripped the additional
quotes.

While these changes obviously can cause BC breaks, we feel that in
the long run the benefits of having consistent behavior for all
program execution functions outweighs the drawbacks of potentially
breaking some code now.
@cmb69
Copy link
Copy Markdown
Member Author

cmb69 commented Feb 10, 2020

If there are no objections, I'll merge this PR in a week. Thanks.

@cmb69
Copy link
Copy Markdown
Member Author

cmb69 commented Feb 17, 2020

Applied as 9ca449e. Thanks.

@cmb69 cmb69 closed this Feb 17, 2020
@cmb69 cmb69 deleted the cmb/consistent-exec branch February 17, 2020 22:19
@carusogabriel carusogabriel added this to the PHP 8.0 milestone May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants