From 97253252366bec440febce1fe426eed9091cd68f Mon Sep 17 00:00:00 2001 From: OndraDol Date: Sat, 4 Apr 2026 22:34:36 +0200 Subject: [PATCH 1/2] fix(FileStorage): harden readMetaAndLock() and readData() against non-cache files (#45) Replace unguarded unserialize() + assert(is_array()) with @unserialize() and is_array() runtime validation in readMetaAndLock(). Non-cache files (e.g. Latte lock files matching the _* pattern) now return null instead of crashing with "unserialize(): Error at offset". Add defensive @unserialize() in readData() to handle corrupted serialized data from truncated writes. --- src/Caching/Storages/FileStorage.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Caching/Storages/FileStorage.php b/src/Caching/Storages/FileStorage.php index 35a0376..15c68ac 100644 --- a/src/Caching/Storages/FileStorage.php +++ b/src/Caching/Storages/FileStorage.php @@ -309,13 +309,14 @@ protected function readMetaAndLock(string $file, int $lock): ?array $size = (int) stream_get_contents($handle, self::MetaHeaderLen); if ($size) { - $meta = stream_get_contents($handle, $size, self::MetaHeaderLen); - if ($meta !== false) { - $meta = unserialize($meta); - assert(is_array($meta)); - $meta[self::File] = $file; - $meta[self::Handle] = $handle; - return $meta; + $raw = stream_get_contents($handle, $size, self::MetaHeaderLen); + if ($raw !== false) { + $meta = @unserialize($raw); // @ - file may not be a valid cache file + if (is_array($meta)) { + $meta[self::File] = $file; + $meta[self::Handle] = $handle; + return $meta; + } } } @@ -337,7 +338,7 @@ protected function readData(array $meta): mixed return match (true) { $data === false => null, empty($meta[self::MetaSerialized]) => $data, - default => unserialize($data), + default => @unserialize($data), // @ - data may be corrupted by a truncated write }; } From 8aa08c70a95146366e024f1779bdb1f7fe6f4375 Mon Sep 17 00:00:00 2001 From: OndraDol Date: Sat, 4 Apr 2026 22:36:15 +0200 Subject: [PATCH 2/2] test(FileStorage): add regression tests for GC with foreign files (#45) Add FileStorage.gc-foreign-files.phpt: - Verifies clean() does not crash when non-cache files (lock files, binary files, empty files) matching _* pattern exist in cache dir - Tests both collector mode (skip foreign files) and Cache::All mode (delete everything) Add FileStorage.corrupted.phpt: - Verifies graceful handling of corrupted metadata, corrupted data payload, truncated files, and all-zero files Enhance FileStorage.stress.phpt: - Place foreign files in cache dir during stress test to verify they don't interfere with concurrent cache operations --- tests/Storages/FileStorage.corrupted.phpt | 118 ++++++++++++++++++ .../FileStorage.gc-foreign-files.phpt | 88 +++++++++++++ tests/Storages/FileStorage.stress.phpt | 8 +- 3 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 tests/Storages/FileStorage.corrupted.phpt create mode 100644 tests/Storages/FileStorage.gc-foreign-files.phpt diff --git a/tests/Storages/FileStorage.corrupted.phpt b/tests/Storages/FileStorage.corrupted.phpt new file mode 100644 index 0000000..6fbeba3 --- /dev/null +++ b/tests/Storages/FileStorage.corrupted.phpt @@ -0,0 +1,118 @@ +save('meta-test', 'hello'); +Assert::same('hello', $cache->load('meta-test')); + +// Find the cache file and corrupt its metadata +$files = glob($dir . '/_*'); +Assert::truthy($files); + +foreach ($files as $file) { + if (is_file($file)) { + // Overwrite with a valid-looking size header but garbage meta. + // "000010" = meta size of 10, followed by non-serialized data. + file_put_contents($file, '000010not-serial'); + } +} + +// Reading corrupted meta should return null, not crash +Assert::null($cache->load('meta-test')); + + +// --- Corrupted serialized data --- + +// Use a fresh storage to avoid interference +$dir2 = getTempDir() . '/data-test'; +@mkdir($dir2, 0777, true); +$storage2 = new FileStorage($dir2); +$cache2 = new Cache($storage2); + +$cache2->save('data-test', ['complex' => 'array', 'with' => [1, 2, 3]]); +Assert::type('array', $cache2->load('data-test')); + +// Find the cache file +$files2 = glob($dir2 . '/_*'); +Assert::truthy($files2); + +foreach ($files2 as $file) { + if (is_file($file)) { + $content = file_get_contents($file); + $size = (int) substr($content, 0, 6); + $headerAndMeta = substr($content, 0, 6 + $size); + // Keep valid header+meta but replace data with garbage + file_put_contents($file, $headerAndMeta . 'CORRUPTED-DATA-NOT-SERIALIZABLE'); + } +} + +// Reading corrupted data should not crash. +// The result may be false (failed unserialize) but must not throw. +Assert::noError(function () use ($cache2) { + $cache2->load('data-test'); +}); + + +// --- Truncated file --- + +$dir3 = getTempDir() . '/truncated-test'; +@mkdir($dir3, 0777, true); +$storage3 = new FileStorage($dir3); +$cache3 = new Cache($storage3); + +$cache3->save('trunc-test', str_repeat('x', 10000)); +Assert::same(str_repeat('x', 10000), $cache3->load('trunc-test')); + +// Truncate the file mid-data +$files3 = glob($dir3 . '/_*'); +foreach ($files3 as $file) { + if (is_file($file)) { + $handle = fopen($file, 'r+b'); + ftruncate($handle, 50); // keep header + partial meta + fclose($handle); + } +} + +// Should return null, not crash +Assert::null($cache3->load('trunc-test')); + + +// --- File with all zero bytes --- + +$dir4 = getTempDir() . '/zeros-test'; +@mkdir($dir4, 0777, true); +$storage4 = new FileStorage($dir4); +$cache4 = new Cache($storage4); + +$cache4->save('zero-test', 'data'); + +$files4 = glob($dir4 . '/_*'); +foreach ($files4 as $file) { + if (is_file($file)) { + file_put_contents($file, str_repeat("\x00", 100)); + } +} + +// All-zero header = size 0, should return null +Assert::null($cache4->load('zero-test')); diff --git a/tests/Storages/FileStorage.gc-foreign-files.phpt b/tests/Storages/FileStorage.gc-foreign-files.phpt new file mode 100644 index 0000000..bf95146 --- /dev/null +++ b/tests/Storages/FileStorage.gc-foreign-files.phpt @@ -0,0 +1,88 @@ +save('key1', 'value1'); +$cache->save('key2', 'value2'); + +Assert::same('value1', $cache->load('key1')); +Assert::same('value2', $cache->load('key2')); + + +// Create foreign files that match the _* pattern used by GC. +// These simulate Latte lock files and other non-cache files that may +// coexist in the same temp directory tree. + +// Case 1: Content starting with digits — triggers (int) cast to non-zero size +// in readMetaAndLock(), then garbage gets passed to unserialize(). +// This is the exact scenario from GH#45 diagnosis by @martinbohmcz: +// PHP 8 interprets "0942e7" as float 942e7, (int) cast = 9420000. +file_put_contents($dir . '/_includes-template.php.lock', '0942e7deadbeef'); + +// Case 2: Binary content +file_put_contents($dir . '/_session.lock', "\xff\xfe\x00\x01\x80\x90binary"); + +// Case 3: Empty file +file_put_contents($dir . '/_empty.lock', ''); + +// Case 4: File with zero-like header (6 bytes of zeros = size 0, skip path) +file_put_contents($dir . '/_zero-header.tmp', '000000some-data'); + +// Case 5: File with valid-looking size but garbage meta +file_put_contents($dir . '/_fake-meta.dat', '000010not-a-serialized-array'); + +// Case 6: File in a subdirectory matching _* pattern +@mkdir($dir . '/_subdir', 0777, true); +file_put_contents($dir . '/_subdir/_nested.lock', 'nested-lock-data'); + + +// GC (collector mode) must not crash on foreign files +Assert::noError(function () use ($storage) { + $storage->clean([]); +}); + + +// Valid cache entries must still be readable +Assert::same('value1', $cache->load('key1')); +Assert::same('value2', $cache->load('key2')); + + +// Foreign files must be untouched (GC should skip, not delete them) +Assert::true(file_exists($dir . '/_includes-template.php.lock')); +Assert::true(file_exists($dir . '/_session.lock')); +Assert::true(file_exists($dir . '/_empty.lock')); +Assert::true(file_exists($dir . '/_zero-header.tmp')); +Assert::true(file_exists($dir . '/_fake-meta.dat')); +Assert::true(file_exists($dir . '/_subdir/_nested.lock')); + + +// Cache::All mode deletes all _* files including foreign ones — expected behavior +$storage->clean([Cache::All => true]); + +Assert::null($cache->load('key1')); +Assert::null($cache->load('key2')); +Assert::false(file_exists($dir . '/_includes-template.php.lock')); +Assert::false(file_exists($dir . '/_session.lock')); +Assert::false(file_exists($dir . '/_empty.lock')); +Assert::false(file_exists($dir . '/_zero-header.tmp')); +Assert::false(file_exists($dir . '/_fake-meta.dat')); diff --git a/tests/Storages/FileStorage.stress.phpt b/tests/Storages/FileStorage.stress.phpt index 0485da0..c118593 100644 --- a/tests/Storages/FileStorage.stress.phpt +++ b/tests/Storages/FileStorage.stress.phpt @@ -31,7 +31,8 @@ function checkStr($s) define('COUNT_FILES', 3); -$storage = new FileStorage(getTempDir()); +$dir = getTempDir(); +$storage = new FileStorage($dir); // clear playground @@ -39,6 +40,11 @@ for ($i = 0; $i <= COUNT_FILES; $i++) { $storage->write((string) $i, randomStr(), []); } +// GH#45: place foreign files in the cache directory to verify they don't +// interfere with normal cache operations or cause unserialize errors during GC +file_put_contents($dir . '/_foreign.lock', '0942e7deadbeef'); +file_put_contents($dir . '/_session.tmp', "\xff\xfe\x00\x01binary"); + // test loop $hits = ['ok' => 0, 'notfound' => 0, 'error' => 0, 'cantwrite' => 0, 'cantdelete' => 0]; for ($counter = 0; $counter < 1000; $counter++) {