Task #35916

Make system call to php binary reliable one for all, especially on Windows

Added by Adrian Föder over 3 years ago. Updated over 2 years ago.

Status:Resolved Start date:2012-04-12
Priority:Should have Due date:
Assigned To:Karsten Dambekalns % Done:

100%

Category:Core
Target version:TYPO3 Flow Base Distribution - 1.1.1
Sprint: Has patch:No
PHP Version: Complexity:

Description

Currently, there are still issues with \TYPO3\FLOW3\Core\Booting\Scripts'

1            $phpBinaryPathAndFilename = escapeshellcmd(\TYPO3\FLOW3\Utility\Files::getUnixStylePath($settings['core']['phpBinaryPathAndFilename']));
2            $command = '"' . $phpBinaryPathAndFilename . '" -c ' . escapeshellarg(php_ini_loaded_file()) . ' -v';
3            system($command, $result);
4            if ($result !== 0) {
5                throw new \TYPO3\FLOW3\Exception('It seems like the PHP binary "' . $settings['core']['phpBinaryPathAndFilename'] . '" cannot be executed by FLOW3. Set the correct path to the PHP executable in Configuration/Settings.yaml, setting FLOW3.core.phpBinaryPathAndFilename.', 1315561483);
6            }
7            throw new \TYPO3\FLOW3\Exception('The compile run failed. Please check the error output or system log for more information.', 1297263663);
8

Its very likely that this call still doesn't success in complicated Windows environment, where the path is e.g. set to C:\Program Files (x86)\Zend\ZendServer\bin\php.exe. Even that escapeshellcmd brings up errors because the (x86) is converted to ^(x86^).

Additionally, the exception message should be more verbose and at least carry the actual $command in order to make error catching more easy.
A few additional file_exists and other stuff may also make that stuff more reliable.

Associated revisions

Revision 17c31e32
Added by Alexander Berl over 2 years ago

[TASK] Make system call to php binary reliable, especially on Windows

This solves execution of php binary on windows environments with paths
containing whitespace and special characters (like round brackets). On
windows, commands containing white spaces must be quoted, but quoted paths may
not be escaped, which escapeshellcmd() wrongly does.

This is solved by using escapeshellarg() on windows instead and also adds more
verbose exception messages in the case of failure.

Change-Id: Ibc7876a8e98f653afaf0bf8c7a4893e0b808cf23
Resolves: #35916
Releases: 1.1, 1.2

Revision d8ce798d
Added by Alexander Berl over 2 years ago

[TASK] Make system call to php binary reliable, especially on Windows

This solves execution of php binary on windows environments with paths
containing whitespace and special characters (like round brackets). On
windows, commands containing white spaces must be quoted, but quoted
paths may not be escaped, which escapeshellcmd() wrongly does.

This is solved by using escapeshellarg() on windows instead and also
adds more verbose exception messages in the case of failure.

Change-Id: Ibc7876a8e98f653afaf0bf8c7a4893e0b808cf23
Resolves: #35916
Releases: 1.1, 1.2

History

#1 Updated by Alexander Berl over 3 years ago

Further details on the escaping problem, which is actually a PHP Bug can be found here:
https://bugs.php.net/bug.php?id=43261
The last comment from 2012-02-12 17:38 UTC is by me, but since this bug is as old as 2008 I doubt it will get fixed anytime.

Other than that +1 for the more verbose exception message and would suggest it also includes the system call return value (last line of output from the call), since this sometimes contains helpful information too. I was stumbled a few times on an "It seems like the PHP binary..." message, which was actually caused by an PHP Syntax error or similar, causing the system call to fail.

#2 Updated by Bastian Waidelich about 3 years ago

  • Category set to Core
  • Assigned To deleted (Bastian Waidelich)

Definitely makes sense, but I can't take care of it atm

#3 Updated by Karsten Dambekalns about 3 years ago

  • Target version deleted (1.1)

#4 Updated by Gerrit Code Review about 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#5 Updated by Karsten Dambekalns almost 3 years ago

  • Assigned To set to Karsten Dambekalns
  • Target version set to 1.1

Assigning to me (so it doesn't get lost), because the change author is not assignable.

#6 Updated by Gerrit Code Review almost 3 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#7 Updated by Gerrit Code Review almost 3 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#8 Updated by Karsten Dambekalns almost 3 years ago

  • Target version changed from 1.1 to 1.1.1

#9 Updated by Gerrit Code Review over 2 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#10 Updated by Gerrit Code Review over 2 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#11 Updated by Gerrit Code Review over 2 years ago

Patch set 6 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#12 Updated by Gerrit Code Review over 2 years ago

Patch set 7 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13423

#13 Updated by Gerrit Code Review over 2 years ago

Patch set 1 for branch FLOW3-1.1 has been pushed to the review server.
It is available at https://review.typo3.org/17093

#14 Updated by Alexander Berl over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF