Think over using ctime in F::modified()

[fabianmichael]

Describe the bug
In Kirby 3.x, the F::modified() method uses the max value of mtime and ctime to determine, when a file was modified. I‘ve recently set up an environment, where a staging server is used for editing content. A "publish" button in the panel is then used by editors for publishing the current state of staging to a live server by copying the whole Kirby installation using rsync.

The problem is, that the ctime (change time) changes every time a file’s metadata (permissions, ownership, content, path etc.) is changed, whereas the mtime (modified time) only changes when a file’s actual content is updated. This leads to the effect, that we cannot use a shared media folder between releases, because most source images do not change whenever the site is published to the live server. But for the server, they seem to be changed because the ctime is always higher than the mtime after a file has been copied. The bad side-effect of that behavior is, that this basically wiped the complete thumbnail cache on every publishing action and as the image URLs also change, this means that the browser cannot effectively cache any content images for returning visitors.

I don’t know, if this is intentional and was implemented like this to solve an actual issue, but I want to point out this following note from the PHP manual. Before reading this, I was under the impression that ctime stand for creation time and not change time.

Note:
Note also that in some Unix texts the ctime of a file is referred to as being the creation time of the file. This is wrong. There is no creation time for Unix files in most Unix filesystems.
Source: https://www.php.net/manual/en/function.filectime.php

To Reproduce
Steps to reproduce the behavior:

1. Install Kirby anywhere and add the content.salt option to the config file to prevent Kirby from using the absolute path for generating content hashes.
2. Upload an image to the content folder and echo it’s URL in the frontend
3. Copy the folder using rsync -t to anywhere else (should probably also work with your OS’s file manager, as long as it does not change the mtime of files while copying).
4. Open the frontend of the copied Kirby installation and compare media hashes or both files. They should be different.

Expected behavior
The same content files should produce a persistent hash, if their content was not changed. A better way of generating hashes for image files could be to use their crc32 checksum (any other fast checksum algorithm should be fine, as to my understanding, this is not really relevant for security in this case). But I don’t know whether the performance implications of that would be too significant if a page contains a lot of images or links to any other files from the content folder.

This does not only affect my obscure setup, but any setup that is based on replacing the whole Kirby installation with a new deployment, including uploading everything manually using SFTP. I would really appreciate to having either a solution that allows me to override the media hash creation or some solution that would resolve this issue for all Kirby installations.

Kirby Version
All Kirby 3 versions

[lukasbestle]

I fully agree. This change is actually planned for Kirby 3.6 because we have run into the same issue with the trykirby.com demo server (where we had to patch the line manually to make it work for now).

In Kirby 2 we still only used filemtime(). The max([$mtime, $ctime]); change was added during the Kirby 3 beta. In my backup of the old beta repository I can see that the commit message was "More reliable modification check for plugins". Back then the PluginAssets class concatenated CSS and JS files of the installed plugins. The resulting file was cached based on the filemtime() of all source files. For some reason this was unreliable (?), so in the mentioned commit F::modified() was updated to use the ctime as a fallback. Of course this change also affected the other uses of F::modified() – especially the media hash generation.

(Today, the plugin assets are handled by the Plugins class, which no longer caches the concatenated files, but uses the mtime for cache-busting in the file URL.)

I talked to @bastianallgeier about the ctype fix a few months ago and he said it was about some server setups where mtime is apparently not really reliable. We both agree that we should get rid of this hack because it complicates so many other things and we are not even sure anymore if it's really needed.

@bastianallgeier With the additional background information on the origin of this hack, do you maybe remember again what the reason for the hack was? How do we proceed: Can we get rid of it completely (= go back to just filemtime()) or do we need a transition period of some sort – however that may look like (I don't see a possibility for one, maybe you do).

[ronaldtorres]

@fabianmichael Thanks for comment about it, I had the same problem for various months and I temporally fix it with custom file methods for generate a media hash that only use filemtime method.

[bastianallgeier]

@lukasbestle I think the source for this change had to do with FTP uploads and caching. When you deploy a new version that way you would end up with a filemtime that's still old although the file has changed. That's somehow how I remember it. I might be wrong.

I think we could risk to change it back. But we would need to test that again.

[lukasbestle]

@bastianallgeier I don't know how to reproduce this FTP upload issue. Could you please take a look?

[bastianallgeier]

@lukasbestle I don't know how to reproduce it either. This issue appeared in the very early days of Kirby. I only remember it very vaguely.

[lukasbestle]

@bastianallgeier Should we go forward with this then? I'd love to include the fix in 3.6 as it's technically a breaking change, but a very simple one.

[bastianallgeier]

@lukasbestle yes, please!

[bastianallgeier]

✅