laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.36k stars 10.97k forks source link

Job implementing Serializable interface fails with "The payload is invalid" exception #44539

Closed igor-krein closed 2 years ago

igor-krein commented 2 years ago

Description:

If a job class implements PHP's Serializable interface, and doesn't implement \Illuminate\Contracts\Queue\ShouldBeEncrypted interface, the job's payload unserialization fails.

[2022-10-10 08:57:29] local.ERROR: The payload is invalid. {"exception":"[object] (Illuminate\\Contracts\\Encryption\\DecryptException(code: 0): The payload is invalid. at /[path]/vendor/laravel/framework/src/Illuminate/Encryption/Encrypter.php:214)
[stacktrace]
#0 /[path]/vendor/laravel/framework/src/Illuminate/Encryption/Encrypter.php(151): Illuminate\\Encryption\\Encrypter->getJsonPayload(NULL)
#1 /[path]/vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(101): Illuminate\\Encryption\\Encrypter->decrypt('C:21:\"App\\\\Jobs\\\\...')
#2 /[path]/vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(60): Illuminate\\Queue\\CallQueuedHandler->getCommand(Array)
#3 /[path]/vendor/laravel/framework/src/Illuminate/Queue/Jobs/Job.php(98): Illuminate\\Queue\\CallQueuedHandler->call(Object(Illuminate\\Queue\\Jobs\\DatabaseJob), Array)
#4 /[path]/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(425): Illuminate\\Queue\\Jobs\\Job->fire()
#5 /[path]/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(375): Illuminate\\Queue\\Worker->process('database', Object(Illuminate\\Queue\\Jobs\\DatabaseJob), Object(Illuminate\\Queue\\WorkerOptions))
#6 /[path]/vendor/laravel/framework/src/Illuminate/Queue/Worker.php(326): Illuminate\\Queue\\Worker->runJob(Object(Illuminate\\Queue\\Jobs\\DatabaseJob), 'database', Object(Illuminate\\Queue\\WorkerOptions))
#7 /[path]/vendor/laravel/framework/src/Illuminate/Queue/Console/WorkCommand.php(150): Illuminate\\Queue\\Worker->runNextJob('database', 'batch', Object(Illuminate\\Queue\\WorkerOptions))

Notes

I believe, the problem exists since Laravel 8; it's just I am upgrading from Laravel 7 to 9. Also, I suspect the bug is pretty rare.

The problematic piece of code is located at line 96 of Illuminate/Queue/CallQueuedHandler.php, where it is being determined whether the job is encrypted or not.

    protected function getCommand(array $data)
    {
        if (str_starts_with($data['command'], 'O:')) {    // <<<<< line 96 !!!!!
            return unserialize($data['command']);
        }

        if ($this->container->bound(Encrypter::class)) {
            return unserialize($this->container[Encrypter::class]->decrypt($data['command']));
        }

        throw new RuntimeException('Unable to extract job payload.');
    }

Condition if (str_starts_with($data['command'], 'O:')) works for regular classes, but for those that implement Serializable interface, serialize function generates strings started with 'C:'.

See: https://www.phpinternalsbook.com/php5/classes_objects/serialization.html

Steps To Reproduce:

  1. Create job class
namespace App\Jobs;

use Illuminate\Contracts\Queue\ShouldQueue;

class Job implements ShouldQueue, \Serializable
{
    public function serialize()
    {
        $jobData = [
            'queue' => $this->queue,
            // add more data
        ];
        return serialize($jobData);
    }

    public function unserialize($data)
    {
        $jobData = unserialize($data);
        $this->queue = $jobData['queue'];
        // fill more data
    }

    // implement ShouldQueue...
}
  1. dispatch job

  2. run queue worker (php artisan queue:work)

morloderex commented 2 years ago

@igor-krein This is a php bug and not a specific laravel bug.

The \Serializable interface shouldn't be used anymore sense it has been superseded by __serialize and __unserialize magic methods.

And as I can see you are using PHP 8.1 which means you should see deprecation warnings in your log files.

igor-krein commented 2 years ago

@morloderex Thanks, wasn't aware of that (we are upgrading PHP as well along with Laravel; also, I didn't see any warnings yet, which is strange).

I'll try to get rid of \Serializable and then notify here separately.

igor-krein commented 2 years ago

@morloderex Just checked, and looks like this is the solution I need.

Think, I can close the issue now. Thanks again.