fix: hardening for opcache.enable_file_override#734
Conversation
MichaIng
force-pushed
the
fix/harden-for-enable_file_override
branch
2 times, most recently
from
c342721 to
0678abf
Compare
If the PHP instance has aggressive caching enabled, with `opcache.enable_file_override` and sufficiently long or disabled `opcache.validate_timestamps`, `file_exists($versionFileName)` can retrun true after the file was removed from disk. `require_once $versionFileName;` however requires the file to exist on disk, even if it has an OPcache entry. With this commit, the cache entry is invalidated before `file_exists($versionFileName)` is called, to harden the updater for instances with `opcache.enable_file_override`. Signed-off-by: MichaIng <micha@dietpi.com>
MichaIng
force-pushed
the
fix/harden-for-enable_file_override
branch
from
0678abf to
b75ce7e
Compare
Altahrim
left a comment
Altahrim
left a comment
There was a problem hiding this comment.
I think we already have this on server side 👍
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
3 participants
If the PHP instance has aggressive caching enabled, with
opcache.enable_file_overrideand sufficiently long or disabledopcache.validate_timestamps,file_exists($versionFileName)can retrun true after the file was removed from disk in the respective updater step.require_once $versionFileName;however requires the file to exist on disk, even if it has an OPcache entry.With this commit, the cache entry is invalidated before
file_exists($versionFileName)is called, to harden the updater for instances withopcache.enable_file_override.Alternatives to this approach:
config.php. But I am not sure about possible implications, e.g. for the updater steps after files were moved in place, where currently the new version string is used, whileconfig.phpcontains the old version string.version.phpand when to useconfig.php: useconfig.phponly explicitly after old files were removed, and before new files were moved in place, otherwise requireversion.php. However, this is surely more complex to implement, and it is less flexible to handle aborted and reset updates, e.g. when the current step flag has been removed. Only after the step of moving new files in place,version.phpcan be relied on without causing a regression.opcache.enable_file_override=0, but as a caching enthusiast I would be sad if I needed to reintroduce additional disk hits.