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

[5.6] Caching collections with database (PostgreSQL) cache driver doesn't work #25466

Closed
chescos opened this issue Sep 5, 2018 · 21 comments
Closed

Comments

@chescos
Copy link

chescos commented Sep 5, 2018

  • Laravel Version: 5.6.38
  • PHP Version: 7.2.5
  • Database Driver & Version: PostgreSQL 10.3

Description:

Caching collections when using the database driver doesn't work when using PostgreSQL. The issue seems to be that in PostgreSQL null bytes can't be stored in columns of the type text.

This is what ends up being saved into the database:

O:39:"Illuminate\Database\Eloquent\Collection":1:{s:8:"

This string was serialized but it's cut off and therefore corrupted, and it can not be unserialized. When trying to read it from cache, this exception is thrown:

unserialize(): Error at offset 52 of 55 bytes

This is an example SQL query that ends up saving a corrupted serialized string into the database:

[2018-09-05 14:38:34] local.INFO: array (
  'query' => 'insert into "cache" ("key", "value", "expiration") values (?, ?, ?)',
  'bindings' => 
  array (
    0 => 'app_name_cachespatie.permission.cache',
    1 => 'O:39:"Illuminate\\Database\\Eloquent\\Collection":1:{s:8:"' . "\0" . '*' . "\0" . 'items";a:1:{i:0;O:35:"Spatie\\Permission\\Models\\Permission":28:{s:7:"guarded";a:1:{i:0;s:2:"id";}s:13:"' . "\0" . '*' . "\0" . 'connection";s:5:"pgsql";s:8:"' . "\0" . '*' . "\0" . 'table";s:11:"permissions";s:13:"' . "\0" . '*' . "\0" . 'primaryKey";s:2:"id";s:10:"' . "\0" . '*' . "\0" . 'keyType";s:3:"int";s:12:"incrementing";b:1;s:7:"' . "\0" . '*' . "\0" . 'with";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'withCount";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'perPage";i:15;s:6:"exists";b:1;s:18:"wasRecentlyCreated";b:0;s:13:"' . "\0" . '*' . "\0" . 'attributes";a:5:{s:2:"id";i:19;s:4:"name";s:6:"q2Yk6f";s:10:"guard_name";s:3:"web";s:10:"created_at";s:19:"2018-09-05 14:36:59";s:10:"updated_at";s:19:"2018-09-05 14:36:59";}s:11:"' . "\0" . '*' . "\0" . 'original";a:5:{s:2:"id";i:19;s:4:"name";s:6:"q2Yk6f";s:10:"guard_name";s:3:"web";s:10:"created_at";s:19:"2018-09-05 14:36:59";s:10:"updated_at";s:19:"2018-09-05 14:36:59";}s:10:"' . "\0" . '*' . "\0" . 'changes";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'casts";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'dates";a:0:{}s:13:"' . "\0" . '*' . "\0" . 'dateFormat";N;s:10:"' . "\0" . '*' . "\0" . 'appends";a:0:{}s:19:"' . "\0" . '*' . "\0" . 'dispatchesEvents";a:0:{}s:14:"' . "\0" . '*' . "\0" . 'observables";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'relations";a:1:{s:5:"roles";O:39:"Illuminate\\Database\\Eloquent\\Collection":1:{s:8:"' . "\0" . '*' . "\0" . 'items";a:1:{i:0;O:29:"Spatie\\Permission\\Models\\Role":27:{s:7:"guarded";a:1:{i:0;s:2:"id";}s:13:"' . "\0" . '*' . "\0" . 'connection";s:5:"pgsql";s:8:"' . "\0" . '*' . "\0" . 'table";s:5:"roles";s:13:"' . "\0" . '*' . "\0" . 'primaryKey";s:2:"id";s:10:"' . "\0" . '*' . "\0" . 'keyType";s:3:"int";s:12:"incrementing";b:1;s:7:"' . "\0" . '*' . "\0" . 'with";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'withCount";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'perPage";i:15;s:6:"exists";b:1;s:18:"wasRecentlyCreated";b:0;s:13:"' . "\0" . '*' . "\0" . 'attributes";a:5:{s:2:"id";i:19;s:4:"name";s:6:"DWZhv1";s:10:"guard_name";s:3:"web";s:10:"created_at";s:19:"2018-09-05 14:36:59";s:10:"updated_at";s:19:"2018-09-05 14:36:59";}s:11:"' . "\0" . '*' . "\0" . 'original";a:7:{s:2:"id";i:19;s:4:"name";s:6:"DWZhv1";s:10:"guard_name";s:3:"web";s:10:"created_at";s:19:"2018-09-05 14:36:59";s:10:"updated_at";s:19:"2018-09-05 14:36:59";s:19:"pivot_permission_id";i:19;s:13:"pivot_role_id";i:19;}s:10:"' . "\0" . '*' . "\0" . 'changes";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'casts";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'dates";a:0:{}s:13:"' . "\0" . '*' . "\0" . 'dateFormat";N;s:10:"' . "\0" . '*' . "\0" . 'appends";a:0:{}s:19:"' . "\0" . '*' . "\0" . 'dispatchesEvents";a:0:{}s:14:"' . "\0" . '*' . "\0" . 'observables";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'relations";a:1:{s:5:"pivot";O:44:"Illuminate\\Database\\Eloquent\\Relations\\Pivot":29:{s:11:"pivotParent";O:35:"Spatie\\Permission\\Models\\Permission":28:{s:7:"guarded";a:1:{i:0;s:2:"id";}s:13:"' . "\0" . '*' . "\0" . 'connection";N;s:8:"' . "\0" . '*' . "\0" . 'table";s:11:"permissions";s:13:"' . "\0" . '*' . "\0" . 'primaryKey";s:2:"id";s:10:"' . "\0" . '*' . "\0" . 'keyType";s:3:"int";s:12:"incrementing";b:1;s:7:"' . "\0" . '*' . "\0" . 'with";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'withCount";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'perPage";i:15;s:6:"exists";b:0;s:18:"wasRecentlyCreated";b:0;s:13:"' . "\0" . '*' . "\0" . 'attributes";a:1:{s:10:"guard_name";s:3:"web";}s:11:"' . "\0" . '*' . "\0" . 'original";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'changes";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'casts";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'dates";a:0:{}s:13:"' . "\0" . '*' . "\0" . 'dateFormat";N;s:10:"' . "\0" . '*' . "\0" . 'appends";a:0:{}s:19:"' . "\0" . '*' . "\0" . 'dispatchesEvents";a:0:{}s:14:"' . "\0" . '*' . "\0" . 'observables";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'relations";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'touches";a:0:{}s:10:"timestamps";b:1;s:9:"' . "\0" . '*' . "\0" . 'hidden";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'visible";a:0:{}s:11:"' . "\0" . '*' . "\0" . 'fillable";a:0:{}s:46:"' . "\0" . 'Spatie\\Permission\\Models\\Permission' . "\0" . 'roleClass";N;s:52:"' . "\0" . 'Spatie\\Permission\\Models\\Permission' . "\0" . 'permissionClass";N;}s:13:"' . "\0" . '*' . "\0" . 'foreignKey";s:13:"permission_id";s:13:"' . "\0" . '*' . "\0" . 'relatedKey";s:7:"role_id";s:10:"' . "\0" . '*' . "\0" . 'guarded";a:0:{}s:13:"' . "\0" . '*' . "\0" . 'connection";N;s:8:"' . "\0" . '*' . "\0" . 'table";s:20:"role_has_permissions";s:13:"' . "\0" . '*' . "\0" . 'primaryKey";s:2:"id";s:10:"' . "\0" . '*' . "\0" . 'keyType";s:3:"int";s:12:"incrementing";b:1;s:7:"' . "\0" . '*' . "\0" . 'with";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'withCount";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'perPage";i:15;s:6:"exists";b:1;s:18:"wasRecentlyCreated";b:0;s:13:"' . "\0" . '*' . "\0" . 'attributes";a:2:{s:13:"permission_id";i:19;s:7:"role_id";i:19;}s:11:"' . "\0" . '*' . "\0" . 'original";a:2:{s:13:"permission_id";i:19;s:7:"role_id";i:19;}s:10:"' . "\0" . '*' . "\0" . 'changes";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'casts";a:0:{}s:8:"' . "\0" . '*' . "\0" . 'dates";a:0:{}s:13:"' . "\0" . '*' . "\0" . 'dateFormat";N;s:10:"' . "\0" . '*' . "\0" . 'appends";a:0:{}s:19:"' . "\0" . '*' . "\0" . 'dispatchesEvents";a:0:{}s:14:"' . "\0" . '*' . "\0" . 'observables";a:0:{}s:12:"' . "\0" . '*' . "\0" . 'relations";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'touches";a:0:{}s:10:"timestamps";b:0;s:9:"' . "\0" . '*' . "\0" . 'hidden";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'visible";a:0:{}s:11:"' . "\0" . '*' . "\0" . 'fillable";a:0:{}}}s:10:"' . "\0" . '*' . "\0" . 'touches";a:0:{}s:10:"timestamps";b:1;s:9:"' . "\0" . '*' . "\0" . 'hidden";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'visible";a:0:{}s:11:"' . "\0" . '*' . "\0" . 'fillable";a:0:{}s:46:"' . "\0" . 'Spatie\\Permission\\Models\\Role' . "\0" . 'permissionClass";N;}}}}s:10:"' . "\0" . '*' . "\0" . 'touches";a:0:{}s:10:"timestamps";b:1;s:9:"' . "\0" . '*' . "\0" . 'hidden";a:0:{}s:10:"' . "\0" . '*' . "\0" . 'visible";a:0:{}s:11:"' . "\0" . '*' . "\0" . 'fillable";a:0:{}s:46:"' . "\0" . 'Spatie\\Permission\\Models\\Permission' . "\0" . 'roleClass";N;s:52:"' . "\0" . 'Spatie\\Permission\\Models\\Permission' . "\0" . 'permissionClass";N;}}}',
    2 => 1536237514,
  ),
  'time' => 1.31,
)

Steps To Reproduce:

  1. Configure the config/database.php with the PostgreSQL driver and the config/cache.php with the database driver
  2. Create the cache table with php artisan cache:table and migrate it
  3. Get any collection from the database, e.g $collection = User::limit(3)->get()
  4. Cache the collection, e.g Cache::put('test', $collection, 5)
  5. Try to read the the value from cache, e.g Cache::get('test')

In step 4, the corrupted string will be successfully saved into the database, without any exception or warning. In step 5, an exception will be thrown when trying to unserialize the corrupted string.

@mfn
Copy link
Contributor

mfn commented Sep 5, 2018

Did it every work for you? How did you define and create the cache table? Did you not get any error when storing it in PostgresSQL? Can you check the pgsql log?

You probably think the database column has "enough space" (you mentioned this in spatie/laravel-permission#857 ) but I think you're running in a PgSQL specific problem: you cannot always expect store "null bytes" in PgSQL => see the SO answer => https://stackoverflow.com/a/28816294/47573

PHPs serialization format (but also JSON as I've experienced this a few times already) do support and make use of "null bytes" and always cause troubles with PgSQL if you want to use the text or json* data types. I never used the bytea workaround mentioned in the answer (my fix was to remove the null bytes from JSON payloads) but I think you should give it a try.

@chescos
Copy link
Author

chescos commented Sep 5, 2018

@mfn No, it never worked for me before.

The cache table was created with a Laravel migration. I generated the schema with the command php artisan cache:table, so it's the default Laravel schema for the caching table. Here is the schema:

Schema::create('cache', function (Blueprint $table) {
    $table->string('key')->unique();
    $table->mediumText('value');
    $table->integer('expiration');
});

I've read the SO answer that you linked... so it seems like the issue is that PostgreSQL can't store "null bytes" in text coulumns and my value column is of the type text. And Laravel doesn't seem to support the bytea data type.

I see that this is an issue that is related to PostgreSQL, because it can't store "null bytes". But can this be fixed on the PHP side, so that there is no need to store null bytes when caching a collection with the PostgreSQL driver?

@staudenmeir
Copy link
Contributor

We could use Base64 encoding on PostgreSQL to fix this.

We can either encode all values (breaking change) or only the ones that contain null bytes (non-breaking change).

@mfn
Copy link
Contributor

mfn commented Sep 6, 2018

And Laravel doesn't seem to support the bytea data type.

It does, it's the binary type; see https://github.com/laravel/framework/blob/5.7/src/Illuminate/Database/Schema/Grammars/PostgresGrammar.php#L685-L688 .

We could use Base64 encoding on PostgreSQL to fix this.

My only gripe with this is that it increases the payload size and is a breaking change.

Seems safer to me to simply figure out a proper datatype for to use with PgSQL?

@staudenmeir
Copy link
Contributor

It's possible to implement Base64 encoding as a non-breaking change.

I think this would be the better solution because it doesn't require any actions by the user. If we use a binary column, we have to add a note/warning to the documentation and adjust the artisan cache:table command.

@mfn
Copy link
Contributor

mfn commented Sep 6, 2018

You don't know if the cache table isn't read by some other (non-laravel) application too; not totally out of the world IMHO.

@mfn
Copy link
Contributor

mfn commented Sep 6, 2018

To me any custom encoding feels like a hack.

The serialization format has null bytes and the database supports it, you just have to use the correct type.

IMHO educating is the proper "fix" here.

@chescos
Copy link
Author

chescos commented Sep 6, 2018

But if only null bytes would be Base64 encoded, that shouldn't affect any other applications that read from the table because currently null bytes can't be stored at all. So I doubt that anybody is storing and reading those corrupted strings since you can't use them for anything. And all other, non-corrupted values would be unaffected by this change. If this is possible to do, it sounds like a better solution to me.

About changing from text to bytea for the cache table... Is that without other side-effects and can the text column be altered to a byetea column for users who already have a cache table?

@staudenmeir
Copy link
Contributor

It's not possible to convert the column with $table->binary('value')->change() ("Datatype mismatch"), it requires raw SQL.

Using a binary column for PostgreSQL also requires changes to the DatabaseStore class. Both inserting and retrieving don't work with the current implementation.

So the DatabaseStore class has to be adjusted in both cases. But with Base64 encoding, the user doesn't have to change the migration/database. I think this is more relevant than the increased payload size.

@chescos
Copy link
Author

chescos commented Sep 10, 2018

@staudenmeir thanks for implementing this! <3

@eternalBlast
Copy link

eternalBlast commented Jun 19, 2019

Hi @staudenmeir , would you mind to share where to implement the Base64 you suggested?
I also used text type $table->text('value'); but still got that offset error. Thanks.

@staudenmeir
Copy link
Contributor

@eternalBlast What Laravel version are you using?

@eternalBlast
Copy link

eternalBlast commented Jun 20, 2019

@staudenmeir I am using Laravel 5.5.45 LTS.

@staudenmeir
Copy link
Contributor

This has only been fixed in Laravel 5.7. I'll submit a PR for 5.5.

@staudenmeir
Copy link
Contributor

@eternalBlast It will be fixed in the next release.

@eternalBlast
Copy link

eternalBlast commented Jun 21, 2019

Okay @staudenmeir. Thanks a lot 👍

@eternalBlast
Copy link

When will this fix available? @staudenmeir

@driesvints
Copy link
Member

@eternalBlast not sure yet. I'll check in with Taylor but he's on vacation right now.

@eternalBlast
Copy link

Okay @driesvints. Please notify when it is done. Thanks.

@staudenmeir
Copy link
Contributor

@eternalBlast Laravel 5.5.46 has been released.

@eternalBlast
Copy link

Thanks a lot.

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

5 participants