Php serializer was not serializing arrays by default. #6155

claytondaley · 2014-09-07T16:24:21Z

ham1 · 2014-09-10T20:48:04Z

This seems to fix the issue #6137 for WP-Piwik but I now get error on the visitor map inside Piwik itself (see below).

Could this be a problem with the visitor map?

e.g. Warning: Illegal string offset 'metrics' in /var/www/xxx/plugins/UserCountryMap/Controller.php on line 226 

Backtrace -->

#0 Piwik\Error::errorHandler(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:226]
#1 Piwik\Plugins\UserCountryMap\Controller->getMetrics(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:89]
#2 Piwik\Plugins\UserCountryMap\Controller->visitorMap(...) called at [:]
#3 call_user_func_array(...) called at [/var/www/xxx/core/FrontController.php:577]
#4 Piwik\FrontController->doDispatch(...) called at [/var/www/xxx/core/FrontController.php:86]
#5 Piwik\FrontController->dispatch(...) called at [/var/www/xxx/core/dispatch.php:34]
#6 require_once(...) called at [/var/www/xxx/index.php:46]


There is an error. Please report the message (Piwik 2.6.1) and full backtrace in the Piwik forums (please do a Search first as it might have been reported already!).

Warning: Invalid argument supplied for foreach() in /var/www/xxx/plugins/UserCountryMap/Controller.php on line 226 

Backtrace -->

#0 Piwik\Error::errorHandler(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:226]
#1 Piwik\Plugins\UserCountryMap\Controller->getMetrics(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:89]
#2 Piwik\Plugins\UserCountryMap\Controller->visitorMap(...) called at [:]
#3 call_user_func_array(...) called at [/var/www/xxx/core/FrontController.php:577]
#4 Piwik\FrontController->doDispatch(...) called at [/var/www/xxx/core/FrontController.php:86]
#5 Piwik\FrontController->dispatch(...) called at [/var/www/xxx/core/dispatch.php:34]
#6 require_once(...) called at [/var/www/xxx/index.php:46]


There is an error. Please report the message (Piwik 2.6.1) and full backtrace in the Piwik forums (please do a Search first as it might have been reported already!).

Warning: Illegal string offset 'processedMetrics' in /var/www/xxx/plugins/UserCountryMap/Controller.php on line 231 

Backtrace -->

#0 Piwik\Error::errorHandler(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:231]
#1 Piwik\Plugins\UserCountryMap\Controller->getMetrics(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:89]
#2 Piwik\Plugins\UserCountryMap\Controller->visitorMap(...) called at [:]
#3 call_user_func_array(...) called at [/var/www/xxx/core/FrontController.php:577]
#4 Piwik\FrontController->doDispatch(...) called at [/var/www/xxx/core/FrontController.php:86]
#5 Piwik\FrontController->dispatch(...) called at [/var/www/xxx/core/dispatch.php:34]
#6 require_once(...) called at [/var/www/xxx/index.php:46]


There is an error. Please report the message (Piwik 2.6.1) and full backtrace in the Piwik forums (please do a Search first as it might have been reported already!).

Warning: Invalid argument supplied for foreach() in /var/www/xxx/plugins/UserCountryMap/Controller.php on line 231 

Backtrace -->

#0 Piwik\Error::errorHandler(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:231]
#1 Piwik\Plugins\UserCountryMap\Controller->getMetrics(...) called at [/var/www/xxx/plugins/UserCountryMap/Controller.php:89]
#2 Piwik\Plugins\UserCountryMap\Controller->visitorMap(...) called at [:]
#3 call_user_func_array(...) called at [/var/www/xxx/core/FrontController.php:577]
#4 Piwik\FrontController->doDispatch(...) called at [/var/www/xxx/core/FrontController.php:86]
#5 Piwik\FrontController->dispatch(...) called at [/var/www/xxx/core/dispatch.php:34]
#6 require_once(...) called at [/var/www/xxx/index.php:46]

reissmann · 2014-09-10T21:06:43Z

I can confirm this. Fixes WP-Piwik issue, but breaks visitor map.

Warning: Illegal string offset 'metrics' in /var/www/piwik/plugins/UserCountryMap/Controller.php on line 226

Backtrace -->

#0 Piwik\Error::errorHandler(...) called at [/var/www/piwik/plugins/UserCountryMap/Controller.php:226]
#1 Piwik\Plugins\UserCountryMap\Controller->getMetrics(...) called at [/var/www/piwik/plugins/UserCountryMap/Controller.php:89]
#2 Piwik\Plugins\UserCountryMap\Controller->visitorMap(...) called at [:]
#3 call_user_func_array(...) called at [/var/www/piwik/core/FrontController.php:577]
#4 Piwik\FrontController->doDispatch(...) called at [/var/www/piwik/core/FrontController.php:86]
#5 Piwik\FrontController->dispatch(...) called at [/var/www/piwik/core/dispatch.php:34]
#6 require_once(...) called at [/var/www/piwik/index.php:46]

