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 #1259

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 @@ -30,6 +30,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 @@ -250,6 +251,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 @@ -60,6 +64,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 e7ca1318e3c0989b6c87cec874ca0d9328374a7e..90d5f2dbfc01400e9c4e9844c29b8226f2499836 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -1398,6 +1398,11 @@ void NavigationRequest::OnStartChecksComplete(
frame_tree_node_, begin_params_.get(), &report_raw_headers);
devtools_instrumentation::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 ff83421f2bb8268ccc32d95ed9d2b4e7715c8d7b..cb23ffa115d155490ad1ddaccb4b008781761830 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -1430,6 +1430,13 @@ class CONTENT_EXPORT ContentBrowserClient {
content::PreviewsState initial_state,
content::NavigationHandle* navigation_handle,
const net::HttpResponseHeaders* response_headers);
+
+ // 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