TypeError Phpfastcache\Drivers\Files\Driver::driverRead(): Return value must be of type ?array, bool returned

[Jankyz]

What type of issue is this?

Exception/Error/Warning/Notice/Deprecation

Operating system + version

Linux 5.10.198-187.748.amzn2.x86_64

PHP version

8.0.26

Connector/Database version (if applicable)

No response

Phpfastcache version

9.1.3 ✅

Describe the issue you're facing

I'm being swamped with errors of this kind

TypeError Phpfastcache\Drivers\Files\Driver::driverRead(): Return value must be of type ?array, bool returned

I use Files as a driver
I think that this method has a problem with return from this

    return $this->decode($content);

inside we have

return $value ? \unserialize($value, ['allowed_classes' => true]) : null;

and when data cannot be unserialized then the function returns false

example of invalid serialized object

$content = 'a:1:{s:5:"Test";s:15:"serialize here!";}';

Expected behavior

return should be array|null

• I have searched over the Wiki

[github-actions]

Hello curious contributor !
Since it seems to be your first contribution, make sure that you've been:

• Reading and searching out our WIKI
• Reading and agreed with our Code Of Conduct
• Reading and understood our Coding Guideline
• Reading our README
  If everything looks unclear to you, tell us what 😄
  The Phpfastcache Team

[Geolim4]

Hello,

That's weird, unserialize only returns false upon invalid string passed through.

I'll take a look, but you are may facing corrupted stored cache files.

Cheers,Georges

[Jankyz]

it looks like Phpfastcache can serialize objects and save them (no errors), but it has a problem when we want to get the same objects and unserialize them...

[Jankyz]

Georges what about allowing people to choose between two methods in driver config

• serialize
• json_encode
  ?

[Geolim4]

Georges what about allowing people to choose between two methods in driver config

* serialize

* json_encode
  ?

Because unlike unserialize, json_decode does not allow de-serializing object with custom class names.

[jdalsem]

Just ran into this myself. It was indeed a corrupted cache file in my situation. Any reason why you are not handling this? The decode function could be returning null if the unserialize function returns false

In case the passed string is not unserializeable, false is returned and E_NOTICE is issued.

[Geolim4]

Just ran into this myself. It was indeed a corrupted cache file in my situation. Any reason why you are not handling this? The decode function could be returning null if the unserialize function returns false

In case the passed string is not unserializeable, false is returned and E_NOTICE is issued.

I'm not handling this case because if it happens it is a server issue rather than a Phpfastcache issue. So if a corrupted cache occurs the idea is to let you know be aware that there's an issue somewhere. No to handle it silently like nothing happened :)

[jdalsem]

I'm not handling this case because if it happens it is a server issue rather than a Phpfastcache issue. So if a corrupted cache occurs the idea is to let you know be aware that there's an issue somewhere. No to handle it silently like nothing happened :)

I understand, but you typehinted your functions, thus you are creating a coding issue. If you want this problem to be handled upstream, why not throw an exception?

[Geolim4]

I understand, but you typehinted your functions, thus you are creating a coding issue. If you want this problem to be handled upstream, why not throw an exception?

I'm not creating a coding issue, your server is causing coding issue by storing corrupted files expecting Phpfastcache to silently handle the issue and get over it. If an exception is thrown or a PHP error appears, the result is still the same: An error somewhere in your application prevent Phpfastcache from running smoothly. Badly unserialize is one of the functions that should've thrown on invalid (like json_decode does now), but it still doesn't and return scalar on error instead of throwing exception.

So until now I felt no necessity to catch this kind of error because it is very rare and outside of Phpfastcache scope, at least in your case. 9.2 is coming in 2024, I'll add an exception on this block of code, but again, it is not a coding issue, just a server issue.

[jdalsem]

I'm not creating a coding issue

If your return types do not match, it is a coding issue. (mismatch between driverRead and decode return types)

If an exception is thrown or a PHP error appears, the result is still the same

With an exception i can determine upstream if it is a cache contents issue. A php coding error should not be used to identify this.

An error somewhere in your application

Corruption can happen for many reasons. In my experience it is almost never an application or phpfastcache issue, but just some weird stuff happening on disk.

The biggest problem i have is that i can not 'repair' the corrupted data, as the code crashes on a PHP issue. With an exception thrown i might be able to catch and delete the cache contents (hopefully that will not trigger the same issue).

[Geolim4]

For now I consider that issue as an exclusive server-side issue.
I can make a better error handling if you'd like so, but it is a low priority until next month since the 9.2 will be shipped with some new features, driver updates and internal performances improvements.

That block of code never thrown any errors until now even in the worst conditions with huge stress tests configured on the CI :/

[jdalsem]

I can make a better error handling if you'd like so

That would be nice.

That block of code never thrown any errors until now even in the worst conditions with huge stress tests configured on the CI :/

I think (for now) it is just a coïncidence it happend. We are using phpfastcache multiple years without any issue related to corrupted cache contents, so there is no need to rush anything now.

[Geolim4]

Will be implemented as of v9.2.