-
Notifications
You must be signed in to change notification settings - Fork 888
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
Filter user-tracking parameters from query string #3239
Conversation
4dd3953
to
57a3a42
Compare
Note to reviewers: while the custom query string parsing code is unfortunate, Chromium's
Given that the format of the query string is not specified in RFC 3986, some frameworks don't agree with the way that the Mozilla parser interprets the query string and instead considers My parser aims at keeping to a minimum the changes we make to the query string and handles seemingly invalid query strings just fine. I have tried to optimize for the cases where (1) there is no query string and (2) the query string doesn't include any of the trackers. |
57a3a42
to
8045692
Compare
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides minors and nits, I think we should try to simplify the parsing part.
8aaf9a2
to
ec7f061
Compare
ec7f061
to
4910d39
Compare
4910d39
to
57f61a0
Compare
20b255a
to
63fd13f
Compare
All review comments addressed in latest revision.
63fd13f
to
86b95c7
Compare
86b95c7
to
10a9663
Compare
10a9663
to
281c794
Compare
281c794
to
85b00d4
Compare
…browser#4239) If a URL's query string includes one of the parameter names known to track individual users, we remove them. We essentially apply the following to the query string: s/&(fbclid|gclid|msclkid|mc_eid)=[^&]+//g s/^(fbclid|gclid|msclkid|mc_eid)=[^&]+&//g s/^(fbclid|gclid|msclkid|mc_eid)=[^&]+$//g https://support.google.com/analytics/answer/7519794 https://stackoverflow.com/questions/52847475/what-is-fbclid-the-new-facebook-parameter https://about.ads.microsoft.com/en-us/blog/post/january-2018/conversion-tracking-update-on-bing-ads https://developer.mailchimp.com/documentation/mailchimp/guides/getting-started-with-ecommerce/#e-commerce-tracking-and-reports
85b00d4
to
609f59d
Compare
Fixes brave/brave-browser#4239.
If a URL's query string includes one of the parameter names known
to track individual users, we remove them.
https://support.google.com/analytics/answer/7519794
https://stackoverflow.com/questions/52847475/what-is-fbclid-the-new-facebook-parameter
https://about.ads.microsoft.com/en-us/blog/post/january-2018/conversion-tracking-update-on-bing-ads
https://developer.mailchimp.com/documentation/mailchimp/guides/getting-started-with-ecommerce/#e-commerce-tracking-and-reports
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Visit the following URLs:
and verify that the query string is removed and that the URL bar only shows
https://brave.com/
.Reviewer Checklist:
After-merge Checklist:
changes has landed on.