Skip to content

Support encryption, avoid double __unserialize call #62

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

Merged
merged 2 commits into from
Aug 13, 2022

Conversation

dshafik
Copy link
Contributor

@dshafik dshafik commented Jul 29, 2022

Laravel uses the magic method __unserialize() in the SerializesModels trait, which does a bunch of work to rehydrate models attached to jobs. This gets called every time the job is unserialized. The addition of an unserialize() call in the constructor to get the queue property causes it to run twice because of the original call to unserialize() is in \Illuminate\Queue\CallQueuedHandler->getCommand().

CallQueuedHandler->getCommand() also implements support for encrypted command payloads.

This change brings over the encryption support logic from CallQueuedHandler->getCommand() and passed ['allowed_classes' => false] to the unserialize() call. This will make it not hydrate to the original job object, but instead to an instance of __PHP_Incomplete_Class avoiding the unnecessary call to __unserialize() with this unserialize() call. We then cast to an (array) to access the queue property without issue.

@dshafik
Copy link
Contributor Author

dshafik commented Jul 29, 2022

I actually see there is another call to unserialize() in TaskHandler::handleTask(), I'll submit an updated PR with a fix for that one too shortly

@marickvantuil
Copy link
Member

Thanks for the PR! Will you be updating it to fix the other unserialize() call? When it's all finished, I can merge it.

Laravel uses the magic method `__unserialize()` in the `SerializesModels` trait, which does a bunch of work to rehydrate models attached to jobs. This gets called every time the job is unserialized. The addition of an `unserialize()` call in the constructor to get the `queue` property causes it to run twice because of the original call to `unserialize()` is in `\Illuminate\Queue\CallQueuedHandler->getCommand()`.

`CallQueuedHandler->getCommand()` also implements support for encrypted command payloads.

This change brings over the encryption support logic from `CallQueuedHandler->getCommand()` and passed `['allowed_classes' => false]` to the `unserialize()` call. This will make it _not_ hydrate to the original job object, but instead to an instance of `__PHP_Incomplete_Class` avoiding the unnecessary call to `__unserialize()` with this `unserialize()` call. We then cast to an `(array)` to access the `queue` property without issue.
@dshafik
Copy link
Contributor Author

dshafik commented Aug 9, 2022

@marickvantuil Updated the PR to avoid the unserialize() in all places

@marickvantuil marickvantuil merged commit 39646cc into stackkit:master Aug 13, 2022
@marickvantuil
Copy link
Member

Thanks! Released a new version v3.2.0

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.

2 participants