From 3c1eda00d7e9879558397fe8c8cfaea24b0de4e0 Mon Sep 17 00:00:00 2001
From: Benoit Galati <benoit.galati@gmail.com>
Date: Fri, 22 Dec 2023 10:14:06 +0100
Subject: [PATCH] [Messenger] PhpSerializer: TypeError should throw
 MessageDecodingFailedException

Actually, the fix should handle more cases than only TypeError.
---
 Tests/Fixtures/DummyMessageTyped.php          | 18 ++++++++++
 .../Serialization/PhpSerializerTest.php       | 33 ++++++++++++++-----
 Transport/Serialization/PhpSerializer.php     | 22 ++++++-------
 3 files changed, 54 insertions(+), 19 deletions(-)
 create mode 100644 Tests/Fixtures/DummyMessageTyped.php

diff --git a/Tests/Fixtures/DummyMessageTyped.php b/Tests/Fixtures/DummyMessageTyped.php
new file mode 100644
index 00000000..53144040
--- /dev/null
+++ b/Tests/Fixtures/DummyMessageTyped.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace Symfony\Component\Messenger\Tests\Fixtures;
+
+class DummyMessageTyped implements DummyMessageInterface
+{
+    private string $message;
+
+    public function __construct(string $message)
+    {
+        $this->message = $message;
+    }
+
+    public function getMessage(): string
+    {
+        return $this->message;
+    }
+}
diff --git a/Tests/Transport/Serialization/PhpSerializerTest.php b/Tests/Transport/Serialization/PhpSerializerTest.php
index b31914a9..07b82519 100644
--- a/Tests/Transport/Serialization/PhpSerializerTest.php
+++ b/Tests/Transport/Serialization/PhpSerializerTest.php
@@ -16,6 +16,7 @@
 use Symfony\Component\Messenger\Exception\MessageDecodingFailedException;
 use Symfony\Component\Messenger\Stamp\NonSendableStampInterface;
 use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
+use Symfony\Component\Messenger\Tests\Fixtures\DummyMessageTyped;
 use Symfony\Component\Messenger\Transport\Serialization\PhpSerializer;
 
 class PhpSerializerTest extends TestCase
@@ -33,21 +34,21 @@ public function testEncodedIsDecodable()
 
     public function testDecodingFailsWithMissingBodyKey()
     {
+        $serializer = new PhpSerializer();
+
         $this->expectException(MessageDecodingFailedException::class);
         $this->expectExceptionMessage('Encoded envelope should have at least a "body", or maybe you should implement your own serializer');
 
-        $serializer = new PhpSerializer();
-
         $serializer->decode([]);
     }
 
     public function testDecodingFailsWithBadFormat()
     {
+        $serializer = new PhpSerializer();
+
         $this->expectException(MessageDecodingFailedException::class);
         $this->expectExceptionMessageMatches('/Could not decode/');
 
-        $serializer = new PhpSerializer();
-
         $serializer->decode([
             'body' => '{"message": "bar"}',
         ]);
@@ -55,11 +56,11 @@ public function testDecodingFailsWithBadFormat()
 
     public function testDecodingFailsWithBadBase64Body()
     {
+        $serializer = new PhpSerializer();
+
         $this->expectException(MessageDecodingFailedException::class);
         $this->expectExceptionMessageMatches('/Could not decode/');
 
-        $serializer = new PhpSerializer();
-
         $serializer->decode([
             'body' => 'x',
         ]);
@@ -67,11 +68,11 @@ public function testDecodingFailsWithBadBase64Body()
 
     public function testDecodingFailsWithBadClass()
     {
+        $serializer = new PhpSerializer();
+
         $this->expectException(MessageDecodingFailedException::class);
         $this->expectExceptionMessageMatches('/class "ReceivedSt0mp" not found/');
 
-        $serializer = new PhpSerializer();
-
         $serializer->decode([
             'body' => 'O:13:"ReceivedSt0mp":0:{}',
         ]);
@@ -99,6 +100,22 @@ public function testNonUtf8IsBase64Encoded()
         $this->assertTrue((bool) preg_match('//u', $encoded['body']), 'Encodes non-UTF8 payloads');
         $this->assertEquals($envelope, $serializer->decode($encoded));
     }
+
+    /**
+     * @requires PHP 7.4
+     */
+    public function testDecodingFailsForPropertyTypeMismatch()
+    {
+        $serializer = new PhpSerializer();
+        $encodedEnvelope = $serializer->encode(new Envelope(new DummyMessageTyped('true')));
+        // Simulate a change of property type in the code base
+        $encodedEnvelope['body'] = str_replace('s:4:\"true\"', 'b:1', $encodedEnvelope['body']);
+
+        $this->expectException(MessageDecodingFailedException::class);
+        $this->expectExceptionMessageMatches('/Could not decode/');
+
+        $serializer->decode($encodedEnvelope);
+    }
 }
 
 class DummyPhpSerializerNonSendableStamp implements NonSendableStampInterface
diff --git a/Transport/Serialization/PhpSerializer.php b/Transport/Serialization/PhpSerializer.php
index e6332b3c..17db0296 100644
--- a/Transport/Serialization/PhpSerializer.php
+++ b/Transport/Serialization/PhpSerializer.php
@@ -20,9 +20,6 @@
  */
 class PhpSerializer implements SerializerInterface
 {
-    /**
-     * {@inheritdoc}
-     */
     public function decode(array $encodedEnvelope): Envelope
     {
         if (empty($encodedEnvelope['body'])) {
@@ -38,9 +35,6 @@ public function decode(array $encodedEnvelope): Envelope
         return $this->safelyUnserialize($serializeEnvelope);
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function encode(Envelope $envelope): array
     {
         $envelope = $envelope->withoutStampsOfType(NonSendableStampInterface::class);
@@ -62,24 +56,30 @@ private function safelyUnserialize(string $contents)
             throw new MessageDecodingFailedException('Could not decode an empty message using PHP serialization.');
         }
 
-        $signalingException = new MessageDecodingFailedException(sprintf('Could not decode message using PHP serialization: %s.', $contents));
         $prevUnserializeHandler = ini_set('unserialize_callback_func', self::class.'::handleUnserializeCallback');
-        $prevErrorHandler = set_error_handler(function ($type, $msg, $file, $line, $context = []) use (&$prevErrorHandler, $signalingException) {
+        $prevErrorHandler = set_error_handler(function ($type, $msg, $file, $line, $context = []) use (&$prevErrorHandler) {
             if (__FILE__ === $file) {
-                throw $signalingException;
+                throw new \ErrorException($msg, 0, $type, $file, $line);
             }
 
             return $prevErrorHandler ? $prevErrorHandler($type, $msg, $file, $line, $context) : false;
         });
 
         try {
-            $meta = unserialize($contents);
+            /** @var Envelope */
+            $envelope = unserialize($contents);
+        } catch (\Throwable $e) {
+            if ($e instanceof MessageDecodingFailedException) {
+                throw $e;
+            }
+
+            throw new MessageDecodingFailedException('Could not decode Envelope: '.$e->getMessage(), 0, $e);
         } finally {
             restore_error_handler();
             ini_set('unserialize_callback_func', $prevUnserializeHandler);
         }
 
-        return $meta;
+        return $envelope;
     }
 
     /**