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 stats leakage #86

Closed
wants to merge 4 commits into from
Closed

Fix stats leakage #86

wants to merge 4 commits into from

Conversation

chrmod
Copy link
Member

@chrmod chrmod commented Oct 11, 2023

PR aims to prevent adblocker and anti-tracking stats "leaking" between navigations. Sadly the solution is not complete for Ghostery use-case as the same problem is also present in the Ghostery codebase.

@chrmod
Copy link
Member Author

chrmod commented Oct 11, 2023

@philipp-classen @smalluban please review commits in isolation and do not squash on merge.

@chrmod chrmod force-pushed the fix-stats-leakage branch from 75025e9 to 33664aa Compare October 12, 2023 07:33
Copy link

@smalluban smalluban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the commits separately, and they look fine. However, my knowledge about the ghostery common is very limited (and how the webrequest pipeline works), so I cannot say much more.

},
}
};
if (!global.crypto) {
Copy link
Member

@philipp-classen philipp-classen Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is currently written (after the PR), the tests have the side effects to change
global.crypto === undefined (before tests) to global.crypto !== undefined (after tests)

@chrmod chrmod force-pushed the fix-stats-leakage branch 2 times, most recently from 8e3681d to 79e74d7 Compare October 12, 2023 09:25
@chrmod chrmod force-pushed the fix-stats-leakage branch from 79e74d7 to e2f148f Compare October 12, 2023 11:50
@chrmod
Copy link
Member Author

chrmod commented Oct 12, 2023

closing in favor of smaller PRs

@chrmod chrmod closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants