On Windows depending on the PHP versionrename() can fail if the target file is being executed. Since the source file is not used by another process usingcopy() instead should be safe to be used.

carsonbot added Status: Needs Review Bug Cache labels

Jul 6, 2024

carsonbot added this to the5.4 milestone

Jul 6, 2024

xabbuh mentioned this pull request

Jul 6, 2024

[Cache] FilesystemCommonTrait - rename fails on windows#50326

Closed

nicolas-grekas approved these changes

Jul 6, 2024

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 6, 2024

nicolas-grekas reviewed

Jul 6, 2024

View reviewed changes

src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php OutdatedShow resolvedHide resolved

use copy() instead of rename() on Windows

6cfef7e

On Windows depending on the PHP version rename() can fail if the targetfile is being executed. Since the source file is not used by anotherprocess using copy() instead should be safe to be used.

xabbuh force-pushed theissue-50326 branch from37585eb to6cfef7eCompare

July 6, 2024 08:37

Copy link

Member

nicolas-grekas commentedJul 6, 2024

Thank you@xabbuh.

nicolas-grekas merged commitb437362 intosymfony:5.4

Jul 6, 2024

xabbuh deleted the issue-50326 branch

July 6, 2024 15:01

This was referencedJul 26, 2024

Release v5.4.42#57843

Merged

Release v6.4.10#57844

Merged

Release v7.0.10#57845

Merged

Release v7.1.3#57846

Merged

Copy link

gggeek commentedJul 26, 2024

I am curious:

iscopy an atomic operation on windows? If not, using it is not a very good idea - cache readers might get a half-written version of the file
doescopy actually allow to overwrite a file which is being executed?
under which scenario would someone be executing a file straight out of the Sf cache? I would think that the cache storage is kind of 'private access'...

Copy link

gggeek commentedJul 26, 2024

ps: further thoughts

it seems the OP is conflating issues in different systems which have differenr fixes, eg. composer is runningcopy vsunlink as external shell processes
the proposed fix for php-src seems at 1st sight better than this. It would not work for all cases where the target file is executed, but at least cover the common scenario of the target file being a php script...