Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 3rd party cookie / global visitorid race condition #13109

Merged

Conversation

MichaelRoosz
Copy link
Contributor

When 3rd party cookies (_pk_uid) are enabled and a pageview is tracked to multiple siteids at once, then there is a race condition:

A new user without the 3rd party cookie arrives at page X.
The pageview of X is tracked to siteid A and at the same time to siteid B.
Both track requests do not have the 3rd party cookie set.
When matomo processes these requests, it will use the visitorid that came with the request as visitorid for the 3rd party cookie.
Here is a race condition: one of the requests will win, its visitorid will be the new global visitorid in the 3rd party cookie.
The visitorid of the request that lost remains in the database as an additional visitor+visit, and thus corrupting the data.

My fix:
Set the 3rd party cookie when sending the piwik javascript code (in /js/tracker.php).
This will of course not fix the problem, when the js is loaded directly from /piwik.js.
Thus I would recommend to add this case to the FAQ: When using 3rd party cookies AND tracking to multiple siteids at once, one must use /js/tracker.php and not /piwik.js to load the js in the browser.

@sgiehl sgiehl added the Needs Review PRs that need a code review label Jun 28, 2018
@MichaelRoosz
Copy link
Contributor Author

Updated: do not write third party cookie if ignore cookie is present

@MichaelRoosz MichaelRoosz force-pushed the fix_3rdparty_cookie_race_condition branch from ca96a26 to ad18e01 Compare October 17, 2018 06:40
@MichaelRoosz MichaelRoosz force-pushed the fix_3rdparty_cookie_race_condition branch from ad18e01 to 31b2216 Compare October 17, 2018 06:58
@MichaelRoosz
Copy link
Contributor Author

Updated: cleaned & improved the pr

@mattab mattab added Needs Review PRs that need a code review and removed Needs Review PRs that need a code review labels Jun 27, 2019
@MichaelRoosz
Copy link
Contributor Author

@tsteur As this is quite a critical bug, is there anything I can do to get this merged? Creating a test for a race condition would be kind of hard.

@tsteur
Copy link
Member

tsteur commented Jun 29, 2020

Tested it and works 👍 Happy to merge @MichaelHeerklotz . Good find... was probably tricky :)

@tsteur tsteur merged commit be326c4 into matomo-org:3.x-dev Jun 29, 2020
jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
… cookie during js code fetch (matomo-org#13109)

Co-authored-by: Michael Heerklotz <michael.heerklotz@check24.de>
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
… cookie during js code fetch (matomo-org#13109)

Co-authored-by: Michael Heerklotz <michael.heerklotz@check24.de>
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2020
@MichaelRoosz MichaelRoosz deleted the fix_3rdparty_cookie_race_condition branch October 29, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants