Merge pull request #567 from pcoutinho/fix-php81-serializable-depreca…

…tion fix: try using __serialize when obj implements \Serializable
rollbar · May 11, 2022 · 32fdf9f · 32fdf9f
2 parents 2086428 + 259c34e
commit 32fdf9f
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 10 deletions.
diff --git a/src/DataBuilder.php b/src/DataBuilder.php
@@ -1015,10 +1015,14 @@ protected function resolveCustomContent(mixed $custom): array
     {
         // This check is placed first because it should return a string|null, and we want to return an array.
         if ($custom instanceof \Serializable) {
-            trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED);
             // We don't return this value instead we run it through the rest of the checks. The same is true for the
             // next check.
-            $custom = $custom->serialize();
+            if (method_exists($custom, '__serialize')) {
+                $custom = $custom->__serialize();
+            } else {
+                trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED);
+                $custom = $custom->serialize();
+            }
         } else {
             if ($custom instanceof SerializerInterface) {
                 $custom = $custom->serialize();

diff --git a/src/Utilities.php b/src/Utilities.php
@@ -147,9 +147,13 @@ private static function serializeObject(
             $serialized = self::circularReferenceLabel($obj);
         } else {
             if ($obj instanceof \Serializable) {
-                trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED);
                 self::markSerialized($obj, $objectHashes);
-                $serialized = $obj->serialize();
+                if (method_exists($obj, '__serialize')) {
+                    $serialized = $obj->__serialize();
+                } else {
+                    trigger_error("Using the Serializable interface has been deprecated.", E_USER_DEPRECATED);
+                    $serialized = $obj->serialize();
+                }
             } elseif ($obj instanceof SerializerInterface) {
                 self::markSerialized($obj, $objectHashes);
                 $serialized = $obj->serialize();

diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php
@@ -12,6 +12,7 @@
 use Rollbar\Defaults;
 
 use Rollbar\TestHelpers\CustomSerializable;
+use Rollbar\TestHelpers\DeprecatedSerializable;
 use Rollbar\TestHelpers\Exceptions\SilentExceptionSampleRate;
 use Rollbar\TestHelpers\Exceptions\FiftyFiftyExceptionSampleRate;
 use Rollbar\TestHelpers\Exceptions\FiftyFityChildExceptionSampleRate;
@@ -531,7 +532,7 @@ public function testCustomObject()
         $this->assertSame($expectedCustom, $actualCustom);
     }
 
-    public function testCustomSerializable()
+    public function testDeprecatedSerializable()
     {
         $expectedCustom = array(
             "foo" => "bar",
@@ -540,7 +541,7 @@ public function testCustomSerializable()
         $config = new Config(array(
             "access_token" => $this->getTestAccessToken(),
             "environment" => $this->env,
-            "custom" => new CustomSerializable($expectedCustom),
+            "custom" => new DeprecatedSerializable($expectedCustom),
         ));
 
         // New error handler to make sure we get the deprecated notice
@@ -566,7 +567,43 @@ public function testCustomSerializable()
 
         $this->assertSame($expectedCustom, $actualCustom);
     }
-
+
+    public function testCustomSerializable()
+    {
+        $expectedCustom = array(
+            "foo" => "bar",
+            "fuzz" => "buzz"
+        );
+        $config = new Config(array(
+            "access_token" => $this->getTestAccessToken(),
+            "environment" => $this->env,
+            "custom" => new CustomSerializable($expectedCustom),
+        ));
+
+        // Make sure the deprecation notice is not sent if the object implements __serializable as it should
+        set_error_handler(function (
+            int $errno,
+            string $errstr,
+        ) : bool {
+            $this->assertStringNotContainsString("Serializable", $errstr);
+            $this->assertStringNotContainsString("deprecated", $errstr);
+            return true;
+        }, E_USER_DEPRECATED);
+
+        $result = $config->getDataBuilder()->makeData(
+            Level::INFO,
+            "Test message with custom data added dynamically.",
+            array(),
+        );
+
+        // Clear the handler, so it does not mess with other tests.
+        restore_error_handler();
+
+        $actualCustom = $result->getCustom();
+
+        $this->assertSame($expectedCustom, $actualCustom);
+    }
+
     public function testMaxItems()
     {
         $config = new Config(array(

diff --git a/tests/TestHelpers/CustomSerializable.php b/tests/TestHelpers/CustomSerializable.php
@@ -1,4 +1,6 @@
-<?php namespace Rollbar\TestHelpers;
+<?php
+
+namespace Rollbar\TestHelpers;
 
 class CustomSerializable implements \Serializable
 {
@@ -11,7 +13,7 @@ public function __construct($data)
 
     public function serialize()
     {
-        return $this->data;
+        throw new \Exception("Not implemented");
     }
 
     public function unserialize(string $data)
@@ -21,7 +23,7 @@ public function unserialize(string $data)
 
     public function __serialize(): array
     {
-        throw new \Exception("Not implemented");
+        return $this->data;
     }
 
     public function __unserialize(array $data): void

diff --git a/tests/TestHelpers/DeprecatedSerializable.php b/tests/TestHelpers/DeprecatedSerializable.php
@@ -0,0 +1,21 @@
+<?php namespace Rollbar\TestHelpers;
+
+class DeprecatedSerializable implements \Serializable
+{
+    public $data;
+
+    public function __construct($data)
+    {
+        $this->data = $data;
+    }
+
+    public function serialize()
+    {
+        return $this->data;
+    }
+
+    public function unserialize(string $data)
+    {
+        throw new \Exception("Not implemented");
+    }
+}
diff --git a/tests/UtilitiesTest.php b/tests/UtilitiesTest.php
@@ -1,9 +1,12 @@
 <?php namespace Rollbar;
 
+use Rollbar\Payload\Level;
+use Rollbar\TestHelpers\CustomSerializable;
 use Rollbar\TestHelpers\CycleCheck\ParentCycleCheck;
 use Rollbar\TestHelpers\CycleCheck\ChildCycleCheck;
 use Rollbar\TestHelpers\CycleCheck\ParentCycleCheckSerializable;
 use Rollbar\TestHelpers\CycleCheck\ChildCycleCheckSerializable;
+use Rollbar\TestHelpers\DeprecatedSerializable;
 
 class UtilitiesTest extends BaseRollbarTest
 {
@@ -173,4 +176,59 @@ public function testSerializeForRollbarNestingLevels()
         $this->assertArrayHasKey('three', $result['one']['two']);
         $this->assertArrayHasKey('four', $result['one']['two']['three']);
     }
+
+    public function testSerializationOfDeprecatedSerializable()
+    {
+        $data = ['foo' => 'bar'];
+
+        $obj = array(
+            "serializedObj" => new DeprecatedSerializable($data),
+        );
+        $objectHashes = array();
+
+        // Make sure the deprecation notice is sent if the object implements deprecated Serializable interface
+        set_error_handler(function (
+            int $errno,
+            string $errstr,
+        ) : bool {
+            $this->assertStringContainsString("Serializable", $errstr);
+            $this->assertStringContainsString("deprecated", $errstr);
+            return true;
+        }, E_USER_DEPRECATED);
+
+        $result = Utilities::serializeForRollbar($obj, null, $objectHashes);
+
+        // Clear the handler, so it does not mess with other tests.
+        restore_error_handler();
+
+        $this->assertEquals(['foo' => 'bar'], $result['serializedObj']);
+    }
+
+    public function testSerializationOfCustomSerializable()
+    {
+        $data = ['foo' => 'bar'];
+
+        $obj = array(
+            "serializedObj" => new CustomSerializable($data),
+        );
+        $objectHashes = array();
+
+        // Make sure the deprecation notice is NOT sent if the object implements Serializable but it's using
+        // __serialize and __unserialize properly
+        set_error_handler(function (
+            int $errno,
+            string $errstr,
+        ) : bool {
+            $this->assertStringNotContainsString("Serializable", $errstr);
+            $this->assertStringNotContainsString("deprecated", $errstr);
+            return true;
+        }, E_USER_DEPRECATED);
+
+        $result = Utilities::serializeForRollbar($obj, null, $objectHashes);
+
+        // Clear the handler, so it does not mess with other tests.
+        restore_error_handler();
+
+        $this->assertEquals(['foo' => 'bar'], $result['serializedObj']);
+    }
 }