[Piwik1.3]Visit is not identified correctly due to "always matching on config_id"

Hi, all
I have found something that I believe is a logic error in core/Tracker/Visit.php, from line 876 to 894.

On line 876, there’s a comment reads: “we always match on the config_id, except if the current request forces the visitor id”, I think this is not always a good idea, sometimes it can be too accurate to be correct.

config_id is a hash of a whole lot of things, including IP, this is fine. but sometimes a user may have different IPs(on the same router) when connecting to the internet, for such a case, this user can be correctly recognized as the same unique visitor, but the current visit will be identified as a new visit no matter how long has passed since his/her last visit cause now he/she has different config_id compared to the previous visit.

There’s a configuration trust_visitors_cookies, when set to 1, I think should always take top priority and config_id should be completely ignored. and then the current visit be safely identified as “known” regardless of anything else(include config_id) if the idvisitor exists(because this is my control, I set it to always to trust idvisitor), and leave the last identification to $this->isLastActionInTheSameVisit() on line 174.

Please confirm me or deny me.

Regards,
Kevin

Hi Kevin, again you are right :wink: http://dev.piwik.org/trac/changeset/4575 thanks!

And a follow up health check: http://dev.piwik.org/trac/changeset/4576

My fix:


// we always match on the config_id, except if the current request forces the visitor id or trust_visitors_cookies=1
// BUG: http://forum.piwik.org/read.php?2,75963
if(!$forcedVisitorId && !(Piwik_Tracker_Config::getInstance()->Tracker['trust_visitors_cookies'])) {
	$where .= ' AND config_id = ? ';
	$bindSql[] = $configId;
}

// We force to match a visitor ID
// 1) If the visitor cookies should be trusted (ie. intranet) - config file setting
// 2) or if the Visitor ID was forced via the Tracking API setVisitorId()
if(Piwik_Tracker_Config::getInstance()->Tracker['trust_visitors_cookies'] || $forcedVisitorId ) {
	if(!empty($this->visitorInfo['idvisitor'])) {
		printDebug("Matching the visitor based on his idcookie: ".bin2hex($this->visitorInfo['idvisitor']) ."...");

		$where .= ' AND idvisitor = ?';
		$bindSql[] = $this->visitorInfo['idvisitor'];
	}  else {
		// Forced idvisitor or trust_visitors_cookies=1, but empty idvisitor
		return;
	}
}

Hi Kevin/Matt,

I have used trust_visitors_cookies = 1 setting int the config.ini file but still piwik is logging on the basis of IP addresses and not on the basis of vistor ID. Is there something else that needs to be done to achieve the same?

@himanshu.mehta1 are you using latest piwik version? this bug should be fixed. If it is not working for you please let me know