Add more tests for format=original.

core/Common.php

@@ -261,15 +261,19 @@ public static function mb_strtoupper($string)
      *
      * @param string $string String to unserialize
      * @param array $allowedClasses Class names that should be allowed to unserialize
-     *
+     * @param bool $rethrow Whether to rethrow exceptions or not.
      * @return mixed
      */
-    public static function safe_unserialize($string, $allowedClasses = [])
+    public static function safe_unserialize($string, $allowedClasses = [], $rethrow = false)
     {
         if (PHP_MAJOR_VERSION >= 7) {
             try {
                 return unserialize($string, ['allowed_classes' => empty($allowedClasses) ? false : $allowedClasses]);
             } catch (\Throwable $e) {
+                if ($rethrow) {
+                    throw $e;
+                }
+
                 $logger = StaticContainer::get('Psr\Log\LoggerInterface');
                 $logger->debug('Unable to unserialize a string: {message} (string = {string})', [
                     'message' => $e->getMessage(),

tests/PHPUnit/Framework/TestCase/SystemTestCase.php

@@ -13,11 +13,16 @@
 use Piwik\Common;
 use Piwik\Config;
 use Piwik\Container\StaticContainer;
+use Piwik\DataTable;
 use Piwik\DataTable\Manager;
+use Piwik\Date;
 use Piwik\Db;
 use Piwik\DbHelper;
 use Piwik\Http;
+use Piwik\Period;
+use Piwik\Plugin\ProcessedMetric;
 use Piwik\ReportRenderer;
+use Piwik\Site;
 use Piwik\Tests\Framework\Constraint\ResponseCode;
 use Piwik\Tests\Framework\Constraint\HttpResponseText;
 use Piwik\Tests\Framework\TestRequest\ApiTestConfig;
@@ -386,7 +391,9 @@ protected function _testApiUrl($testName, $apiId, $requestUrl, $compareAgainst,
         $_GET = $requestUrl;
         unset($_GET['serialize']);
 
-        $processedResponse = Response::loadFromApi($params, $requestUrl);
+        $onlyCheckUnserialize = !empty($params['onlyCheckUnserialize']);
+
+        $processedResponse = Response::loadFromApi($params, $requestUrl, $normailze = !$onlyCheckUnserialize);
         if (empty($compareAgainst)) {
             $processedResponse->save($processedFilePath);
         }
@@ -396,6 +403,38 @@ protected function _testApiUrl($testName, $apiId, $requestUrl, $compareAgainst,
             $this->assertValidXML($response);
         }
 
+        if ($onlyCheckUnserialize) {
+            if (empty($response) || is_numeric($response)) {
+                return; // pass
+            }
+
+            // check the data can be successfully unserialized, nothing else
+            try {
+                $unserialized = Common::safe_unserialize($response, [
+                    DataTable::class,
+                    DataTable\Simple::class,
+                    DataTable\Row::class,
+                    DataTable\Map::class,
+                    Site::class,
+                    Date::class,
+                    Period::class,
+                    Period\Day::class,
+                    Period\Week::class,
+                    Period\Month::class,
+                    Period\Year::class,
+                    Period\Range::class,
+                    ProcessedMetric::class,
+                ], true);
+
+                if ($unserialized === false) {
+                    throw new \Exception("Unknown serialization error.");
+                }
+            } catch (\Exception $ex) {
+                $this->comparisonFailures[] = new \Exception("Processed response in '$processedFilePath' could not be unserialized: " . $ex->getMessage());
+            }
+            return;
+        }
+
         $_GET = $originalGET;
 
         try {

tests/PHPUnit/Framework/TestRequest/ApiTestConfig.php

@@ -192,6 +192,13 @@ class ApiTestConfig
      */
     public $keepLiveIds = false;
 
+    /**
+     * For format=original tests. Will forego comparison w/ expected files and just make sure unserialize works.
+     *
+     * @var bool
+     */
+    public $onlyCheckUnserialize = false;
+
     /**
      * Constructor. Sets class properties using an associative array mapping property names w/ values.
      *

tests/PHPUnit/Framework/TestRequest/Collection.php

@@ -48,6 +48,7 @@ class Collection
         'CustomAlerts',
         'Insights',
         'LogViewer',
+        'Referrers.getKeywordNotDefinedString',
     );
 
     /**

tests/PHPUnit/Framework/TestRequest/Response.php

@@ -26,13 +26,13 @@ class Response
 
     private $requestUrl;
 
-    public function __construct($apiResponse, $params, $requestUrl)
+    public function __construct($apiResponse, $params, $requestUrl, $normalize = true)
     {
         $this->params = $params;
         $this->requestUrl = $requestUrl;
 
         $apiResponse = (string) $apiResponse;
-        $this->processedResponseText = $this->normalizeApiResponse($apiResponse);
+        $this->processedResponseText = $normalize ? $this->normalizeApiResponse($apiResponse) : $apiResponse;
     }
 
     public function getResponseText()
@@ -56,15 +56,15 @@ public static function loadFromFile($path, $params, $requestUrl)
         return new Response($contents, $params, $requestUrl);
     }
 
-    public static function loadFromApi($params, $requestUrl)
+    public static function loadFromApi($params, $requestUrl, $normalize = true)
     {
         $testRequest = new Request($requestUrl);
 
         // Cast as string is important. For example when calling
         // with format=original, objects or php arrays can be returned.
         $response = (string) $testRequest->process();
 
-        return new Response($response, $params, $requestUrl);
+        return new Response($response, $params, $requestUrl, $normalize);
     }
 
     public static function assertEquals(Response $expected, Response $actual, $message = false)

tests/PHPUnit/System/OneVisitorTwoVisitsTest.php

@@ -86,6 +86,15 @@ public function getApiForTesting()
                                )
             )),
 
+            array('all', array('idSite' => $idSite,
+                'date' => $dateTime,
+                'format' => 'original',
+                'otherRequestParameters' => array(
+                    'serialize' => '1',
+                ),
+                'onlyCheckUnserialize' => true,
+            )),
+
             // test API.get (for bug that incorrectly reorders columns of CSV output)
             //   note: bug only affects rows after first
             array('API.get', array('idSite'                 => $idSite,