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

Double serialization #18

Open
judgej opened this issue Nov 7, 2019 · 4 comments
Open

Double serialization #18

judgej opened this issue Nov 7, 2019 · 4 comments

Comments

@judgej
Copy link

judgej commented Nov 7, 2019

I got this error when fetching an item from cache:

In CacheItemPool.php line 49:
unserialize(): Error at offset 0 of 2 bytes

Looking at the item that was being unserialized here, it was:

Illuminate\Support\Collection^ {#518
  #items: []
}

It was not a string. It was already deserialized. So looking into the code, it seems that this package will serialize and unserialize items that go into and come out of the Laravel/Illuminate cache. Laravel will serialize/unserialize data going in and out anyway, so it's done twice. Maybe the older versions of Laravel did not work this way, and this is a leftover.

The reason I believe I got this error, is that I hit a cached item that had been pushed to cache using an Illuminate\Contracts\Cache\Repository repository - the standard Laravel way - and pulled out using Madewithlove\IlluminatePsrCacheBridge\Laravel\CacheItemPool. The two store data in the underlying cache that is not compatible - the one stored by this package is serialized twice.

Is this intended? It means cache items can be stored and retreived by either the Laravel cache, or the madewithlove cache, but can never be passed between the two.

@judgej
Copy link
Author

judgej commented Nov 7, 2019

To illustrate further, in a service I created a cache item with this package and read it using the laravel cache:

$cacheItem = new \Madewithlove\IlluminatePsrCacheBridge\Laravel\CacheItem('xyz');
$cacheItem->set('foobar')
$this->cacheItemPool->save($cacheItem);
dump(\Cache::get('xyz'));

I would expect to dump foobar, but instead get this:

s:6:"foobar";

It's not an exception thrown, but it's the wrong value; it's not what was cached. They are not playing nice together. I believe they should, and they can if the additional layer of serialization is removed. In the meantime, it's an accident waiting to happen, as it did for me.

(Edited as I wasn't using set() correctly in my first example.)

@judgej
Copy link
Author

judgej commented Nov 7, 2019

If this were fixed to not serialize items going into the Lumen cache, then the items coming out would not need deserializing. However, to support a switchover, this package could check if the data out looks like a serialized string and attempt to deserialize it if it is. It's not foolproof, because the caching pool should not need to do deep-reading of the cached data, and should treat all items in the same consistent way, but it may help. Maybe even as a decorator so the workaround could be added only by people who want to used it.

@dinamic
Copy link

dinamic commented Nov 14, 2019

I'm having issues with this as well.

@hannesvdvreken thoughts?

@hannesvdvreken
Copy link
Contributor

passing on to @Anahkiasen

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

No branches or pull requests

3 participants