Conversation
…ll in CacheTrait (#9669)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9669
Problem
In
FileSystemCacheItemPool::save(), the pool calledserialize($item->get()). This discarded all expiration metadata and cache item status on save. WhengetItem()was subsequently called, it instantiated anew TypedItem($key)and called$item->set(...), setting$isHittotrueand clearing any expiration. As a result, items never expired in the file cache, leading to stale authentication tokens being reused indefinitely. Additionally, inCacheTrait::getCachedValue(), when$cacheItem->isHit()was false, the method implicitly fell off without an explicit return statement.Solution
FileSystemCacheItemPool::save(): Directly serialize$item(serialize($item)) so expiration and cache metadata are retained.FileSystemCacheItemPool::getItem(): Unserialize the data; if it is an instance ofCacheItemInterface, return it directly. For backward compatibility with legacy cache files on disk containing raw serialized values, fallback to creating anew TypedItem($key)and setting the value.CacheTrait::getCachedValue(): Add an explicitreturn null;at the end of the method when cache items are missed or expired.Verification
testSaveAndGetExpiredIteminFileSystemCacheItemPoolTestverifying that expired cache items returnfalseforisHit()andnullforget().testGetItemLegacyCacheFileinFileSystemCacheItemPoolTestverifying backward compatibility with legacy serialized cache files.testFailsPullFromCacheWhenItemIsNotHitinCacheTraitTestverifying explicitnullreturn on cache miss.HOME=/tmp vendor/bin/phpunit Auth/tests/Cache/FileSystemCacheItemPoolTest.php(passed).HOME=/tmp vendor/bin/phpunit Auth/tests/CacheTraitTest.php(passed).vendor/bin/php-tools cs-fixer googleapis/google-cloud-php --ref HEAD(0 files that can be fixed).