-
Notifications
You must be signed in to change notification settings - Fork 862
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
Issue 2252: Refactor the referrer hiding. #1240
Conversation
4f150fd
to
ee2308c
Compare
TODO:
|
browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Outdated
Show resolved
Hide resolved
referrer); | ||
} | ||
|
||
|
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.
nit: Please remove the extra line.
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.
done
|
||
} // namespace | ||
|
||
|
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.
nit: remove the extra line
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.
done
b9efbe8
to
f7a53f4
Compare
it's easier to review per-commit, since I've clang-formatted one of the files. |
83914b2
to
07efa8b
Compare
Fix #2252 Current solution for hiding the referer string requires readirecting literally all requests for which the referrer was modified. This approach conflicts with the fact that certain requests are not allowed to be redirected (i.e. CORS preflight requests). That said, current approach breaks a lot of sites. To refactor this we move the referrer hiding code to the earlier stage, where navigation requests are created and started. This is a convenient place to patch both renderer and browser initiated navigations and utilize ContentBrowserClient interface on the browser thread.
We have to keep this at least for a while, because we still want to hide referrers for all requests, including resource requests originated from renderer. But now we do not want to trigger internal redirects for each request to avoid CORS or any other issues.
07efa8b
to
71bc678
Compare
Issue 2252: Refactor the referrer hiding.
Issue 2252: Refactor the referrer hiding.
Issue 2252: Refactor the referrer hiding.
Fix brave/brave-browser#2252
Current solution for hiding the referer string requires readirecting literally
all requests for which the referrer was modified. This approach conflicts
with the fact that certain requests are not allowed to be redirected (i.e.
CORS preflight requests). That said, current approach breaks a lot of sites.
To refactor this we move the referrer hiding code to the earlier stage,
where navigation requests are created and started. This is a convenient
place to patch both renderer and browser initiated navigations and utilize
ContentBrowserClient interface on the browser thread.
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests
) ongit rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist: