Skip to content

Commit

Permalink
Revert of Apply Referrer-Policy header when following redirects (patc…
Browse files Browse the repository at this point in the history
…hset chromium#17 id:320001 of https://codereview.chromium.org/2100583002/ )

Reason for revert:
Broke msan tests https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests/builds/17218/steps/net_unittests%20on%20Ubuntu-12.04/logs/URLRequestJob.RedirectTransactionWithReferrerPolicyHeader

Original issue's description:
> Apply Referrer-Policy header when following redirects
>
> When a Referrer-Policy header is received during a redirect,
> URLRequestJob parses it and updates the referrer and referrer policy on
> the request, if necessary.
>
> The Referrer-Policy header is being implemented as an experimental web
> platform feature. The experimental web platform feature flag is plumbed
> to URLRequestJob via a boolean on URLRequestContext. This flag should be
> temporary and only live until the Referrer-Policy feature ships.
>
> Intent to Implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Umj9iVRJM70
> Implementation plan: https://docs.google.com/document/d/1SyQhP6Y7BHIQXWL8S1saWqMuar4hoWLGigQuHmizg3g/edit
>
> BUG=619228
>
> Committed: https://crrev.com/550ba7f9533922cfeac9709d99815cec9b2ad52a
> Cr-Commit-Position: refs/heads/master@{#403017}

TBR=jochen@chromium.org,eugenebut@chromium.org,mmenke@chromium.org,palmer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=619228

Review-Url: https://codereview.chromium.org/2108423002
Cr-Commit-Position: refs/heads/master@{#403075}
  • Loading branch information
estark authored and Commit bot committed Jun 30, 2016
1 parent ee62ba6 commit ede2091
Show file tree
Hide file tree
Showing 19 changed files with 21 additions and 410 deletions.
6 changes: 0 additions & 6 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
#include "content/public/browser/host_zoom_map.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/resource_context.h"
#include "content/public/common/content_switches.h"
#include "net/base/keygen_handler.h"
#include "net/cert/cert_verifier.h"
#include "net/cert/ct_log_verifier.h"
Expand Down Expand Up @@ -1030,11 +1029,6 @@ void ProfileIOData::Init(

main_request_context_->set_enable_brotli(io_thread_globals->enable_brotli);

// TODO(estark): Remove this once the Referrer-Policy header is no
// longer an experimental feature. https://crbug.com/619228
main_request_context_->set_enable_referrer_policy_header(
command_line.HasSwitch(switches::kEnableExperimentalWebPlatformFeatures));

std::unique_ptr<ChromeNetworkDelegate> network_delegate(
new ChromeNetworkDelegate(
#if defined(ENABLE_EXTENSIONS)
Expand Down
4 changes: 0 additions & 4 deletions content/common/resource_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,12 @@ IPC_STRUCT_TRAITS_BEGIN(content::ResourceResponseInfo)
IPC_STRUCT_TRAITS_MEMBER(cors_exposed_header_names)
IPC_STRUCT_TRAITS_END()

IPC_ENUM_TRAITS_MAX_VALUE(net::URLRequest::ReferrerPolicy,
net::URLRequest::MAX_REFERRER_POLICY - 1)

IPC_STRUCT_TRAITS_BEGIN(net::RedirectInfo)
IPC_STRUCT_TRAITS_MEMBER(status_code)
IPC_STRUCT_TRAITS_MEMBER(new_method)
IPC_STRUCT_TRAITS_MEMBER(new_url)
IPC_STRUCT_TRAITS_MEMBER(new_first_party_for_cookies)
IPC_STRUCT_TRAITS_MEMBER(new_referrer)
IPC_STRUCT_TRAITS_MEMBER(new_referrer_policy)
IPC_STRUCT_TRAITS_MEMBER(referred_token_binding_host)
IPC_STRUCT_TRAITS_END()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,4 @@ LayoutTestURLRequestContextGetter::GetProxyService() {
return net::ProxyService::CreateDirect();
}

bool LayoutTestURLRequestContextGetter::ShouldEnableReferrerPolicyHeader() {
return true;
}

} // namespace content
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class LayoutTestURLRequestContextGetter : public ShellURLRequestContextGetter {
std::unique_ptr<net::NetworkDelegate> CreateNetworkDelegate() override;
std::unique_ptr<net::ProxyConfigService> GetProxyConfigService() override;
std::unique_ptr<net::ProxyService> GetProxyService() override;
bool ShouldEnableReferrerPolicyHeader() override;

private:
DISALLOW_COPY_AND_ASSIGN(LayoutTestURLRequestContextGetter);
Expand Down
9 changes: 0 additions & 9 deletions content/shell/browser/shell_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@ ShellURLRequestContextGetter::GetProxyService() {
std::move(proxy_config_service_), 0, url_request_context_->net_log());
}

bool ShellURLRequestContextGetter::ShouldEnableReferrerPolicyHeader() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableExperimentalWebPlatformFeatures);
}

net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);

Expand All @@ -130,10 +125,6 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
url_request_context_->set_net_log(net_log_);
network_delegate_ = CreateNetworkDelegate();
url_request_context_->set_network_delegate(network_delegate_.get());
// TODO(estark): Remove this once the Referrer-Policy header is no
// longer an experimental feature. https://crbug.com/619228
url_request_context_->set_enable_referrer_policy_header(
ShouldEnableReferrerPolicyHeader());
storage_.reset(
new net::URLRequestContextStorage(url_request_context_.get()));
storage_->set_cookie_store(CreateCookieStore(CookieStoreConfig()));
Expand Down
4 changes: 0 additions & 4 deletions content/shell/browser/shell_url_request_context_getter.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ class ShellURLRequestContextGetter : public net::URLRequestContextGetter {
virtual std::unique_ptr<net::ProxyConfigService> GetProxyConfigService();
virtual std::unique_ptr<net::ProxyService> GetProxyService();

// TODO(estark): Remove this once the Referrer-Policy header is no
// longer an experimental feature. https://crbug.com/619228
virtual bool ShouldEnableReferrerPolicyHeader();

private:
bool ignore_certificate_errors_;
base::FilePath base_path_;
Expand Down
9 changes: 0 additions & 9 deletions ios/web/public/referrer_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,6 @@ TEST(ReferrerUtilTest, PolicyForNavigation) {
EXPECT_TRUE(policy == ReferrerPolicyDefault ||
policy == ReferrerPolicyNoReferrerWhenDowngrade);
break;
case net::URLRequest::ORIGIN:
EXPECT_TRUE(policy == ReferrerPolicyOrigin);
break;
case net::URLRequest::NO_REFERRER:
EXPECT_TRUE(policy == ReferrerPolicyNever);
break;
case net::URLRequest::MAX_REFERRER_POLICY:
FAIL();
break;
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions net/url_request/redirect_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@

namespace net {

RedirectInfo::RedirectInfo()
: status_code(-1),
new_referrer_policy(
URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE) {}
RedirectInfo::RedirectInfo() : status_code(-1) {}

RedirectInfo::RedirectInfo(const RedirectInfo& other) = default;

Expand Down
5 changes: 0 additions & 5 deletions net/url_request/redirect_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <string>

#include "net/base/net_export.h"
#include "net/url_request/url_request.h"
#include "url/gurl.h"

namespace net {
Expand Down Expand Up @@ -39,10 +38,6 @@ struct NET_EXPORT RedirectInfo {
// The new HTTP referrer header.
std::string new_referrer;

// The new referrer policy that should be obeyed if there are
// subsequent redirects.
URLRequest::ReferrerPolicy new_referrer_policy;

// The hostname of the referrer if it asked the client to include a referred
// Token Binding when following the redirect; otherwise this is the empty
// string.
Expand Down
10 changes: 0 additions & 10 deletions net/url_request/url_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -479,15 +479,6 @@ void URLRequest::SetReferrer(const std::string& referrer) {

void URLRequest::set_referrer_policy(ReferrerPolicy referrer_policy) {
DCHECK(!is_pending_);
// External callers shouldn't be setting NO_REFERRER or
// ORIGIN. |referrer_policy_| is only applied during server redirects,
// so external callers must set the referrer themselves using
// SetReferrer() for the initial request. Once the referrer has been
// set to an origin or to an empty string, there is no point in
// setting the policy to NO_REFERRER or ORIGIN as it would have the
// same effect as using NEVER_CLEAR_REFERRER across redirects.
DCHECK_NE(referrer_policy, NO_REFERRER);
DCHECK_NE(referrer_policy, ORIGIN);
referrer_policy_ = referrer_policy;
}

Expand Down Expand Up @@ -987,7 +978,6 @@ int URLRequest::Redirect(const RedirectInfo& redirect_info) {
}

referrer_ = redirect_info.new_referrer;
referrer_policy_ = redirect_info.new_referrer_policy;
first_party_for_cookies_ = redirect_info.new_first_party_for_cookies;
token_binding_referrer_ = redirect_info.referred_token_binding_host;

Expand Down
36 changes: 16 additions & 20 deletions net/url_request/url_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,31 +81,27 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
NetworkDelegate* network_delegate,
const std::string& scheme);

// A ReferrerPolicy for the request can be set with
// set_referrer_policy() and controls the contents of the Referer
// header when URLRequest follows server redirects.
// Referrer policies (see set_referrer_policy): During server redirects, the
// referrer header might be cleared, if the protocol changes from HTTPS to
// HTTP. This is the default behavior of URLRequest, corresponding to
// CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE. Alternatively, the
// referrer policy can be set to strip the referrer down to an origin upon
// cross-origin navigation (ORIGIN_ONLY_ON_TRANSITION_CROSS_ORIGIN), or
// never change the referrer header (NEVER_CLEAR_REFERRER). Embedders will
// want to use these options when implementing referrer policy support
// (https://w3c.github.io/webappsec/specs/referrer-policy/).
//
// REDUCE_REFERRER_GRANULARITY_ON_TRANSITION_CROSS_ORIGIN is a slight variant
// on CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE: If the request
// downgrades from HTTPS to HTTP, the referrer will be cleared. If the request
// transitions cross-origin (but does not downgrade), the referrer's
// granularity will be reduced (currently stripped down to an origin rather
// than a full URL). Same-origin requests will send the full referrer.
enum ReferrerPolicy {
// Clear the referrer header if the protocol changes from HTTPS to
// HTTP. This is the default behavior of URLRequest.
CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
// A slight variant on
// CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE: If the
// request downgrades from HTTPS to HTTP, the referrer will be
// cleared. If the request transitions cross-origin (but does not
// downgrade), the referrer's granularity will be reduced (currently
// stripped down to an origin rather than a full URL). Same-origin
// requests will send the full referrer.
REDUCE_REFERRER_GRANULARITY_ON_TRANSITION_CROSS_ORIGIN,
// Strip the referrer down to an origin upon cross-origin navigation.
ORIGIN_ONLY_ON_TRANSITION_CROSS_ORIGIN,
// Never change the referrer.
NEVER_CLEAR_REFERRER,
// Strip the referrer down to the origin regardless of the redirect
// location.
ORIGIN,
// Always clear the referrer regardless of the redirect location.
NO_REFERRER,
MAX_REFERRER_POLICY
};

// First-party URL redirect policy: During server redirects, the first-party
Expand Down
4 changes: 1 addition & 3 deletions net/url_request/url_request_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ URLRequestContext::URLRequestContext()
sdch_manager_(nullptr),
network_quality_estimator_(nullptr),
url_requests_(new std::set<const URLRequest*>),
enable_brotli_(false),
enable_referrer_policy_header_(false) {}
enable_brotli_(false) {}

URLRequestContext::~URLRequestContext() {
AssertNoURLRequests();
Expand Down Expand Up @@ -67,7 +66,6 @@ void URLRequestContext::CopyFrom(const URLRequestContext* other) {
set_http_user_agent_settings(other->http_user_agent_settings_);
set_network_quality_estimator(other->network_quality_estimator_);
set_enable_brotli(other->enable_brotli_);
set_enable_referrer_policy_header(other->enable_referrer_policy_header_);
}

const HttpNetworkSession::Params* URLRequestContext::GetNetworkSessionParams(
Expand Down
13 changes: 0 additions & 13 deletions net/url_request/url_request_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,6 @@ class NET_EXPORT URLRequestContext

bool enable_brotli() const { return enable_brotli_; }

void set_enable_referrer_policy_header(bool enable_referrer_policy_header) {
enable_referrer_policy_header_ = enable_referrer_policy_header;
}

bool enable_referrer_policy_header() const {
return enable_referrer_policy_header_;
}

private:
// ---------------------------------------------------------------------------
// Important: When adding any new members below, consider whether they need to
Expand Down Expand Up @@ -277,11 +269,6 @@ class NET_EXPORT URLRequestContext
// Enables Brotli Content-Encoding support.
bool enable_brotli_;

// Enables parsing and applying the Referrer-Policy header when
// following redirects. TODO(estark): remove this flag once
// Referrer-Policy ships (https://crbug.com/619228).
bool enable_referrer_policy_header_;

DISALLOW_COPY_AND_ASSIGN(URLRequestContext);
};

Expand Down
72 changes: 3 additions & 69 deletions net/url_request/url_request_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/profiler/scoped_tracker.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/values.h"
Expand Down Expand Up @@ -63,56 +62,6 @@ std::string ComputeMethodForRedirect(const std::string& method,
return method;
}

// A redirect response can contain a Referrer-Policy header
// (https://w3c.github.io/webappsec-referrer-policy/). This function
// checks for a Referrer-Policy header, and parses it if
// present. Returns the referrer policy that should be used for the
// request.
URLRequest::ReferrerPolicy ProcessReferrerPolicyHeaderOnRedirect(
URLRequest* request) {
URLRequest::ReferrerPolicy new_policy = request->referrer_policy();

std::string referrer_policy_header;
request->GetResponseHeaderByName("Referrer-Policy", &referrer_policy_header);
std::vector<std::string> policy_tokens =
base::SplitString(referrer_policy_header, ",", base::TRIM_WHITESPACE,
base::SPLIT_WANT_NONEMPTY);

for (const auto& token : policy_tokens) {
if (base::CompareCaseInsensitiveASCII(token, "never") == 0 ||
base::CompareCaseInsensitiveASCII(token, "no-referrer") == 0) {
new_policy = URLRequest::NO_REFERRER;
continue;
}

if (base::CompareCaseInsensitiveASCII(token, "default") == 0 ||
base::CompareCaseInsensitiveASCII(token,
"no-referrer-when-downgrade") == 0) {
new_policy =
URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE;
continue;
}

if (base::CompareCaseInsensitiveASCII(token, "origin") == 0) {
new_policy = URLRequest::ORIGIN;
continue;
}

if (base::CompareCaseInsensitiveASCII(token, "origin-when-cross-origin") ==
0) {
new_policy = URLRequest::ORIGIN_ONLY_ON_TRANSITION_CROSS_ORIGIN;
continue;
}

if (base::CompareCaseInsensitiveASCII(token, "always") == 0 ||
base::CompareCaseInsensitiveASCII(token, "unsafe-url") == 0) {
new_policy = URLRequest::NEVER_CLEAR_REFERRER;
continue;
}
}
return new_policy;
}

} // namespace

URLRequestJob::URLRequestJob(URLRequest* request,
Expand Down Expand Up @@ -395,13 +344,6 @@ GURL URLRequestJob::ComputeReferrerForRedirect(

case URLRequest::NEVER_CLEAR_REFERRER:
return original_referrer;
case URLRequest::ORIGIN:
return original_referrer.GetOrigin();
case URLRequest::NO_REFERRER:
return GURL();
case URLRequest::MAX_REFERRER_POLICY:
NOTREACHED();
return GURL();
}

NOTREACHED();
Expand Down Expand Up @@ -456,7 +398,6 @@ void URLRequestJob::NotifyHeadersComplete() {

GURL new_location;
int http_status_code;

if (IsRedirectResponse(&new_location, &http_status_code)) {
// Redirect response bodies are not read. Notify the transaction
// so it does not treat being stopped as an error.
Expand Down Expand Up @@ -1003,18 +944,11 @@ RedirectInfo URLRequestJob::ComputeRedirectInfo(const GURL& location,
request_->first_party_for_cookies();
}

if (request_->context()->enable_referrer_policy_header()) {
redirect_info.new_referrer_policy =
ProcessReferrerPolicyHeaderOnRedirect(request_);
} else {
redirect_info.new_referrer_policy = request_->referrer_policy();
}

// Alter the referrer if redirecting cross-origin (especially HTTP->HTTPS).
redirect_info.new_referrer =
ComputeReferrerForRedirect(redirect_info.new_referrer_policy,
request_->referrer(), redirect_info.new_url)
.spec();
ComputeReferrerForRedirect(request_->referrer_policy(),
request_->referrer(),
redirect_info.new_url).spec();

std::string include_referer;
request_->GetResponseHeaderByName("include-referer-token-binding-id",
Expand Down
Loading

0 comments on commit ede2091

Please sign in to comment.