Framework: RedisCache yields unserialization error when set to a locale that does not use a dot as decimal point

[makuser]

• Laravel Version: 5.5.27
• PHP Version: 7.0.22-1~dotdeb+8.1
• Database Driver & Version: predis v1.1.1 & redis-server v3.2.6-13.2.6-1

Description:

Remembering a decimal number when set to a locale that does not use a dot as decimal point, will yield an unserialization error.

Logdump:

local.ERROR: unserialize(): Error at offset 0 of 4 bytes {"userId":1,"email":"[email protected]","exception":"[object] (ErrorException(code: 0): unserialize(): Error at offset 0 of 4 bytes at /proj/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php:287)
[stacktrace]
#0 [internal function]: Illuminate\\Foundation\\Bootstrap\\HandleExceptions->handleError(8, 'unserialize(): Error at offset 0 of 4 bytes', '/proj/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php', 287, array('value' => '2,29'))
#1 /proj/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php(287): unserialize('2,29')
#2 /proj/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php(56): Illuminate\\Cache\\RedisStore->unserialize('2,29')
#3 /proj/vendor/laravel/framework/src/Illuminate/Cache/Repository.php(86): Illuminate\\Cache\\RedisStore->get('test')
#4 /proj/vendor/laravel/framework/src/Illuminate/Cache/Repository.php(318): Illuminate\\Cache\\Repository->get('test')
#5 /proj/vendor/laravel/framework/src/Illuminate/Cache/CacheManager.php(304): Illuminate\\Cache\\Repository->remember('test', 1, Object(Closure))
#6 /proj/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(221): Illuminate\\Cache\\CacheManager->__call('remember', array('test', 1, object(Closure)))
#7 /proj/app/User.php(219): Illuminate\\Support\\Facades\\Facade::__callStatic('remember', Array)
#8 /proj/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(438): App\\User->getRankingVolumeAttribute(NULL)
#9 /proj/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(340): Illuminate\\Database\\Eloquent\\Model->mutateAttribute('ranking_volume', NULL)
#10 /proj/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(313): Illuminate\\Database\\Eloquent\\Model->getAttributeValue('ranking_volume')
#11 /proj/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1383): Illuminate\\Database\\Eloquent\\Model->getAttribute('ranking_volume')
#12 /proj/vendor/laravel/framework/src/Illuminate/Support/Arr.php(148): Illuminate\\Database\\Eloquent\\Model->offsetExists('ranking_volume')
#13 /proj/vendor/laravel/framework/src/Illuminate/Support/helpers.php(472): Illuminate\\Support\\Arr::exists(Object(App\\User), 'ranking_volume')
#14 /proj/vendor/laravel/framework/src/Illuminate/Support/Collection.php(1547): data_get(Object(App\\User), Array)
#15 /proj/vendor/laravel/framework/src/Illuminate/Support/Collection.php(1382): Illuminate\\Support\\Collection->Illuminate\\Support\\{closure}(Object(App\\User), 0)
#16 /proj/vendor/laravel/framework/src/Illuminate/Support/Collection.php(1407): Illuminate\\Support\\Collection->sortBy(Object(Closure), 0, true)
#17 /proj/app/Http/Controllers/RankingController.php(63): Illuminate\\Support\\Collection->sortByDesc('ranking_volume')
#18 [internal function]: App\\Http\\Controllers\\RankingController->index(Object(Illuminate\\Http\\Request))
#19 /proj/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): call_user_func_array(Array, Array)
#20 /proj/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(45): Illuminate\\Routing\\Controller->callAction('index', Array)
#21 /proj/vendor/laravel/framework/src/Illuminate/Routing/Route.php(212): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(App\\Http\\Controllers\\RankingController), 'index')
#22 /proj/vendor/laravel/framework/src/Illuminate/Routing/Route.php(169): Illuminate\\Routing\\Route->runController()
#23 /proj/vendor/laravel/framework/src/Illuminate/Routing/Router.php(658): Illuminate\\Routing\\Route->run()
#24 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(30): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#25 /proj/app/Http/Middleware/Locale.php(26): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#26 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): App\\Http\\Middleware\\Locale->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#27 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#28 /proj/vendor/laravel/framework/src/Illuminate/Routing/Middleware/SubstituteBindings.php(41): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#29 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Routing\\Middleware\\SubstituteBindings->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#30 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#31 /proj/vendor/laravel/framework/src/Illuminate/Auth/Middleware/Authenticate.php(43): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#32 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Auth\\Middleware\\Authenticate->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#33 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#34 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/VerifyCsrfToken.php(67): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#35 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Foundation\\Http\\Middleware\\VerifyCsrfToken->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#36 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#37 /proj/vendor/laravel/framework/src/Illuminate/View/Middleware/ShareErrorsFromSession.php(49): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#38 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\View\\Middleware\\ShareErrorsFromSession->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#39 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#40 /proj/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(63): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#41 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Session\\Middleware\\StartSession->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#42 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#43 /proj/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/AddQueuedCookiesToResponse.php(37): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#44 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Cookie\\Middleware\\AddQueuedCookiesToResponse->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#45 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#46 /proj/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php(59): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#47 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Cookie\\Middleware\\EncryptCookies->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#48 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#49 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(102): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#50 /proj/vendor/laravel/framework/src/Illuminate/Routing/Router.php(660): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#51 /proj/vendor/laravel/framework/src/Illuminate/Routing/Router.php(635): Illuminate\\Routing\\Router->runRouteWithinStack(Object(Illuminate\\Routing\\Route), Object(Illuminate\\Http\\Request))
#52 /proj/vendor/laravel/framework/src/Illuminate/Routing/Router.php(601): Illuminate\\Routing\\Router->runRoute(Object(Illuminate\\Http\\Request), Object(Illuminate\\Routing\\Route))
#53 /proj/vendor/laravel/framework/src/Illuminate/Routing/Router.php(590): Illuminate\\Routing\\Router->dispatchToRoute(Object(Illuminate\\Http\\Request))
#54 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(176): Illuminate\\Routing\\Router->dispatch(Object(Illuminate\\Http\\Request))
#55 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(30): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}(Object(Illuminate\\Http\\Request))
#56 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(30): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#57 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#58 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#59 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(30): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#60 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#61 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#62 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#63 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#64 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#65 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/CheckForMaintenanceMode.php(46): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#66 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(149): Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#67 /proj/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#68 /proj/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(102): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#69 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(151): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#70 /proj/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(116): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter(Object(Illuminate\\Http\\Request))
#71 /proj/public/index.php(53): Illuminate\\Foundation\\Http\\Kernel->handle(Object(Illuminate\\Http\\Request))
#72 {main}
"} 

Steps To Reproduce:

setlocale(LC_ALL, 'de_DE.UTF-8');

$cached = Cache::remember('test', 1, function () {
    return 1 + 1.29;
});

var_dump($cached);

Expectation:

var_dump($cached) would return float(2,29).

[mfn]

I think your "steps to reproduce" miss the essential step _after_ the var_dump($cached), namely:

Cache::get('test');

because then it will throw an exception as you said.

[makuser]

@mfn are you familiar with the Cache::remember() command? It 'combines' set and get along with an expiration time, such that a self-made cache expiration logic is not required.

So to fully test Cache::remember() the code needs to be run at least twice for it to happen. The problem occurs when receiving data from the cache database (redis in this case), so it will not occur the first time.

BTW: I checked the redis database, and it does indeed hold the value "2,29".

[mfn]

are you familiar with the Cache::remember() command?

I thought I'm. Just wanted to point out that your steps don't show the error unless what I said 🤷‍♀️

[36864]

This seems to be an issue with the way laravel decides whether or not to serialize/unserialize a value before storing.
https://github.com/laravel/framework/blob/2b4ee74ce61a860fcb02f6635f72bf7c4b18d7a9/src/Illuminate/Cache/RedisStore.php#L276

In this case, the value is numeric (1 + 1.29 = 2.29) so it isn't serialized when being stored. Since the locale uses commas as a decimal separator, when the value is written it will be written as 2,29, which is not a numeric value when it is read, which results in an attempt to unserialize the value 2,29 which is not a valid target for unserialization.

I don't have a local redis environment so I'm unable to test this, but could you please try changing lines 276 and 287 of Illuminate\Cache\RedisStore to remove the is_numeric check and just return serialize($value)/unserialize($value) respectively and see if it fixes your issue?

[sisve]

@36864 That sounds like something that would fix this specific issue, but it would also break things that worked with these values directly; either external systems or perhaps statistical functions in redis itself.

.NET has a InvariantCulture (which just so happen to be very similar to en-US...) that is usually use for serializing and deserializing data. Does php have anything similar for converting scalars to strings? Is this one of those LC_*-constants that setlocale uses? Do we need to wrap all our serialize/deserialize code to set a common predefined locale and restore the previous one afterwards?

[36864]

@sisve Sorry if I was unclear. I only asked the OP to try that change to determine if that's the reason this is happening, since I can't test this locally.

Once it is confirmed that the issue lies here, we can discuss how to fix it.

I think wrapping serialization as you've described would be a good solution to the problem, however it doesn't really prevent breaking external systems. Any system that is expecting to find a "raw" numeric value in a specific locale would fail That said, it might be easier to change a locale than to start deserializing values.

That said, worrying about external applications accessing our application cache feels a bit out-of-scope for what a framework should do. Personally I don't see why numeric values should be treated differently from all other data types for caching. Happy to be proved wrong here.

[mfn]

Not op, but I've local and redis and the changed proposed by @36864 make this work:

Before

>>> setlocale(LC_ALL, 'de_DE.UTF-8');
=> "de_DE.UTF-8"
>>> $cached = Cache::remember('test', 1, function () {
...     return 1 + 1.29;
... });
=> 2.29
>>> var_dump($cached);
float(2,29)
=> null
>>> Cache::get('test');
PHP Notice:  unserialize(): Error at offset 0 of 4 bytes in /vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php on line 287

After

>>> setlocale(LC_ALL, 'de_DE.UTF-8');                                                                                                                                                  => "de_DE.UTF-8"
>>> $cached = Cache::remember('test', 1, function () {                                                                                                                                     return 1 + 1.29;                                                                                                                                                                   });
=> 2.29
>>> var_dump($cached);
float(2,29)
=> null
>>> $value = Cache::get('test');
=> 2.29
>>> var_dump($value);
float(2,29)
=> null

However…

• This change was introduced in https://github.com/laravel/framework/commit/8493e10f9662fbd2adfe104fd7b49fce5607f415
• It will break two tests if changed:
  
  
  
  • \Illuminate\Tests\Cache\CacheRedisStoreTest::testRedisValueIsReturnedForNumerics and
  
  
  • \Illuminate\Tests\Cache\CacheRedisStoreTest::testSetMethodProperlyCallsRedisForNumerics
  
  

[36864]

Thanks @mfn.

While it would be nice to understand why that change was made in the first place, I guess we don't want to break existing tests, so @sisve's suggestion of forcing a specific locale for serialization is our best bet. Even though I pointed out that it might break existing external applications, I would argue that it's an unlikely scenario to be putting values into the laravel cache to be used only by external applications and never by laravel.

According to the php documentation, it should be sufficient to set LC_NUMERIC to any locale that uses periods for decimal points. Something like this wrapping both serialize and unserialize:

$current_numeric_locale = setlocale(LC_NUMERIC, 0);
setlocale(LC_NUMERIC, 'en');
//serialize/unserialize
setlocale(LC_NUMERIC, $current_numeric_locale);

[sisve]

My worry about breaking external applications was about using the builtin serialize/unserialize methods and their corresponding serialization format. I'm all for forcing the locale for our own serialization routines so they are consistent. I also doubt we'll break much by forcing the locale, since the current behavior doesn't even work for ourselves at the moment.

Would json_encode work in this specific case? The json standard enforces a decimal point as a separator. json_encode(13.37) === '13.37'. It's probably a hack, but it would at least work for numeric values (which this issue is about).

[36864]

Well, either solution is arguably a hack.

At this point I think it would be important to know why the change was made to not serialize numeric values and if the current implementation still makes sense 4 years later. ping @taylorotwell

[TheOpenDevProject]

@taylorotwell Any answers to this?

[driesvints]

I think this is kind of related to the changes I made to Horizon about a year ago: https://github.com/laravel/horizon/pull/477

The thing is that I don't think there's any good solution for this and that it's best to make sure that you always store with a decimal point to avoid problems with locales?

[taylorotwell]

There is nothing we can do here without breaking other systems, so I see little point in keeping this open to be honest. It can be worked around in user land.