Commit e6567ba9 authored by asanka's avatar asanka Committed by Commit bot

[downloads] Set initiator when handling downloads via a[download]

Downloads initiated via clicking on an anchor with a 'download'
attribute should behave the same way with regard to cookie handling as a
download initiated via a top level navigation.

This CL adds logic to set the initiator origin for URLRequests initiated
in response to such downloads so that the handling of SameSite cookies
follows suit with top level navigations.

R=mkwst@chromium.org, clamy@chromium.org
BUG=648043
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2561903002
Cr-Commit-Position: refs/heads/master@{#439137}
parent d857418d
...@@ -50,6 +50,7 @@ ...@@ -50,6 +50,7 @@
#include "content/shell/browser/shell_download_manager_delegate.h" #include "content/shell/browser/shell_download_manager_delegate.h"
#include "content/shell/browser/shell_network_delegate.h" #include "content/shell/browser/shell_network_delegate.h"
#include "device/power_save_blocker/power_save_blocker.h" #include "device/power_save_blocker/power_save_blocker.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h" #include "net/test/embedded_test_server/http_response.h"
...@@ -2484,6 +2485,85 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeBlobURL) { ...@@ -2484,6 +2485,85 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeBlobURL) {
download->GetTargetFilePath().BaseName().value().c_str()); download->GetTargetFilePath().BaseName().value().c_str());
} }
IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeSameSiteCookie) {
base::ThreadRestrictions::ScopedAllowIO allow_io_during_test;
const std::string kOriginOne = "one.example";
const std::string kOriginTwo = "two.example";
ASSERT_TRUE(embedded_test_server()->Start());
const std::string real_host = embedded_test_server()->host_port_pair().host();
host_resolver()->AddRule(kOriginOne, real_host);
host_resolver()->AddRule(kOriginTwo, real_host);
GURL echo_cookie_url =
embedded_test_server()->GetURL(kOriginOne, "/echoheader?cookie");
// download-attribute-same-site-cookie sets two cookies. One "A=B" is set with
// SameSite=Strict. The other one "B=C" doesn't have this flag. In general
// a[download] should behave the same as a top level navigation.
//
// The page then simulates a click on an <a download> link whose target is the
// /echoheader handler on the same origin.
DownloadItem* download = StartDownloadAndReturnItem(
shell(),
embedded_test_server()->GetURL(
kOriginOne,
std::string(
"/download/download-attribute-same-site-cookie.html?target=") +
echo_cookie_url.spec()));
WaitForCompletion(download);
std::string file_contents;
ASSERT_TRUE(
base::ReadFileToString(download->GetTargetFilePath(), &file_contents));
// Initiator and target are same-origin. Both cookies should have been
// included in the request.
EXPECT_STREQ("A=B; B=C", file_contents.c_str());
// The test isn't complete without verifying that the initiator isn't being
// incorrectly set to be the same as the resource origin. The
// download-attribute test page doesn't set any cookies but creates a download
// via a <a download> link to the target URL. In this case:
//
// Initiator origin: kOriginTwo
// Resource origin: kOriginOne
// First-party origin: kOriginOne
download = StartDownloadAndReturnItem(
shell(),
embedded_test_server()->GetURL(
kOriginTwo, std::string("/download/download-attribute.html?target=") +
echo_cookie_url.spec()));
WaitForCompletion(download);
ASSERT_TRUE(
base::ReadFileToString(download->GetTargetFilePath(), &file_contents));
// The initiator and the target are not same-origin. Only the second cookie
// should be sent along with the request.
EXPECT_STREQ("B=C", file_contents.c_str());
// OriginOne redirects through OriginTwo.
//
// Initiator origin: kOriginOne
// Resource origin: kOriginOne
// First-party origin: kOriginOne
GURL redirect_url = embedded_test_server()->GetURL(
kOriginTwo, std::string("/server-redirect?") + echo_cookie_url.spec());
download = StartDownloadAndReturnItem(
shell(),
embedded_test_server()->GetURL(
kOriginOne, std::string("/download/download-attribute.html?target=") +
redirect_url.spec()));
WaitForCompletion(download);
ASSERT_TRUE(
base::ReadFileToString(download->GetTargetFilePath(), &file_contents));
EXPECT_STREQ("A=B; B=C", file_contents.c_str());
}
// The file empty.bin is served with a MIME type of application/octet-stream. // The file empty.bin is served with a MIME type of application/octet-stream.
// The content body is empty. Make sure this case is handled properly and we // The content body is empty. Make sure this case is handled properly and we
// don't regress on http://crbug.com/320394. // don't regress on http://crbug.com/320394.
......
...@@ -181,6 +181,14 @@ std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread( ...@@ -181,6 +181,14 @@ std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread(
"If-Range", has_etag ? params->etag() : params->last_modified(), true); "If-Range", has_etag ? params->etag() : params->last_modified(), true);
} }
// Downloads are treated as top level navigations. Hence the first-party
// origin for cookies is always based on the target URL and is updated on
// redirects.
request->set_first_party_for_cookies(params->url());
request->set_first_party_url_policy(
net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
request->set_initiator(params->initiator());
for (const auto& header : params->request_headers()) for (const auto& header : params->request_headers())
request->SetExtraRequestHeaderByName(header.first, header.second, request->SetExtraRequestHeaderByName(header.first, header.second,
false /*overwrite*/); false /*overwrite*/);
......
...@@ -270,6 +270,7 @@ void RenderFrameMessageFilter::DownloadUrl(int render_view_id, ...@@ -270,6 +270,7 @@ void RenderFrameMessageFilter::DownloadUrl(int render_view_id,
int render_frame_id, int render_frame_id,
const GURL& url, const GURL& url,
const Referrer& referrer, const Referrer& referrer,
const url::Origin& initiator,
const base::string16& suggested_name, const base::string16& suggested_name,
const bool use_prompt) const { const bool use_prompt) const {
if (!resource_context_) if (!resource_context_)
...@@ -282,6 +283,7 @@ void RenderFrameMessageFilter::DownloadUrl(int render_view_id, ...@@ -282,6 +283,7 @@ void RenderFrameMessageFilter::DownloadUrl(int render_view_id,
parameters->set_suggested_name(suggested_name); parameters->set_suggested_name(suggested_name);
parameters->set_prompt(use_prompt); parameters->set_prompt(use_prompt);
parameters->set_referrer(referrer); parameters->set_referrer(referrer);
parameters->set_initiator(initiator);
if (url.SchemeIsBlob()) { if (url.SchemeIsBlob()) {
ChromeBlobStorageContext* blob_context = ChromeBlobStorageContext* blob_context =
...@@ -342,12 +344,9 @@ void RenderFrameMessageFilter::CheckPolicyForCookies( ...@@ -342,12 +344,9 @@ void RenderFrameMessageFilter::CheckPolicyForCookies(
} }
void RenderFrameMessageFilter::OnDownloadUrl( void RenderFrameMessageFilter::OnDownloadUrl(
int render_view_id, const FrameHostMsg_DownloadUrl_Params& params) {
int render_frame_id, DownloadUrl(params.render_view_id, params.render_frame_id, params.url,
const GURL& url, params.referrer, params.initiator_origin, params.suggested_name,
const Referrer& referrer,
const base::string16& suggested_name) {
DownloadUrl(render_view_id, render_frame_id, url, referrer, suggested_name,
false); false);
} }
...@@ -364,7 +363,7 @@ void RenderFrameMessageFilter::OnSaveImageFromDataURL( ...@@ -364,7 +363,7 @@ void RenderFrameMessageFilter::OnSaveImageFromDataURL(
return; return;
DownloadUrl(render_view_id, render_frame_id, data_url, Referrer(), DownloadUrl(render_view_id, render_frame_id, data_url, Referrer(),
base::string16(), true); url::Origin(), base::string16(), true);
} }
void RenderFrameMessageFilter::OnAre3DAPIsBlocked(int render_frame_id, void RenderFrameMessageFilter::OnAre3DAPIsBlocked(int render_frame_id,
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#endif #endif
struct FrameHostMsg_CreateChildFrame_Params; struct FrameHostMsg_CreateChildFrame_Params;
struct FrameHostMsg_DownloadUrl_Params;
class GURL; class GURL;
namespace net { namespace net {
...@@ -71,6 +72,7 @@ class CONTENT_EXPORT RenderFrameMessageFilter ...@@ -71,6 +72,7 @@ class CONTENT_EXPORT RenderFrameMessageFilter
int render_frame_id, int render_frame_id,
const GURL& url, const GURL& url,
const Referrer& referrer, const Referrer& referrer,
const url::Origin& initiator,
const base::string16& suggested_name, const base::string16& suggested_name,
const bool use_prompt) const; const bool use_prompt) const;
...@@ -97,11 +99,8 @@ class CONTENT_EXPORT RenderFrameMessageFilter ...@@ -97,11 +99,8 @@ class CONTENT_EXPORT RenderFrameMessageFilter
const GetCookiesCallback& callback, const GetCookiesCallback& callback,
const net::CookieList& cookie_list); const net::CookieList& cookie_list);
void OnDownloadUrl(int render_view_id, void OnDownloadUrl(const FrameHostMsg_DownloadUrl_Params& params);
int render_frame_id,
const GURL& url,
const Referrer& referrer,
const base::string16& suggested_name);
void OnSaveImageFromDataURL(int render_view_id, void OnSaveImageFromDataURL(int render_view_id,
int render_frame_id, int render_frame_id,
const std::string& url_str); const std::string& url_str);
......
...@@ -283,6 +283,7 @@ class TestSaveImageFromDataURL : public RenderFrameMessageFilter { ...@@ -283,6 +283,7 @@ class TestSaveImageFromDataURL : public RenderFrameMessageFilter {
int render_frame_id, int render_frame_id,
const GURL& url, const GURL& url,
const Referrer& referrer, const Referrer& referrer,
const url::Origin& initiator,
const base::string16& suggested_name, const base::string16& suggested_name,
const bool use_prompt) const override { const bool use_prompt) const override {
url_string_ = url.spec(); url_string_ = url.spec();
......
...@@ -37,6 +37,7 @@ ...@@ -37,6 +37,7 @@
#include "content/public/common/javascript_message_type.h" #include "content/public/common/javascript_message_type.h"
#include "content/public/common/page_importance_signals.h" #include "content/public/common/page_importance_signals.h"
#include "content/public/common/page_state.h" #include "content/public/common/page_state.h"
#include "content/public/common/referrer.h"
#include "content/public/common/resource_response.h" #include "content/public/common/resource_response.h"
#include "content/public/common/stop_find_action.h" #include "content/public/common/stop_find_action.h"
#include "content/public/common/three_d_api_types.h" #include "content/public/common/three_d_api_types.h"
...@@ -423,6 +424,15 @@ IPC_STRUCT_BEGIN(FrameHostMsg_OpenURL_Params) ...@@ -423,6 +424,15 @@ IPC_STRUCT_BEGIN(FrameHostMsg_OpenURL_Params)
IPC_STRUCT_MEMBER(bool, is_history_navigation_in_new_child) IPC_STRUCT_MEMBER(bool, is_history_navigation_in_new_child)
IPC_STRUCT_END() IPC_STRUCT_END()
IPC_STRUCT_BEGIN(FrameHostMsg_DownloadUrl_Params)
IPC_STRUCT_MEMBER(int, render_view_id)
IPC_STRUCT_MEMBER(int, render_frame_id)
IPC_STRUCT_MEMBER(GURL, url)
IPC_STRUCT_MEMBER(content::Referrer, referrer)
IPC_STRUCT_MEMBER(url::Origin, initiator_origin)
IPC_STRUCT_MEMBER(base::string16, suggested_name)
IPC_STRUCT_END()
IPC_STRUCT_BEGIN(FrameMsg_TextTrackSettings_Params) IPC_STRUCT_BEGIN(FrameMsg_TextTrackSettings_Params)
// Text tracks on/off state // Text tracks on/off state
IPC_STRUCT_MEMBER(bool, text_tracks_enabled) IPC_STRUCT_MEMBER(bool, text_tracks_enabled)
...@@ -1027,12 +1037,7 @@ IPC_MESSAGE_ROUTED1(FrameHostMsg_DidFinishLoad, ...@@ -1027,12 +1037,7 @@ IPC_MESSAGE_ROUTED1(FrameHostMsg_DidFinishLoad,
GURL /* validated_url */) GURL /* validated_url */)
// Initiates a download based on user actions like 'ALT+click'. // Initiates a download based on user actions like 'ALT+click'.
IPC_MESSAGE_CONTROL5(FrameHostMsg_DownloadUrl, IPC_MESSAGE_CONTROL(FrameHostMsg_DownloadUrl, FrameHostMsg_DownloadUrl_Params)
int /* render_view_id */,
int /* render_frame_id */,
GURL /* url */,
content::Referrer /* referrer */,
base::string16 /* suggested_name */)
// Asks the browser to save a image (for <canvas> or <img>) from a data URL. // Asks the browser to save a image (for <canvas> or <img>) from a data URL.
// Note: |data_url| is the contents of a data:URL, and that it's represented as // Note: |data_url| is the contents of a data:URL, and that it's represented as
......
...@@ -15,12 +15,14 @@ ...@@ -15,12 +15,14 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "content/public/browser/download_interrupt_reasons.h" #include "content/public/browser/download_interrupt_reasons.h"
#include "content/public/browser/download_save_info.h" #include "content/public/browser/download_save_info.h"
#include "content/public/common/referrer.h" #include "content/public/common/referrer.h"
#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_context_getter.h"
#include "storage/browser/blob/blob_data_handle.h" #include "storage/browser/blob/blob_data_handle.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h"
namespace content { namespace content {
...@@ -107,6 +109,12 @@ class CONTENT_EXPORT DownloadUrlParameters { ...@@ -107,6 +109,12 @@ class CONTENT_EXPORT DownloadUrlParameters {
referrer_encoding_ = referrer_encoding; referrer_encoding_ = referrer_encoding;
} }
// The origin of the context which initiated the request. See
// net::URLRequest::initiator().
void set_initiator(const base::Optional<url::Origin>& initiator) {
initiator_ = initiator;
}
// If this is a request for resuming an HTTP/S download, |last_modified| // If this is a request for resuming an HTTP/S download, |last_modified|
// should be the value of the last seen Last-Modified response header. // should be the value of the last seen Last-Modified response header.
void set_last_modified(const std::string& last_modified) { void set_last_modified(const std::string& last_modified) {
...@@ -210,6 +218,7 @@ class CONTENT_EXPORT DownloadUrlParameters { ...@@ -210,6 +218,7 @@ class CONTENT_EXPORT DownloadUrlParameters {
bool prefer_cache() const { return prefer_cache_; } bool prefer_cache() const { return prefer_cache_; }
const Referrer& referrer() const { return referrer_; } const Referrer& referrer() const { return referrer_; }
const std::string& referrer_encoding() const { return referrer_encoding_; } const std::string& referrer_encoding() const { return referrer_encoding_; }
const base::Optional<url::Origin>& initiator() const { return initiator_; }
// These will be -1 if the request is not associated with a frame. See // These will be -1 if the request is not associated with a frame. See
// the constructors for more. // the constructors for more.
...@@ -257,6 +266,7 @@ class CONTENT_EXPORT DownloadUrlParameters { ...@@ -257,6 +266,7 @@ class CONTENT_EXPORT DownloadUrlParameters {
int64_t post_id_; int64_t post_id_;
bool prefer_cache_; bool prefer_cache_;
Referrer referrer_; Referrer referrer_;
base::Optional<url::Origin> initiator_;
std::string referrer_encoding_; std::string referrer_encoding_;
int render_process_host_id_; int render_process_host_id_;
int render_view_host_routing_id_; int render_view_host_routing_id_;
......
...@@ -3232,9 +3232,15 @@ void RenderFrameImpl::loadURLExternally(const blink::WebURLRequest& request, ...@@ -3232,9 +3232,15 @@ void RenderFrameImpl::loadURLExternally(const blink::WebURLRequest& request,
bool should_replace_current_entry) { bool should_replace_current_entry) {
Referrer referrer(RenderViewImpl::GetReferrerFromRequest(frame_, request)); Referrer referrer(RenderViewImpl::GetReferrerFromRequest(frame_, request));
if (policy == blink::WebNavigationPolicyDownload) { if (policy == blink::WebNavigationPolicyDownload) {
Send(new FrameHostMsg_DownloadUrl(render_view_->GetRoutingID(), FrameHostMsg_DownloadUrl_Params params;
GetRoutingID(), request.url(), referrer, params.render_view_id = render_view_->GetRoutingID();
suggested_name)); params.render_frame_id = GetRoutingID();
params.url = request.url();
params.referrer = referrer;
params.initiator_origin = request.requestorOrigin();
params.suggested_name = suggested_name;
Send(new FrameHostMsg_DownloadUrl(params));
} else { } else {
OpenURL(request.url(), IsHttpPost(request), OpenURL(request.url(), IsHttpPost(request),
GetRequestBodyForWebURLRequest(request), GetRequestBodyForWebURLRequest(request),
......
<!DOCTYPE html>
<html>
<body>
<a download="suggested-filename" href="">link</a>
<script>
var anchorElement = document.querySelector('a[download]');
url = window.location.href;
anchorElement.href = url.substr(url.indexOf('=') + 1);
anchorElement.click();
</script>
</body>
</html>
HTTP/1.1 200 OK
Content-Type: text/html
Set-Cookie: A=B ; SameSite=Strict ; Path=/ ; Max-Age=3600
Set-Cookie: B=C ; Path=/ ; Max-Age=3600
...@@ -422,6 +422,7 @@ void HTMLAnchorElement::handleClick(Event* event) { ...@@ -422,6 +422,7 @@ void HTMLAnchorElement::handleClick(Event* event) {
document().getSecurityOrigin()->canRequest(completedURL); document().getSecurityOrigin()->canRequest(completedURL);
const AtomicString& suggestedName = const AtomicString& suggestedName =
(isSameOrigin ? fastGetAttribute(downloadAttr) : nullAtom); (isSameOrigin ? fastGetAttribute(downloadAttr) : nullAtom);
request.setRequestorOrigin(SecurityOrigin::create(document().url()));
frame->loader().client()->loadURLExternally( frame->loader().client()->loadURLExternally(
request, NavigationPolicyDownload, suggestedName, false); request, NavigationPolicyDownload, suggestedName, false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment