Merge pull request #12 from Setono/better-unserialization

Better and safer unserialization

psalm-baseline.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<files psalm-version="4.24.0@06dd975cb55d36af80f242561738f16c5f58264f">
+  <file src="src/TagBag.php">
+    <UnusedVariable occurrences="2">
+      <code>$prevErrorHandler</code>
+      <code>$prevErrorHandler</code>
+    </UnusedVariable>
+  </file>
+</files>

psalm.xml

@@ -5,6 +5,7 @@
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
     errorLevel="1"
     phpVersion="8.1"
+    errorBaseline="psalm-baseline.xml"
 >
     <projectFiles>
         <directory name="src"/>

src/Exception/SerializationException.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\TagBag\Exception;
+
+use RuntimeException;
+
+final class SerializationException extends RuntimeException
+{
+    public static function emptyData(): self
+    {
+        return new self('The data given was empty');
+    }
+}

src/TagBag.php

@@ -4,12 +4,14 @@
 
 namespace Setono\TagBag;
 
+use InvalidArgumentException;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Setono\TagBag\Event\PreTagAddedEvent;
 use Setono\TagBag\Event\TagAddedEvent;
+use Setono\TagBag\Exception\SerializationException;
 use Setono\TagBag\Exception\StorageException;
 use Setono\TagBag\Exception\UnsupportedTagException;
 use Setono\TagBag\Generator\FingerprintGeneratorInterface;
@@ -19,7 +21,7 @@
 use Setono\TagBag\Tag\RenderedTag;
 use Setono\TagBag\Tag\TagInterface;
 use Throwable;
-use TypeError;
+use Webmozart\Assert\Assert;
 
 final class TagBag implements TagBagInterface, LoggerAwareInterface
 {
@@ -198,7 +200,7 @@ public function restore(): void
         if (null !== $data) {
             try {
                 $this->tags = $this->unserialize($data);
-            } catch (TypeError $e) {
+            } catch (SerializationException $e) {
                 $this->logger->error(sprintf('Exception thrown when trying to unserialize data. Error was: %s. Data was: %s', $e->getMessage(), $data));
             }
         }
@@ -245,15 +247,62 @@ private function findTagByFingerprint(string $fingerprint): ?array
     }
 
     /**
-     * @psalm-suppress MixedInferredReturnType
+     * @throws SerializationException if the data cannot be unserialized
      *
      * @return array<string, list<RenderedTag>>
+     *
+     * Most of this method is taken from here: https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/Messenger/Transport/Serialization/PhpSerializer.php
      */
     private function unserialize(string $data): array
     {
-        /** @psalm-suppress MixedReturnStatement */
-        return unserialize($data, [
-            'allowed_classes' => true,
-        ]);
+        if ('' === $data) {
+            throw SerializationException::emptyData();
+        }
+
+        $serializationException = new SerializationException(sprintf('Could not unserialize data: %s.', $data));
+        $prevUnserializeHandler = ini_set('unserialize_callback_func', self::class . '::handleUnserializeCallback');
+        /** @psalm-suppress MixedArgumentTypeCoercion */
+        $prevErrorHandler = set_error_handler(static function ($type, $msg, $file, $line, $context = []) use (&$prevErrorHandler, $serializationException) {
+            if (__FILE__ === $file) {
+                throw $serializationException;
+            }
+
+            /** @psalm-suppress MixedFunctionCall */
+            return $prevErrorHandler ? $prevErrorHandler($type, $msg, $file, $line, $context) : false;
+        });
+
+        try {
+            /** @var array<string, list<RenderedTag>> $result */
+            $result = unserialize($data, [
+                'allowed_classes' => [RenderedTag::class],
+            ]);
+            /** @psalm-suppress RedundantConditionGivenDocblockType */
+            Assert::isArray($result);
+            foreach ($result as $section => $tags) {
+                /** @psalm-suppress RedundantConditionGivenDocblockType */
+                Assert::string($section);
+
+                /** @psalm-suppress RedundantConditionGivenDocblockType */
+                Assert::isArray($tags);
+
+                /** @psalm-suppress DocblockTypeContradiction */
+                Assert::allIsInstanceOf($tags, RenderedTag::class);
+            }
+        } catch (InvalidArgumentException $e) {
+            throw new SerializationException(sprintf('The unserialized data was incorrect. Here is the original data: %s', $data));
+        } finally {
+            restore_error_handler();
+            ini_set('unserialize_callback_func', $prevUnserializeHandler);
+        }
+
+        return $result;
+    }
+
+    /**
+     * @internal
+     */
+    public static function handleUnserializeCallback(string $class): void
+    {
+        throw new SerializationException(sprintf('Message class "%s" not found during decoding.', $class));
     }
 }

tests/TagBagTest.php

@@ -269,6 +269,33 @@ public function render(TagInterface $tag): string
         self::assertTrue($this->logger->hasMessageMatching('/^Cannot render tag$/'));
     }
 
+    /**
+     * @test
+     */
+    public function it_catches_unserialize_error(): void
+    {
+        $storage = new class() implements StorageInterface {
+            public function store(string $data): void
+            {
+            }
+
+            public function restore(): ?string
+            {
+                return serialize([
+                    new NotARenderedTag(),
+                ]);
+            }
+
+            public function remove(): void
+            {
+            }
+        };
+        $tagBag = $this->getTagBag($storage);
+        $tagBag->restore();
+
+        self::assertTrue($this->logger->hasMessageMatching('/^Exception thrown when trying to unserialize data/'));
+    }
+
     private function getTag(
         string $content = 'content',
         string $section = null,
@@ -340,3 +367,7 @@ public function hasMessageMatching(string $regexp): bool
         return false;
     }
 }
+
+final class NotARenderedTag
+{
+}