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

[Postgresql] Serialization/Unserialization error while using non-public fields on jobs #42

Closed
abdullahcanakci opened this issue Mar 29, 2022 · 9 comments · Fixed by #51

Comments

@abdullahcanakci
Copy link

abdullahcanakci commented Mar 29, 2022

Hi, I recently started using this package but stumbled upon an issue around non-public fields on jobs.

If a job has a non-public field PHP \unserialize function places a \0\0 value in front of it. When this serialized string is transferred into DB and it is accepted as a null-terminated string and as expected half of it is missing. So when this package tries to unserialize the string, it does not work.

Currently, I made them public to overcome the issue but another solution might be possible. I thought about SerializesModels but, trait usage can not be checked against. So don't really know about it.

Currently, I don't have the repro but if requested I can whip something up.

laravel/framework v8.83.4
ksassnowski/venture 3.5.0
php8.1
Postgres 12

@ksassnowski
Copy link
Owner

A repository demonstrating this problem would be helpful as I’ve definitely used jobs with non-public fields before without any issues.

@abdullahcanakci
Copy link
Author

Hi again,
I recreated the problem in this repo . I have included a hastily put together dockerfile if needed, some manual tweaking is required but everything can be found on the readme.

I have tested both on postgres12, on my host machine, and postgres13 inside the docker and the issue exists for both of them.

If you need anything, I am here to help.

Thanks 👍

@ksassnowski
Copy link
Owner

Thanks for the repo. I'll try and have a look at it soon 👍

@ksassnowski
Copy link
Owner

So my first hunch is that it’s related to this paragraph from PHP's serialize function.

Note that this is a binary string which may include null bytes, and needs to be stored and handled as such. For example, serialize() output should generally be stored in a BLOB field in a database, rather than a CHAR or TEXT field.

I'll try adjusting the migration to create a BLOB field instead of TEXT and see if it changes anything. This might also be worth adding to our CI pipeline.

@ksassnowski ksassnowski changed the title Serialization/Unserialization error while using non-public fields on jobs [Postgresql] Serialization/Unserialization error while using non-public fields on jobs Apr 13, 2022
@ksassnowski
Copy link
Owner

FYI, Laravel itself had the same problem and the way they fixed it was to check if the Postgres driver was used and then base64_encode the serialized string before storing it. We might as well do the same thing. Here’s the relevant PR https://github.com/laravel/framework/pull/36081/files

@stevebauman Thoughts?

@ksassnowski
Copy link
Owner

From a quick test, using the custom serialize and unserialize methods from the PR seems to fix the issue. Now the tests in the provided repo pass. I'll try and whip up a PR for this soon.

@ksassnowski
Copy link
Owner

@abdullahcanakci Can you check out #51 and see if that fixes your error? You can require this branch by using "sassnowski/venture": "dev-base64_encode_psql" in you composer.json.

@abdullahcanakci
Copy link
Author

Thanks so much, works as expected

@ksassnowski
Copy link
Owner

Thanks for reporting and for providing a reproduction repo :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants