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

Simple example not working #52

Closed
shrimpwagon opened this issue Nov 30, 2018 · 7 comments
Closed

Simple example not working #52

shrimpwagon opened this issue Nov 30, 2018 · 7 comments

Comments

@shrimpwagon
Copy link

[2018-11-30 16:47:58] local.DEBUG: The serialized closure is signed. Make sure you use a security provider for both serialization and unserialization.

#0 [internal function]: Opis\Closure\SerializableClosure->unserialize('@{"closure":"a:...')
#1 /home/vagrant/code/vendor/opis/closure/functions.php(34): unserialize('C:32:"Opis\Clos...')
#2 /home/vagrant/code/vendor/spatie/async/src/Runtime/ParentRuntime.php(92): Opis\Closure\unserialize('C:32:"Opis\Clos...')
#3 /home/vagrant/code/vendor/spatie/async/src/Runtime/ChildRuntime.php(23): Spatie\Async\Runtime\ParentRuntime::decodeTask('QzozMjoiT3Bpc1x...')
#4 {main}

    $pool = \Spatie\Async\Pool::create();

    $json = '';



    $pool->add(function() {

        logger('rates test 1');

        return '{"id":"1","label":"test 1","cost":100}';

    })->then(function($out) use(&$json) {

        logger('rates test 2');

        if( $json ) $json .= ',';

        $json .= $out;

    })->catch(function(Throwable $e) {

        logger($e->getMessage());

    });


    $pool->add(function() {

        return '{"id":"2","label":"test 2","cost":200}';

    })->then(function($out) use(&$json) {

        if( $json ) $json .= ',';

        $json .= $out;

    })->catch(function(Throwable $e) {

        //

    });


    $pool->add(function() {

        return '{"id":"3","label":"test 3","cost":300}';

    })->then(function($out) use(&$json) {

        if( $json ) $json .= ',';

        $json .= $out;

    })->catch(function(Throwable $e) {

        //

    });


    $pool->wait();

    logger('rates test 3');

    echo '{"rates":[' . $json . ']}';

The $json variable is blank

@shrimpwagon
Copy link
Author

shrimpwagon commented Nov 30, 2018

I sort of figured out the root cause but I'm unsure of a solid solution.

This code is in a Laravel environment. Laravel sets the secret key at some point.

file: vendor/laravel/framework/src/Illuminate/Queue/QueueServiceProvider.php
line: 228
SerializableClosure::setSecretKey($key);

In Opis, the secret key sets a security provider static property:

public static function setSecretKey($secret)
{
    if(static::$securityProvider === null){
        static::$securityProvider = new SecurityProvider($secret);
    }
}

Adding this line at the beginning of my initial code post seems to fix things:

\Opis\Closure\SerializableClosure::removeSecurityProvider();

This function nulls out that static variable before closure is created.

When the task is unserialized that static property is not set by Laravel.

I believe you need to somehow pull the security provider:

/**
 * @return null|ISecurityProvider
 */
public static function getSecurityProvider()
{
    return static::$securityProvider;
}

And pass this into the child process as well so it can be unserialized with that same security provider.

@shrimpwagon
Copy link
Author

This is the problem with Laravel:

vendor/laravel/framework/src/Illuminate/Queue/QueueServiceProvider.php:

/**
 * Configure Opis Closure signing for security.
 *
 * @return void
 */
protected function registerOpisSecurityKey()
{
    if (Str::startsWith($key = $this->app['config']->get('app.key'), 'base64:')) {
        $key = base64_decode(substr($key, 7));
    }

    SerializableClosure::setSecretKey($key);
}

@shrimpwagon
Copy link
Author

Adding this to the beginning of my original code seems to solve the issue:

\Opis\Closure\SerializableClosure::removeSecurityProvider();

@msarca
Copy link

msarca commented Nov 30, 2018

The solution to this issue is pretty simple

public static function encodeTask($task): string
{
    if ($task instanceof Closure) {
        $task = new SerializableClosure($task);
    }

    // Check if a security provider was set
    if (null !== $securityProvider = SerializableClosure::getSecurityProvider()) {
        // Don't worry about it, our closure will be executed locally
        SerializableClosure::removeSecurityProvider();
    }
    
    $task = base64_encode(serialize($task));
    
    // Restore the security provider, if any
    if ($securityProvider !== null) {
        SerializableClosure::addSecurityProvider($securityProvider);
    }
    
    return $task;
}

@shrimpwagon
Copy link
Author

Created pull request:
#53

@brendt
Copy link
Contributor

brendt commented Dec 3, 2018

I'll close this issue, as the solution seems to be clear: #52 (comment)

@brendt brendt closed this as completed Dec 3, 2018
@arondeparon
Copy link

arondeparon commented Mar 25, 2019

We experienced this issue as well, but did not feel comfortable with unsetting the security provider, so we did some digging.

The actual problem seems to be caused by the fact that Illuminate\Queue\QueueServiceProvider implements DeferrableProvider.

This same class is also responsible for registering the Opis Security key when it is resolved from the container.

The problem, however, is that the queue will (in most cases) not be resolved from the container when unserializing an object, causing this above error.

Solution (hotfix): we temporarily solved this issue by adding this method to AppServiceProvider and to call it. The permanent solution would be to register the secret key outside of the Queue provider to make sure it is not deferred.

    protected function registerOpisSecurityKey()
    {
        if (Str::startsWith($key = $this->app['config']->get('app.key'), 'base64:')) {
            $key = base64_decode(substr($key, 7));
        }

        SerializableClosure::setSecretKey($key);
    }

Bonus: An extra complication that we experienced was the fact that things did work when setting APP_DEBUG to true. This actually helped us to find the problem: in our case, it was Laravel Debugbar that called the mail container, which in turn calls (and initialized) the queue container.

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

4 participants