[...]

…cess()

claytondaley · 2014-09-10T22:24:56Z

I've update the pull request to correct UserCountryMap:

I update line 223 to unserialize the response (this causes it to look exactly like line 54).
Note that it would also be possible (and I find it more visually appealing) to instead include "&serialize=0" in the request URL. I considered making this change as well, but am unclear if transmitting raw PHP is as safe as sending text that must be unserialized.

If you look at the format section of the Reporting API Reference, it clearly states that PHP should return serialized data by default so I continue to believe that this is the appropriate change and that any function assuming otherwise was updated in error.

ham1 · 2014-09-11T19:59:23Z

Can confirm this fixes the map issue 👍
Also agree with your interpretation of the documentation - however does someone know if this was changed for a reason, but the documentation wasn't updated?

claytondaley · 2014-09-11T21:57:00Z

It's very hard to trace because the logical flow was reordered during a refactor. In both original and revised, getResponse() calls handleArray().

In the old version, handleArray() "Converts the given simple array to a data table" and calls getRenderedDataTable(). In the PHP block, getRenderedDataTable() calls caseRendererPHPSerialize()

        if ($format == 'php') {     
            $renderer->setSerialize($this->caseRendererPHPSerialize());     
            $renderer->setPrettyDisplay(Common::getRequestVar('prettyDisplay', false, 'int', $this->request));      
        } else

By default, caseRendererPHPSerialize() returns true

protected function caseRendererPHPSerialize($defaultSerializeValue = 1)     
    {       
        $serialize = Common::getRequestVar('serialize', $defaultSerializeValue, 'int', $this->request);     
        if ($serialize) {       
            return true;        
        }       
        return false;       
    }

In the revised code, handleArray() calls renderArray(), which did not exist in the old code. In the PHP version of renderArray(), shouldSerialze() is defaulted to false:

    public function renderArray($array)
    {
        if (!Piwik::isMultiDimensionalArray($array)) {
            return parent::renderArray($array);
        }

        if ($this->shouldSerialize(0)) {
            return serialize($array);
        }

        return $array;
    }

While the PHP version of renderDataTable() also defaults to false in this particular commit:

        $tableRenderer->setSerialize($this->shouldSerialize(0));

it was later corrected:

        $tableRenderer->setSerialize($this->shouldSerialize(1));

If arrays were still routed through renderDataTable() they too would have been corrected to serializ by default, which is why this patch appears to be consistent with the code prior to the refactor.

mattab · 2014-09-14T05:12:34Z

Thanks for the report & analysis @claytondaley

does someone know if this was changed for a reason, but the documentation wasn't updated?

Probably it was changed by mistake, maybe we didn't have tests for this use case. Because this is an API breaking change, it should have been documented in the Platform changelog: http://developer.piwik.org/changelog

@tsteur maybe you have some idea about this one?

tsteur · 2014-09-14T06:57:30Z

Don't think it was on purpose. It would have been documented otherwise. Tests were very likely not covering this.

mattab · 2014-09-16T03:21:54Z

Thanks for the fix. I'll check if tests pass and will add an integration test case for format=php

Php serializer was not serializing arrays by default.

…imensional arrays

Fixes matomo-org#6137

39d6a96

- Fix UserCountryMap to correctly unserialized call to $request->pro…

81a81c8

…cess()

mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Sep 14, 2014

mattab added this to the Piwik 2.7.0 milestone Sep 16, 2014

mattab pushed a commit that referenced this pull request Sep 16, 2014

Merge pull request #6155 from claytondaley/php_format

5ea20ff

Php serializer was not serializing arrays by default.

mattab merged commit 5ea20ff into matomo-org:master Sep 16, 2014

mattab added a commit that referenced this pull request Sep 16, 2014

refs #6137 #6155 adding unit test showcasing rendering bug on multi d…

464168f

…imensional arrays

claytondaley deleted the php_format branch September 27, 2014 21:55

claytondaley restored the php_format branch March 28, 2017 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Php serializer was not serializing arrays by default. #6155

Php serializer was not serializing arrays by default. #6155

claytondaley commented Sep 7, 2014

ham1 commented Sep 10, 2014

reissmann commented Sep 10, 2014

claytondaley commented Sep 10, 2014

ham1 commented Sep 11, 2014

claytondaley commented Sep 11, 2014

mattab commented Sep 14, 2014

tsteur commented Sep 14, 2014

mattab commented Sep 16, 2014

Php serializer was not serializing arrays by default. #6155

Php serializer was not serializing arrays by default. #6155

Conversation

claytondaley commented Sep 7, 2014

ham1 commented Sep 10, 2014

reissmann commented Sep 10, 2014

claytondaley commented Sep 10, 2014

ham1 commented Sep 11, 2014

claytondaley commented Sep 11, 2014

mattab commented Sep 14, 2014

tsteur commented Sep 14, 2014

mattab commented Sep 16, 2014