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] Queued Closures #25777

Merged
merged 6 commits into from
Sep 26, 2018
Merged

[5.7] Queued Closures #25777

merged 6 commits into from
Sep 26, 2018

Conversation

taylorotwell
Copy link
Member

This reintroduces a feature that was previously present in early versions of Laravel queues. However, there have been improvements to serializable closure libraries in the meantime which allows for better security and better handling of Eloquent models and collections. Namely, signing closures with a hash to prevent modification and arbitrary code execution, as well as transforming and resolving models and collections to ModelIdentifier instances.

This reintroduces a feature that was previously present in early versions of Laravel queues. However, there have been improvements to serializable closure libraries in the meantime which allows for better security and better handling of Eloquent models and collections. Namely, signing closures with a hash to prevent modification and arbitrary code execution, as well as transforming and resolving models and collections to ModelIdentifier instances.
@m1guelpf
Copy link
Contributor

@taylorotwell Any chances of serializing closure routes now that you added a library for serializing closures?

@taylorotwell
Copy link
Member Author

taylorotwell commented Sep 25, 2018

People would need to do measurements to see if the cost of unserializing all the Closures outweighs the benefits of the caching.

That's probably for another thread. Would prefer to keep this limited only to feedback on the PR.

@GrahamCampbell GrahamCampbell changed the title Queued Closures [5.7] Queued Closures Sep 25, 2018
composer.json Show resolved Hide resolved
@GrahamCampbell
Copy link
Member

Namely, signing closures with a hash to prevent modification and arbitrary code execution

I'm pretty sure the previous implementation also supported this? :P

*/
protected function registerOpisSecurityKey()
{
if (Str::startsWith($key = config('app.key'), 'base64:')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config function exists in the foundation namespace, so queue shouldn't really depend on it if we want to continue to allow the queue package to be required by arbitrary software projects. Also, the duplication of the base64 decoding logic is a bit gross.

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