From 3b27e06437b22dc9236131a6999333bf7b83e2ce Mon Sep 17 00:00:00 2001
From: David Badura <david.badura@patchlevel.de>
Date: Tue, 6 Feb 2024 18:54:34 +0100
Subject: [PATCH] improve message serializer

---
 src/EventBus/Serializer/DeserializeFailed.php | 30 +++++++++++++++++++
 .../Serializer/PhpNativeMessageSerializer.php | 17 +++++++++--
 src/Outbox/DoctrineOutboxStore.php            | 19 ++++++++++--
 src/Outbox/OutboxHeaderIssue.php              | 23 ++++++++++++++
 .../PhpNativeMessageSerializerTest.php        | 19 ++++++++++++
 5 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 src/EventBus/Serializer/DeserializeFailed.php
 create mode 100644 src/Outbox/OutboxHeaderIssue.php

diff --git a/src/EventBus/Serializer/DeserializeFailed.php b/src/EventBus/Serializer/DeserializeFailed.php
new file mode 100644
index 000000000..b34f0a917
--- /dev/null
+++ b/src/EventBus/Serializer/DeserializeFailed.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\EventSourcing\EventBus\Serializer;
+
+use Patchlevel\EventSourcing\EventBus\Message;
+use RuntimeException;
+
+use function get_debug_type;
+use function sprintf;
+
+final class DeserializeFailed extends RuntimeException
+{
+    public static function decodeFailed(): self
+    {
+        return new self('Error while decoding message');
+    }
+
+    public static function invalidMessage(mixed $value): self
+    {
+        return new self(
+            sprintf(
+                'Value should me an instance of %s, but is %s',
+                Message::class,
+                get_debug_type($value),
+            ),
+        );
+    }
+}
diff --git a/src/EventBus/Serializer/PhpNativeMessageSerializer.php b/src/EventBus/Serializer/PhpNativeMessageSerializer.php
index e36a727e6..5c9663cdc 100644
--- a/src/EventBus/Serializer/PhpNativeMessageSerializer.php
+++ b/src/EventBus/Serializer/PhpNativeMessageSerializer.php
@@ -8,6 +8,7 @@
 
 use function base64_decode;
 use function base64_encode;
+use function is_string;
 use function serialize;
 use function unserialize;
 
@@ -20,9 +21,21 @@ public function serialize(Message $message): string
 
     public function deserialize(string $content): Message
     {
-        return unserialize(
-            base64_decode($content),
+        $decodedString = base64_decode($content, true);
+
+        if (!is_string($decodedString)) {
+            throw DeserializeFailed::decodeFailed();
+        }
+
+        $message = unserialize(
+            $decodedString,
             ['allowed_classes' => true],
         );
+
+        if (!$message instanceof Message) {
+            throw DeserializeFailed::invalidMessage($message);
+        }
+
+        return $message;
     }
 }
diff --git a/src/Outbox/DoctrineOutboxStore.php b/src/Outbox/DoctrineOutboxStore.php
index 593ed549f..db0fbc0ea 100644
--- a/src/Outbox/DoctrineOutboxStore.php
+++ b/src/Outbox/DoctrineOutboxStore.php
@@ -7,6 +7,7 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Types\Types;
+use Patchlevel\EventSourcing\EventBus\HeaderNotFound;
 use Patchlevel\EventSourcing\EventBus\Message;
 use Patchlevel\EventSourcing\EventBus\Serializer\MessageSerializer;
 use Patchlevel\EventSourcing\Schema\SchemaConfigurator;
@@ -71,8 +72,7 @@ public function markOutboxMessageConsumed(Message ...$messages): void
         $this->connection->transactional(
             function (Connection $connection) use ($messages): void {
                 foreach ($messages as $message) {
-                    $id = $message->customHeader(self::HEADER_OUTBOX_IDENTIFIER);
-
+                    $id = $this->extractId($message);
                     $connection->delete($this->outboxTable, ['id' => $id]);
                 }
             },
@@ -108,4 +108,19 @@ public function configureSchema(Schema $schema, Connection $connection): void
 
         $table->setPrimaryKey(['id']);
     }
+
+    private function extractId(Message $message): int
+    {
+        try {
+            $value = $message->customHeader(self::HEADER_OUTBOX_IDENTIFIER);
+        } catch (HeaderNotFound) {
+            throw OutboxHeaderIssue::missingHeader(self::HEADER_OUTBOX_IDENTIFIER);
+        }
+
+        if (!is_int($value)) {
+            throw OutboxHeaderIssue::invalidHeaderType($value);
+        }
+
+        return $value;
+    }
 }
diff --git a/src/Outbox/OutboxHeaderIssue.php b/src/Outbox/OutboxHeaderIssue.php
new file mode 100644
index 000000000..817e1d8c2
--- /dev/null
+++ b/src/Outbox/OutboxHeaderIssue.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\EventSourcing\Outbox;
+
+use RuntimeException;
+
+use function gettype;
+use function sprintf;
+
+final class OutboxHeaderIssue extends RuntimeException
+{
+    public static function missingHeader(string $header): self
+    {
+        return new self(sprintf('missing header "%s"', $header));
+    }
+
+    public static function invalidHeaderType(mixed $value): self
+    {
+        return new self(sprintf('Invalid header given: need type "int" got "%s"', gettype($value)));
+    }
+}
diff --git a/tests/Unit/EventBus/Serializer/PhpNativeMessageSerializerTest.php b/tests/Unit/EventBus/Serializer/PhpNativeMessageSerializerTest.php
index 19c421ab9..f26a6f031 100644
--- a/tests/Unit/EventBus/Serializer/PhpNativeMessageSerializerTest.php
+++ b/tests/Unit/EventBus/Serializer/PhpNativeMessageSerializerTest.php
@@ -6,6 +6,7 @@
 
 use DateTimeImmutable;
 use Patchlevel\EventSourcing\EventBus\Message;
+use Patchlevel\EventSourcing\EventBus\Serializer\DeserializeFailed;
 use Patchlevel\EventSourcing\EventBus\Serializer\PhpNativeMessageSerializer;
 use Patchlevel\EventSourcing\Tests\Unit\Fixture\ProfileId;
 use Patchlevel\EventSourcing\Tests\Unit\Fixture\ProfileVisited;
@@ -51,6 +52,24 @@ public function testDeserialize(): void
         ], $message->headers());
     }
 
+    public function testDeserializeEncodeFailed(): void
+    {
+        $this->expectException(DeserializeFailed::class);
+
+        $nativeSerializer = new PhpNativeMessageSerializer();
+
+        $nativeSerializer->deserialize('FOO');
+    }
+
+    public function testDeserializeNotAMessage(): void
+    {
+        $this->expectException(DeserializeFailed::class);
+
+        $nativeSerializer = new PhpNativeMessageSerializer();
+
+        $nativeSerializer->deserialize('Tzo4OiJzdGRDbGFzcyI6MDp7fQ==');
+    }
+
     public function testEquals(): void
     {
         $event = new ProfileVisited(