@anonymous-piwik-user opened this Issue on January 25th 2014

File: core/Common.php
Line #: On or about 942
Function: getCampaignParameters()
Line: array_walk_recursive($return, 'trim');

This should do nothing. 'trim' does not meet the callback requirements of array_walk_recursive.

callback expects 2 parameters, my_callback(&$value, $index). trim takes trim($str, $charset)
a) $index will mess up charset
b) if you want to change $value it needs to be referenced, trim returns the value

Suggested resolutions:
1) remove offending line, it has never worked and no one noticed.
2) Move the trim function to the loop
From This:
foreach ($return as &$list) {
if (strpos($list, ',') !== false) {
$list = explode(',', $list);
} else {
$list = array($list);
}
}
To This:
foreach ($return as &$list) {
if (strpos($list, ',') !== false) {
$list = array_map('trim', explode(',', $list));
} else {
$list = array(trim($list));
}
}

3) Create wrapper function for trim:
array_walk_recursive($return, create_function('&$v, $k', '$v = trim($v);');

Backstory:
I am currently playing with getting piwik running on hhvm. I am was getting a warning from the offending line. That is how and why I cam across this.

Code History:
The code was added with regard to ticket #855
with a commit message of:

Fixes #855 Now detecting google style campaign parameters (utm_campaign, utm_term)
git-svn-id: http://dev.piwik.org/svn/trunk@4389 59fd770c-687e-43c8-a1e3-f5a4ff64c105

Committed By: mattpiwik
On: 2011-04-10 23:36:30

@mattab commented on January 27th 2014 Owner

In d71d0b1d1abbfc2b958a155c95b4143d4142962d: Fixes #4572 Make code work. Thanks for the report! (please keep feedback coming about HipHop virtual machine! that's super interesting :))

@amcnea commented on July 9th 2014

This comment is irrelevant to the issue.

Just giving an update on HHVM. This was a while ago, so, I am going on my memory of events. I was successfully able to get the collectors working under HHVM and it did improve performance significantly. The admin interface was still very buggy, however, I didn't need or work on getting that running because we had a standard Apache server which the ad-ops guys used.

A couple of the issues which came up were:
1) There were a few PHP Constants that HHVM didn't have pre-define, so, I just defined them in the root php file.
2) There was an issue with some function (I forget the name), but luckily you had an if statement which switched between the Core PHP defined function and an equivalent User defined function. I don't remember why the if statement was selecting the PHP defined version, but I basically just removed the if statement and forced the User defined function to always be used.

I think those were the only issues and after that the collectors worked perfectly. Of course we only tested the aspects of the collectors which we were using, so, I am not sure if there may be other issues if other features are enabled.

This Issue was closed on January 27th 2014
Powered by GitHub Issue Mirror