Fixed \XF\Util\File::copyFileToAbstractedPath causes failure in PHP8.0+ with S3-Compatable Storages

[VersoBit]

Affected version

2.2.9

Reporting back on an bug that was previously fixed, that has now become "unfixed" by deprecation of the old @ Error Control Operator functionality on PHP8.0+

Let me break this down, recently we were working to Integrate with Cloudflare R2, which is a fully compatible S3 API; however R2 runs it's own fclose similar to Azure Blob Storage; this means that when fclose() is run within the copyFileToAbstractedPath method, it would throw an error - while this behavior could be suppressed in PHP 7.4<, it is no longer the case in PHP8+.


(see: https://www.php.net/manual/en/language.operators.errorcontrol.php#:~:text=and so forth.-,Warning,cause the script to terminate with no indication as to why.,-See Also)

This issue was previously raised by two developers: @JulianD and @Jake B.
See: https://xenforo.com/community/threads/fclose-to-fclose.161603/ and https://xenforo.com/community/threa...ath-runs-fclose-even-when-unnecessary.176626/

To solve this error in PHP8, I applied the fix listed in @Jake B.'s report; we changed the @fclose($fp) to:

   if (get_resource_type($fp) === 'stream') {
      fclose($fp);
   }

And this allowed Cloudflare R2 to successfully upload files to their endpoint without getting the below error:

TypeError: fclose(): supplied resource is not a valid stream resource src/XF/Util/File.php:188
Generated by: User Jun 25, 2022 at 5:41 PM
Stack trace
#0 src/XF/Util/File.php(188): fclose(Resource id #31)
#1 src/XF/Service/Attachment/Preparer.php(78): XF\Util\File::copyFileToAbstractedPath('/home/<home dir>...', 'internal-data:/...')
#2 src/addons/SV/AttachmentImprovements/XF/Service/Attachment/Preparer.php(76): XF\Service\Attachment\Preparer->insertDataFromFile(Object(XF\FileWrapper), 563, Array)
#3 src/XF/Service/Attachment/Preparer.php(38): SV\AttachmentImprovements\XF\Service\Attachment\Preparer->insertDataFromFile(Object(XF\FileWrapper), 563, Array)
#4 src/XF/Attachment/Manipulator.php(172): XF\Service\Attachment\Preparer->insertAttachment(Object(SV\AttachmentImprovements\XF\Attachment\Post), Object(XF\FileWrapper), Object(SV\DailyStatistics\XF\Entity\User), 'ba153ad993853d7...')
#5 src/XF/Pub/Controller/Attachment.php(91): XF\Attachment\Manipulator->insertAttachmentFromUpload(Object(XF\Http\Upload), NULL)
#6 src/addons/ChunkedUploads/XF/Pub/Controller/Attachment.php(48): XF\Pub\Controller\Attachment->actionUpload()
#7 src/XF/Mvc/Dispatcher.php(352): ChunkedUploads\XF\Pub\Controller\Attachment->actionUpload(Object(XF\Mvc\ParameterBag))
#8 src/XF/Mvc/Dispatcher.php(259): XF\Mvc\Dispatcher->dispatchClass('XF:Attachment', 'Upload', Object(XF\Mvc\RouteMatch), Object(SV\AttachmentImprovements\XF\Pub\Controller\Attachment), NULL)
#9 src/XF/Mvc/Dispatcher.php(115): XF\Mvc\Dispatcher->dispatchFromMatch(Object(XF\Mvc\RouteMatch), Object(SV\AttachmentImprovements\XF\Pub\Controller\Attachment), NULL)
#10 src/XF/Mvc/Dispatcher.php(57): XF\Mvc\Dispatcher->dispatchLoop(Object(XF\Mvc\RouteMatch))
#11 src/XF/App.php(2352): XF\Mvc\Dispatcher->run()
#12 src/XF.php(524): XF\App->run()
#13 index.php(20): XF::runApp('XF\\Pub\\App')
#14 {main}
Request state
array(4) {
  ["url"] => string(92) "/attachments/upload?type=post&context[thread_id]=11257&hash=ba153ad993853d7a59d3fafe6a7b6ed8"
  ["referrer"] => string(51) "https://site.com/threads/post-staging.11257/"
  ["_GET"] => array(4) {
    ["/attachments/upload"] => string(0) ""
    ["type"] => string(4) "post"
    ["context"] => array(1) {
      ["thread_id"] => string(5) "11257"
    }
    ["hash"] => string(32) "ba153ad993853d7a59d3fafe6a7b6ed8"
  }
  ["_POST"] => array(11) {
    ["_xfToken"] => string(8) "********"
    ["_xfResponseType"] => string(4) "json"
    ["_xfWithData"] => string(1) "1"
    ["flowChunkNumber"] => string(1) "2"
    ["flowChunkSize"] => string(8) "41943040"
    ["flowCurrentChunkSize"] => string(8) "41943040"
    ["flowTotalSize"] => string(9) "170311249"
    ["flowIdentifier"] => string(27) "170311249-demofile.zip"
    ["flowFilename"] => string(20) "demofile.zip"
    ["flowRelativePath"] => string(20) "demofile.zip"
    ["flowTotalChunks"] => string(1) "5"
  }
}

(some data removed for PII)

I propose that for supporting future versions of PHP, that the @ E.C.O. should be changed to statements that correctly handle the error/behavior scenario.

There has also been an inability to upload files to S3 when XFMG is converting the video file, not sure what's causing that, but I will have to dig in more to the code where that runs to see if there are any @ statements on fclose's that could be causing an error in command line (as we run our jobs via cron).

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.13).

Change log:

Make stream closing attempts more robust when working with abstract files

There may be a delay before changes are rolled out to the XenForo Community.