Bug #67289
closed
Problems with cf_cache_imagesizes in TYPO3 7.2
100%
Description
In TYPO3 7.2 the old cache_imagesizes has been replaced by a caching framework implementation (#28484).
I see issues with this both in the behaviour as well as by examining TYPO3\CMS\Core\Imaging\GraphicalFunctions
Previously, the key to identify cache_imagesizes entries was "md5filename", built as the name says from md5 of filename.
(see the WHERE part in INSERT and DELETE queries of old implementation)
Now the cache identifier is $statusHash which is sha1() of file mtime and filesize.
(see method generateCacheKeyForImageFile)
Observations:
1. Problem with cache invalidation.
If the file is overwritten, the $statushash almost always changes.
Invalid Cache entry can't be identified anymore and removal is not possible.
In the previous implementation, file overwrite would lead to successful removal of old entries, because the cache key was not based on parameters that change when a file is overwritten but on filename hash. It would only fail on collisions of hash algorithm.
2. Ambiguous identifier
If two files have the same mtime and filesize, but different dimensions, their cache identifiers collide.
This is unlikely but can absolutely happen (I could demonstrate with test files)
This did not happen with the previous implementation (though md5 collisions were possible).
Since filemtime is a very easily changed parameter (might be the same for 1000s of files after an automatic import, or many files are touch()'ed for some reason) and filesize is not required to be unique inside a directory tree, the cache identifier is ambiguous.
3. Management hard to handle for third-party code
In the previous implementation cache_imagesizes was easily manageable:
If a script changed an image file, it could clear out any existing cached dimensions for the file by deleting the entries for that filename. In fact the file name was even kept in cleartext in the rows.
In the current implementation there is no direct access, one can try to access by capturing the previous filesize and mtime before changing the file but it is uncertain whether these haven't been changed already...
Also, $cache->get() and $cache->set() use $statusHash as identifier but $cache->remove tries to use $filePath
Shouldn't these operations use a consistent identifier for the cache?
It looks so when examining the methods in \TYPO3\CMS\Core\Cache\Backend\Typo3DatabaseBackend
I would suggest the following changes
- separate
$identifierand$statusHash - continue to use
$statusHashfor comparison against "hash" in stored data (change detection) $identifiercan be just sha1() of $filePath- use same
$identifierfor get, set, and remove, when we have found that statusHash is changed for the file
Then we should have neither the md5 collisions of old implementation, nor the filesize collisions of the new.
Also invalidation of outdated entries should work again.
Files
Updated by 
Updated by 
Updated by 
Updated by 
