Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Cache::getValue() should return false in case of missing key #247

Merged

Conversation

rhertogh
Copy link
Contributor

@rhertogh rhertogh commented May 16, 2022

Q A
Is bugfix? ✔️
New feature?
Breaks BC? ✔️/❌ (It now adheres to the documentation, but the return value is changed)
Fixed issues yiisoft/yii2/pull/19396

According to \yii\caching\Cache::getValue() the return value should be "false if the value is not in the cache or expired".

Currently null is returned by the getValue() method. When no serializer is specified that null is casted to a string and the native unserialize() function is used in \yii\caching\Cache::get(). The unserialize() function returns false on failure, this actually causes the get method to return the correct value.
However, when a serializer is specified (e.g. igbinary_unserialize) the null value from getValue() is not casted into a string. On PHP 8.1 this causes an error since a string is expected (https://www.php.net/manual/en/function.igbinary-unserialize.php).
When the getValue() method would have returned the expected false, the get() method would have returned it as is without trying to unserialize it, avoiding the error (yiisoft/yii2/blob/master/framework/caching/Cache.php#L135).

This PR fixes the Cache::getValue() method by returning false when the requested key is not found.

@samdark samdark added this to the 2.0.18 milestone May 22, 2022
@samdark samdark merged commit e1cae24 into yiisoft:master May 22, 2022
@samdark
Copy link
Member

samdark commented May 22, 2022

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants