BUGFIX: Files::getFileContents broken in PHP 7.1#821
Conversation
This change set the offset to 0 (default value in file_get_contents. PHP 7.1 change the behaviour of file_get_contents regarding negative offset for many core functions. Without this change the site Root.fusion is not loaded, Files:getFileContents return an empty string. Tested on PHP 7.1.0.
|
@dfeyer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @daniellienert, @kdambekalns and @kitsunet to be potential reviewers. |
|
Please merge this one before #816 |
|
Looks good so far, shouldn't we also change the default to |
Neos.Utility.Files/Classes/Files.php
Outdated
| * @param integer $offset (optional) Offset where reading of the file starts. | ||
| * @param integer $maximumLength (optional) Maximum length to read. Default is -1 (no limit) | ||
| * @return mixed The file content as a string or FALSE if the file could not be opened. | ||
| * @throws FilesException |
There was a problem hiding this comment.
Erm, it doesn't throw anything, right? The exception is caught in line 324…
Neos.Utility.Files/Classes/Files.php
Outdated
| * @throws FilesException | ||
| * @api | ||
| */ | ||
| public static function getFileContents($pathAndFilename, $flags = 0, $context = null, $offset = -1, $maximumLength = -1) |
There was a problem hiding this comment.
$offset should be changed to null, I think (too).
Neos.Utility.Files/Classes/Files.php
Outdated
| @@ -306,6 +306,7 @@ public static function copyDirectoryRecursively($sourceDirectory, $targetDirecto | |||
| * @param integer $offset (optional) Offset where reading of the file starts. | |||
There was a problem hiding this comment.
Add hint that on PHP 7.1 negative offset is supported?
|
I adjusted this a bit, so now the default is |
kdambekalns
left a comment
There was a problem hiding this comment.
Looks good to me now… did not test, though (sorry!)
|
ok, apparently this is the fix for the |
|
@kitsunet Why backport? Does everything else work with 7.1? Then I'd say yes. But we promise 7.1 compatibility starting with 4.0 only, so otherwise… |
|
right, to be checked, probably not, but we should add respective php requirements then. currently you can install flow 3.x / neos 2.x with php 7.1, it just doesn't work. |
|
Ok, if that is the only thing breaking it on 7.1 for older releases, we should backport. If not fix the constraint on PHP -> see #847 |
|
This also fixes PHP 7.1 support for Flow 3.3 / Neos 2.3 - I'll take care of the backport |
With PHP 7.1 the behavior of `file_get_contents()` has been changed allowing negative offsets to be specified. This change adjusts the signature of `TYPO3\Flow\Utility\Files::getFileContents()` accordingly. *Note:* This is a backport of neos#821 which has only been applied to the neos:4.0 branch Fixes: neos#1301
This change set the offset to 0 (default value in
file_get_contents.)PHP 7.1 changed the behaviour of
file_get_contentsregardingnegative offset (support for negative offsets has been added.)
Without this change the sites
Root.fusionis not loaded, so thatFiles::getFileContentsreturn an empty string. Tested on PHP 7.1.0and 7.0.12 (worked without this fix).