[Piwik1.3] Is this a BUG?

Support & Bugs
1

Hi,
I am going to upgrade to Piwik 1.3, I have read the code of core/Tracker/Visit.php and found in the code a bug(might not be a bug, need your confirmation), which starts from #344:


$idVisitor = Piwik_Common::getRequestVar('_id', '', 'string', $this->request);
if(strlen($idVisitor) == Piwik_Tracker::LENGTH_HEX_ID_STRING)
{
	$valuesToUpdate['idvisitor'] = Piwik_Common::hex2bin($idVisitor);
}

In the preceding code, Piwik does not test whether 3rd party cookie is enabled and use 1st party cookie directly. As on #839 in core/Tracker/Visit.php, 3rd cookie takes higher priority when enabled.

It looks to me the preceding code should be modified to take 3rd party cookie into account, something like the following:


$idVisitor = '';
$useThirdPartyCookie = $this->shouldUseThirdPartyCookie();
if($useThirdPartyCookie)
{
	$idVisitor = $this->cookie->get(0);
	if($idVisitor == false || strlen($idVisitor) != Piwik_Tracker::LENGTH_HEX_ID_STRING)
	{
		$idVisitor = Piwik_Common::getRequestVar('_id', '', 'string', $this->request);
	}
}

if(strlen($idVisitor) == Piwik_Tracker::LENGTH_HEX_ID_STRING)
{
	$valuesToUpdate['idvisitor'] = Piwik_Common::hex2bin($idVisitor);
}

Please confirm me if I am wrong or right, thank you!

Regards,
Kevin

2

No it’s not a bug. First party were not used at all in 1.2 to define the idvisitor, a heuristic is used. See the full code it is self documented

3

Thanks for your swift reply, but I still don’t understand what you mean by ‘First party were not used at all in 1.2 to define the idvisitor’, the question is about ‘3rd cookie not used when it should be(use_third_party_id_cookie = 1 in config.ini.php)’

In the recognizeTheVisitor() function, 3rd party cookie is used when it is enabled, while in handleKnownVisit(), 3rd is completely ignored and 1st party cookie(passed as the _id parameter) is used, this will cause inconsistency in the piwik_log_link_visit_action.

Would you please explain it with more details?

4

Experts, would you please explain this problem with more details?

5

kevintse, you are very right indeed! Thanks for the review. I have patched on SVN: http://dev.piwik.org/trac/changeset/4526