Added support for PHP paths with spaces in Magento CLI

[PivitParkour94]

Support for PHP paths that include spaces in the Magento CLI

Description

Found that some Magento CLI commands (bin/magento deploy:mode:set production for example) caused another command to be run, which is all good and fine except because it is then executed at a low level exec function call, if the PHP_BINARY variable had a space in it, it would cause it to break.

Originally solved it simply escaping all spaces in the final command string, but that's a terrible security principle, not to mention it causes any arguments to not be recognised.

End solution seems to be as simple as wrapping the \Magento\Deploy\Model\Filesystem::$functionCallPath in quotes.

Note: If the current state of having M2 running on Windows is that it won't happen that's a real shame, but as far as I know you can have spaces in filepaths on OSX or *nix (you probably wouldn't, but it's at least possible)

Fixed Issues (if relevant)

Rather than creating an issue, I just solved it with a PR because it is so simple

Manual testing scenarios

1. Have a space in your php path
2. Run bin/magento deploy:mode:set production -vvv
3. See that the command runs as expected

Contribution checklist

• [ x ] Pull request has a meaningful description of its purpose
• [ x ] All commits are accompanied by meaningful commit messages
• [ x ] All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-cicd2]

All committers have signed the CLA.

[miguelbalparda]

From the DevDocs, MacOs and Windows are not supported. I haven't seen any server with a space in the PHP_BINARY, I'll check internally to see if this is worth adding and report back.
Can you check the failing tests in the meantime? Same with the CLA.

[oliverklee]

This is incomplete and will break if the path has double quotes in it. We'll also need to use escapeshellcmd.

[PivitParkour94]

that's a much better alternative! escapeshellcmd

[orlangur]

as far as I know you can have spaces in filepaths on OSX or *nix (you probably wouldn't, but it's at least possible)

@PivitParkour94 while obviously it is technically possible, are you aware of ANY supported OS actually having such spaces by default?

[orlangur]

https://github.com/magento/magento2/search?q=php_binary&type=Code&utf8=✓ this is not the only occurrence and I doubt there is any real reason to complicate the code.

@miguelbalparda described issue is a duplicate of #5258 (comment), as I don't see any new argumentation I would insist on rejecting this PR.

[PivitParkour94]

@orlangur This may not be an issue with default setup, but I think skipping it completely would be an oversight, simply because it doesn't fall into the cookie cut PHP installation.

There are multiple places that the PHP_BINARY constant is used, but the issue is in running exec when the path has a space in it.

As it is such a simple fix, I thought I'd add it into the core and that would be good (granted I haven't added the PHPUnit test fix... that's my bad) rather than having to create an entire module that adds this support, it would be nice to just have it in core. Of course it is your choice to make,

Thanks for your time

[miguelbalparda]

simply because it doesn't fall into the cookie cut PHP installation

is an understatement. We have supported and unsupported operating systems and this fix, while simple, seems to be for a system we don't support. We'll come back to this if anything changes in the future. Thanks!