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

Issue 2252: Refactor the referrer hiding #1260

Merged
merged 1 commit into from
Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "chrome/common/url_constants.h"
#include "components/content_settings/core/browser/cookie_settings.h"
#include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/browser_url_handler.h"
Expand Down Expand Up @@ -211,6 +212,38 @@ void BraveContentBrowserClient::AdjustUtilityServiceProcessCommandLine(
}
}

void BraveContentBrowserClient::MaybeHideReferrer(
content::BrowserContext* browser_context,
const GURL& request_url,
const GURL& document_url,
content::Referrer* referrer) {
DCHECK(referrer);
if (document_url.SchemeIs(kChromeExtensionScheme)) {
return;
}

Profile* profile = Profile::FromBrowserContext(browser_context);
const bool allow_referrers =
brave_shields::IsAllowContentSettingsForProfile(
profile,
document_url,
document_url,
CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kReferrers);
const bool shields_up =
brave_shields::IsAllowContentSettingsForProfile(
profile,
document_url,
GURL(),
CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kBraveShields);
brave_shields::ShouldSetReferrer(allow_referrers, shields_up,
referrer->url, document_url, request_url,
request_url.GetOrigin(),
referrer->policy,
referrer);
}

GURL BraveContentBrowserClient::GetEffectiveURL(
content::BrowserContext* browser_context,
const GURL& url) {
Expand Down
9 changes: 9 additions & 0 deletions browser/brave_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

#include <memory>

namespace content {
class BrowserContext;
}

class BraveContentBrowserClient : public ChromeContentBrowserClient {
public:
BraveContentBrowserClient(ChromeFeatureListCreator* chrome_feature_list_creator = nullptr);
Expand Down Expand Up @@ -55,6 +59,11 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient {
const service_manager::Identity& identity,
base::CommandLine* command_line) override;

void MaybeHideReferrer(content::BrowserContext* browser_context,
const GURL& request_url,
const GURL& document_url,
content::Referrer* referrer) override;

GURL GetEffectiveURL(content::BrowserContext* browser_context,
const GURL& url) override;

Expand Down
3 changes: 1 addition & 2 deletions browser/net/brave_network_delegate_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ void BraveNetworkDelegateBase::RunNextCallback(

if (ctx->event_type == brave::kOnBeforeRequest) {
if (!ctx->new_url_spec.empty() &&
(ctx->new_url_spec != ctx->request_url.spec() ||
ctx->referrer_changed) &&
(ctx->new_url_spec != ctx->request_url.spec()) &&
IsRequestIdentifierValid(ctx->request_identifier)) {
*ctx->new_url = GURL(ctx->new_url_spec);
}
Expand Down
18 changes: 7 additions & 11 deletions browser/net/brave_site_hacks_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
using content::BrowserThread;
using content::Referrer;

namespace brave {

namespace {

bool ApplyPotentialReferrerBlock(net::URLRequest* request) {
bool ApplyPotentialReferrerBlock(std::shared_ptr<BraveRequestInfo> ctx,
net::URLRequest* request) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
GURL target_origin = GURL(request->url()).GetOrigin();
GURL tab_origin = request->site_for_cookies().GetOrigin();
GURL target_origin = request->url().GetOrigin();
GURL tab_origin = ctx->tab_origin;
if (tab_origin.SchemeIs(kChromeExtensionScheme)) {
return false;
}
Expand All @@ -52,17 +55,10 @@ bool ApplyPotentialReferrerBlock(net::URLRequest* request) {

} // namespace

namespace brave {

int OnBeforeURLRequest_SiteHacksWork(
const ResponseCallback& next_callback,
std::shared_ptr<BraveRequestInfo> ctx) {

if (ApplyPotentialReferrerBlock(const_cast<net::URLRequest*>(ctx->request))) {
ctx->new_url_spec = ctx->request_url.spec();
ctx->referrer_changed = true;
}

ApplyPotentialReferrerBlock(ctx, const_cast<net::URLRequest*>(ctx->request));
return net::OK;
}

Expand Down
2 changes: 0 additions & 2 deletions browser/net/brave_site_hacks_network_delegate_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

#include "brave/browser/net/url_context.h"

struct BraveRequestInfo;

namespace net {
class URLRequest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerCleared) {
brave::ResponseCallback callback;
int ret = brave::OnBeforeURLRequest_SiteHacksWork(callback, brave_request_info);
EXPECT_EQ(ret, net::OK);
// new_url_spec must be set to trigger an internal redirect
EXPECT_STREQ(brave_request_info->new_url_spec.c_str(), url.spec().c_str());
// new_url should not be set
EXPECT_TRUE(brave_request_info->new_url_spec.empty());
EXPECT_STREQ(request->referrer().c_str(), url.GetOrigin().spec().c_str());
});
}
Expand Down
1 change: 0 additions & 1 deletion browser/net/url_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ struct BraveRequestInfo {
bool allow_http_upgradable_resource = false;
bool allow_1p_cookies = true;
bool allow_3p_cookies = false;
bool referrer_changed = false;
int render_process_id = 0;
int render_frame_id = 0;
int frame_tree_node_id = 0;
Expand Down
65 changes: 48 additions & 17 deletions components/brave_shields/browser/brave_shields_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "brave/common/shield_exceptions.h"
#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/profiles/profile_io_data.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
Expand All @@ -30,6 +31,8 @@ using namespace net::registry_controlled_domains;

namespace brave_shields {

namespace {

bool GetDefaultFromResourceIdentifier(const std::string& resource_identifier,
const GURL& primary_url, const GURL& secondary_url) {
if (resource_identifier == brave_shields::kAds) {
Expand All @@ -48,6 +51,31 @@ bool GetDefaultFromResourceIdentifier(const std::string& resource_identifier,
return false;
}

bool IsAllowContentSetting(HostContentSettingsMap* content_settings,
const GURL& primary_url,
const GURL& secondary_url,
ContentSettingsType setting_type,
const std::string& resource_identifier) {
DCHECK(content_settings);
content_settings::SettingInfo setting_info;
std::unique_ptr<base::Value> value =
content_settings->GetWebsiteSetting(
primary_url, secondary_url, setting_type, resource_identifier,
&setting_info);
ContentSetting setting =
content_settings::ValueToContentSetting(value.get());

// TODO(bbondy): Add a static RegisterUserPrefs method for shields and use
// prefs instead of simply returning true / false below.
if (setting == CONTENT_SETTING_DEFAULT) {
return GetDefaultFromResourceIdentifier(resource_identifier, primary_url,
secondary_url);
}
return setting == CONTENT_SETTING_ALLOW;
}

} // namespace

bool IsAllowContentSettingFromIO(const net::URLRequest* request,
const GURL& primary_url, const GURL& secondary_url,
ContentSettingsType setting_type,
Expand All @@ -66,31 +94,34 @@ bool IsAllowContentSettingFromIO(const net::URLRequest* request,
secondary_url, setting_type, resource_identifier);
}

bool IsAllowContentSettingsForProfile(Profile* profile,
const GURL& primary_url,
const GURL& secondary_url,
ContentSettingsType setting_type,
const std::string& resource_identifier) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(profile);
return IsAllowContentSetting(
HostContentSettingsMapFactory::GetForProfile(profile),
primary_url,
secondary_url,
setting_type,
resource_identifier);
}

bool IsAllowContentSettingWithIOData(ProfileIOData* io_data,
const GURL& primary_url, const GURL& secondary_url,
ContentSettingsType setting_type,
const std::string& resource_identifier) {

if (!io_data) {
return GetDefaultFromResourceIdentifier(resource_identifier, primary_url,
secondary_url);
}
content_settings::SettingInfo setting_info;
std::unique_ptr<base::Value> value =
io_data->GetHostContentSettingsMap()->GetWebsiteSetting(
primary_url, secondary_url,
setting_type,
resource_identifier, &setting_info);
ContentSetting setting =
content_settings::ValueToContentSetting(value.get());

// TODO(bbondy): Add a static RegisterUserPrefs method for shields and use
// prefs instead of simply returning true / false below.
if (setting == CONTENT_SETTING_DEFAULT) {
return GetDefaultFromResourceIdentifier(resource_identifier, primary_url,
secondary_url);
}
return setting == CONTENT_SETTING_ALLOW;
return IsAllowContentSetting(io_data->GetHostContentSettingsMap(),
primary_url,
secondary_url,
setting_type,
resource_identifier);
}

void GetRenderFrameInfo(const URLRequest* request,
Expand Down
7 changes: 7 additions & 0 deletions components/brave_shields/browser/brave_shields_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ struct Referrer;
}

class GURL;
class Profile;
class ProfileIOData;

namespace brave_shields {
Expand All @@ -29,6 +30,12 @@ bool IsAllowContentSettingWithIOData(ProfileIOData* io_data,
ContentSettingsType setting_type,
const std::string& resource_identifier);

bool IsAllowContentSettingsForProfile(Profile* profile,
const GURL& primary_url,
const GURL& secondary_url,
ContentSettingsType setting_type,
const std::string& resource_identifier);

bool IsAllowContentSettingFromIO(const net::URLRequest* request,
const GURL& primary_url, const GURL& secondary_url,
ContentSettingsType setting_type,
Expand Down
16 changes: 16 additions & 0 deletions patches/content-browser-frame_host-navigation_request.cc.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index 3687817b179bdc1966d136d497bef2196dcc881e..9adb66951a8ce5769b6ecdfe5625671975ceff20 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -1405,6 +1405,11 @@ void NavigationRequest::OnStartChecksComplete(
frame_tree_node_, begin_params_.get(), &report_raw_headers);
RenderFrameDevToolsAgentHost::OnNavigationRequestWillBeSent(*this);

+ GetContentClient()->browser()->MaybeHideReferrer(browser_context,
+ common_params_.url,
+ top_document_url,
+ &common_params_.referrer);
+
loader_ = NavigationURLLoader::Create(
browser_context->GetResourceContext(), partition,
std::make_unique<NavigationRequestInfo>(
18 changes: 18 additions & 0 deletions patches/content-public-browser-content_browser_client.h.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index b33545b865e4677ee2e9a909d259df171c12d985..a5d8d152dbeaf53d452c66129b47453e4ae2d20a 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -1364,6 +1364,13 @@ class CONTENT_EXPORT ContentBrowserClient {
// perform additional checks, such as requiring --user-data-dir flag too to
// make sure that insecure contents will not persist accidentally.
virtual bool CanIgnoreCertificateErrorIfNeeded();
+
+ // Brave-speicific: allows the embedder to modify the referrer string
+ // according to user preferences.
+ virtual void MaybeHideReferrer(BrowserContext* browser_context,
+ const GURL& request_url,
+ const GURL& document_url,
+ Referrer* referrer) {}
};

} // namespace content