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.7] Fix database cache on PostgreSQL #25530

Merged
merged 1 commit into from
Sep 10, 2018
Merged

[5.7] Fix database cache on PostgreSQL #25530

merged 1 commit into from
Sep 10, 2018

Conversation

staudenmeir
Copy link
Contributor

Storing serialized objects in the database cache doesn't work on PostgreSQL:

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

Cache::put('user', new User(), 1);
Cache::get('user');

Retrieving the value throws an exception:

unserialize(): Error at offset 21 of 25 bytes

The database cache uses serialize() to serialize the value. For objects, the serialization contains null bytes. PostgreSQL can't store null bytes in a text column, truncates the string (O:8:"App\User":27:{s:11:") and thereby breaks the unserialization.

The proposed solution is using Base64 encoding if the serialized value contains a null byte. When the value is retrieved, we can detect the Base64 encoding by looking for : or ;. A Base64-encoded string can't contain this characters; a serialized string always contains at least one of them.

I used this to find the possible data type serializations:

dd(
    serialize(null),
    serialize(true),
    serialize(1),
    serialize(1.0),
    serialize('a'),
    serialize([]),
    serialize((object) [])
);

This would only be a breaking change if someone was actually using the corrupted serialization – which makes no sense.

An alternative approach is storing the value in a binary column (bytea). This would also require changes to the DatabaseStore and the user would have to adjust the migration. With the Base64 encoding, the user doesn't have to worry about the used database. The only downside is the increased payload size.

Fixes #25466.

@taylorotwell taylorotwell merged commit b2fa4e7 into laravel:5.7 Sep 10, 2018
@staudenmeir staudenmeir deleted the cache-postgresql branch September 10, 2018 14:53
@tillkruss
Copy link
Contributor

@staudenmeir: How do you feel about encoding all cache values for Postgres, instead of checking for characters?

@staudenmeir
Copy link
Contributor Author

@tillkruss In order to simplify the code? I don't think it's worth the increased payload size. And we would still require Str::contains($value, [':', ';']) to make it a non-breaking change.

@MyBestPro-Stephane
Copy link

I think I have the same issue with SQLserver...

@staudenmeir
Copy link
Contributor Author

@MyBestPro-Stephane Please submit a new issue.

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.

4 participants