Review and refactoring of #37

- Updates license docblocks of changed files to reflect current standards. - Use a ternary to get the return value of the `unserialize()` operation (simpler to read than if/else block). - No fluent operations in new classes. - Review all docblocks for formatting, grammar. - Use annotations to require PHP 7 for tests - split out a single test to demonstrate expected behavior in versions prior to 7.0
zendframework · May 14, 2018 · 4e2655f · 4e2655f
1 parent 643e8a6
commit 4e2655f
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 71 deletions.
diff --git a/src/Adapter/PhpSerialize.php b/src/Adapter/PhpSerialize.php
@@ -1,10 +1,8 @@
 <?php
 /**
- * Zend Framework (http://framework.zend.com/)
- *
- * @link      http://github.com/zendframework/zf2 for the canonical source repository
- * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com)
- * @license   http://framework.zend.com/license/new-bsd New BSD License
+ * @see       https://github.com/zendframework/zend-serializer for the canonical source repository
+ * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-serializer/blob/master/LICENSE.md New BSD License
  */
 
 namespace Zend\Serializer\Adapter;
@@ -124,12 +122,10 @@ public function unserialize($serialized)
 
         ErrorHandler::start(E_NOTICE);
 
-        if (PHP_MAJOR_VERSION >= 7) {
-            // the second parameter is only available on PHP 7.0 or higher
-            $ret = unserialize($serialized, ['allowed_classes' => $this->getOptions()->getUnserializeClassWhitelist()]);
-        } else {
-            $ret = unserialize($serialized);
-        }
+        // The second parameter to unserialize() is only available on PHP 7.0 or higher
+        $ret = PHP_MAJOR_VERSION >= 7
+            ? unserialize($serialized, ['allowed_classes' => $this->getOptions()->getUnserializeClassWhitelist()])
+            : unserialize($serialized);
 
         $err = ErrorHandler::stop();
         if ($ret === false) {

diff --git a/src/Adapter/PhpSerializeOptions.php b/src/Adapter/PhpSerializeOptions.php
@@ -1,10 +1,8 @@
 <?php
 /**
- * Zend Framework (http://framework.zend.com/)
- *
- * @link      http://github.com/zendframework/zf2 for the canonical source repository
- * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com)
- * @license   http://framework.zend.com/license/new-bsd New BSD License
+ * @see       https://github.com/zendframework/zend-serializer for the canonical source repository
+ * @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-serializer/blob/master/LICENSE.md New BSD License
  */
 
 namespace Zend\Serializer\Adapter;
@@ -15,31 +13,31 @@
 class PhpSerializeOptions extends AdapterOptions
 {
     /**
-     * The list of allowed classes for unserialization (PHP 7.0+)
+     * The list of allowed classes for unserialization (PHP 7.0+).
+     *
      * Possible values:
-     * Array of class names that are allowed to be unserialized
-     * or true if all classes should be allowed (behavior of pre PHP 7.0)
-     * or false if no classes should be allowed
+     *
+     * - `array` of class names that are allowed to be unserialized
+     * - `true` if all classes should be allowed (behavior pre-PHP 7.0)
+     * - `false` if no classes should be allowed
      *
      * @var string[]|bool
      */
     protected $unserializeClassWhitelist = true;
 
     /**
-     * @param  string[]|bool $unserializeClassWhitelist
-     *
-     * @return PhpSerializeOptions
+     * @param string[]|bool $unserializeClassWhitelist
+     * @return void
      */
     public function setUnserializeClassWhitelist($unserializeClassWhitelist)
     {
-        if (($unserializeClassWhitelist !== true) && (PHP_MAJOR_VERSION < 7)) {
+        if ($unserializeClassWhitelist !== true && PHP_MAJOR_VERSION < 7) {
             throw new Exception\InvalidArgumentException(
-                'Class whitelist for unserialize() is only available on PHP 7.0 or higher.'
+                'Class whitelist for unserialize() is only available on PHP versions 7.0 or higher.'
             );
         }
 
         $this->unserializeClassWhitelist = $unserializeClassWhitelist;
-        return $this;
     }
 
     /**

diff --git a/test/Adapter/PhpSerializeTest.php b/test/Adapter/PhpSerializeTest.php
@@ -1,15 +1,14 @@
 <?php
 /**
- * Zend Framework (http://framework.zend.com/)
- *
- * @link      http://github.com/zendframework/zf2 for the canonical source repository
- * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
- * @license   http://framework.zend.com/license/new-bsd New BSD License
+ * @see       https://github.com/zendframework/zend-serializer for the canonical source repository
+ * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-serializer/blob/master/LICENSE.md New BSD License
  */
 
 namespace ZendTest\Serializer\Adapter;
 
 use PHPUnit\Framework\TestCase;
+use stdClass;
 use Zend\Serializer;
 use Zend\Serializer\Exception\InvalidArgumentException;
 
@@ -167,65 +166,59 @@ public function testUnserializingInvalidStringRaisesException($string, $expected
         $this->adapter->unserialize($string);
     }
 
-    public function testUnserializeNoWhitelistedClasses()
+    /**
+     * @requires PHP 7.0
+     */
+    public function testPhp7WillNotUnserializeObjectsWhenUnserializeWhitelistedClassesIsFalse()
     {
         $value = 'O:8:"stdClass":0:{}';
+        $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
 
-        if (PHP_MAJOR_VERSION >= 7) {
-            $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
-
-            $data = $this->adapter->unserialize($value);
-
-            $this->assertNotInstanceOf(\stdClass::class, $data);
-            $this->assertInstanceOf('__PHP_Incomplete_Class', $data);
-        } else {
-            // In PHP < 7.0 the options-class will throw an exception
-
-            self::expectException(InvalidArgumentException::class);
-            self::expectExceptionMessage('Class whitelist for unserialize() is only available on PHP 7.0 or higher.');
+        $data = $this->adapter->unserialize($value);
 
-            $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
-        }
+        $this->assertNotInstanceOf(stdClass::class, $data);
+        $this->assertInstanceOf('__PHP_Incomplete_Class', $data);
     }
 
-    public function testUnserializeClassNotAllowed()
+    public function testWhenUnserializeClassWhiteListIsFalseButPHPIsPriorTo7AnExceptionIsRaised()
     {
         $value = 'O:8:"stdClass":0:{}';
 
         if (PHP_MAJOR_VERSION >= 7) {
-            $this->adapter->getOptions()->setUnserializeClassWhitelist([\My\Dummy::class]);
+            $this->markTestSkipped(sprintf('Test %s is only needed in PHP versions prior to 7.0', __FUNCTION__));
+        }
 
-            $data = $this->adapter->unserialize($value);
+        self::expectException(InvalidArgumentException::class);
+        self::expectExceptionMessage('Class whitelist for unserialize() is only available on PHP 7.0 or higher.');
+        $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
+    }
+
+    /**
+     * @requires PHP 7.0
+     */
+    public function testUnserializeWillNotUnserializeClassesThatAreNotInTheWhitelist()
+    {
+        $value = 'O:8:"stdClass":0:{}';
 
-            $this->assertNotInstanceOf(\stdClass::class, $data);
-            $this->assertInstanceOf('__PHP_Incomplete_Class', $data);
-        } else {
-            // In PHP < 7.0 the options-class will throw an exception
+        $this->adapter->getOptions()->setUnserializeClassWhitelist([\My\Dummy::class]);
 
-            self::expectException(InvalidArgumentException::class);
-            self::expectExceptionMessage('Class whitelist for unserialize() is only available on PHP 7.0 or higher.');
+        $data = $this->adapter->unserialize($value);
 
-            $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
-        }
+        $this->assertNotInstanceOf(stdClass::class, $data);
+        $this->assertInstanceOf('__PHP_Incomplete_Class', $data);
     }
 
-    public function testUnserializeClassAllowed()
+    /**
+     * @requires PHP 7.0
+     */
+    public function testUnserializeWillUnserializeAnyClassWhenUnserializeWhitelistedClassesIsTrue()
     {
         $value = 'O:8:"stdClass":0:{}';
 
-        if (PHP_MAJOR_VERSION >= 7) {
-            $this->adapter->getOptions()->setUnserializeClassWhitelist([\stdClass::class]);
-
-            $data = $this->adapter->unserialize($value);
-            $this->assertInstanceOf(\stdClass::class, $data);
-            $this->assertNotInstanceOf('__PHP_Incomplete_Class', $data);
-        } else {
-            // In PHP < 7.0 the options-class will throw an exception
+        $this->adapter->getOptions()->setUnserializeClassWhitelist([stdClass::class]);
 
-            self::expectException(InvalidArgumentException::class);
-            self::expectExceptionMessage('Class whitelist for unserialize() is only available on PHP 7.0 or higher.');
-
-            $this->adapter->getOptions()->setUnserializeClassWhitelist(false);
-        }
+        $data = $this->adapter->unserialize($value);
+        $this->assertInstanceOf(stdClass::class, $data);
+        $this->assertNotInstanceOf('__PHP_Incomplete_Class', $data);
     }
 }