Commit 748d6b12 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Support download error response body in content.

This CL adds the logic to download HTTP error response body in content.

Doc: go/chrome-download-non-successful-server-response

Bug: 765778
Change-Id: I367a9678502de8aace3208776b0dcca51e26c764
Reviewed-on: https://chromium-review.googlesource.com/669300Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505190}
parent 264a447d
......@@ -487,6 +487,7 @@ net::EmbeddedTestServer::HandleRequestCallback CreateRedirectHandler(
std::unique_ptr<net::test_server::HttpResponse>
HandleRequestAndSendBasicResponse(
const std::string& relative_url,
net::HttpStatusCode code,
const base::StringPairs& headers,
const std::string& content_type,
const std::string& body,
......@@ -498,6 +499,7 @@ HandleRequestAndSendBasicResponse(
response->AddCustomHeader(pair.first, pair.second);
response->set_content_type(content_type);
response->set_content(body);
response->set_code(code);
}
return std::move(response);
}
......@@ -506,11 +508,12 @@ HandleRequestAndSendBasicResponse(
// HTTP 200 status code, a Content-Type header and a body.
net::EmbeddedTestServer::HandleRequestCallback CreateBasicResponseHandler(
const std::string& relative_url,
net::HttpStatusCode code,
const base::StringPairs& headers,
const std::string& content_type,
const std::string& body) {
return base::Bind(&HandleRequestAndSendBasicResponse, relative_url, headers,
content_type, body);
return base::Bind(&HandleRequestAndSendBasicResponse, relative_url, code,
headers, content_type, body);
}
// Helper class to "flatten" handling of
......@@ -2321,8 +2324,9 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, ReferrerForHTTPS) {
net::EmbeddedTestServer::Type::TYPE_HTTPS);
net::EmbeddedTestServer http_origin(net::EmbeddedTestServer::Type::TYPE_HTTP);
https_origin.ServeFilesFromDirectory(GetTestFilePath("download", ""));
http_origin.RegisterRequestHandler(CreateBasicResponseHandler(
"/download", base::StringPairs(), "application/octet-stream", "Hello"));
http_origin.RegisterRequestHandler(
CreateBasicResponseHandler("/download", net::HTTP_OK, base::StringPairs(),
"application/octet-stream", "Hello"));
ASSERT_TRUE(https_origin.InitializeAndListen());
ASSERT_TRUE(http_origin.InitializeAndListen());
......@@ -2358,7 +2362,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, CookiePolicy) {
cookie_header.push_back(
std::make_pair(std::string("Set-Cookie"), std::string("A=B")));
origin_one.RegisterRequestHandler(CreateBasicResponseHandler(
"/foo", cookie_header, "application/octet-stream", "abcd"));
"/foo", net::HTTP_OK, cookie_header, "application/octet-stream", "abcd"));
ASSERT_TRUE(origin_one.Start());
origin_two.RegisterRequestHandler(
......@@ -2419,8 +2423,9 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
CreateRedirectHandler("/ping", origin_two.GetURL("/download")));
origin_one.StartAcceptingConnections();
origin_two.RegisterRequestHandler(CreateBasicResponseHandler(
"/download", base::StringPairs(), "application/octet-stream", "Hello"));
origin_two.RegisterRequestHandler(
CreateBasicResponseHandler("/download", net::HTTP_OK, base::StringPairs(),
"application/octet-stream", "Hello"));
origin_two.StartAcceptingConnections();
NavigateToURLAndWaitForDownload(
......@@ -2468,8 +2473,9 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
CreateRedirectHandler("/ping", origin_two.GetURL("/pong")));
origin_two.RegisterRequestHandler(
CreateRedirectHandler("/pong", origin_one.GetURL("/download")));
origin_one.RegisterRequestHandler(CreateBasicResponseHandler(
"/download", base::StringPairs(), "application/octet-stream", "Hello"));
origin_one.RegisterRequestHandler(
CreateBasicResponseHandler("/download", net::HTTP_OK, base::StringPairs(),
"application/octet-stream", "Hello"));
origin_one.StartAcceptingConnections();
origin_two.StartAcceptingConnections();
......@@ -2846,4 +2852,93 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadIgnoresXFO) {
downloads[0]->GetTargetFilePath().BaseName().value());
}
// Verify that the response body of non-successful server response can be
// downloaded to a file, when |fetch_error_body| sets to true.
IN_PROC_BROWSER_TEST_F(DownloadContentTest, FetchErrorResponseBody) {
net::EmbeddedTestServer server;
const std::string kNotFoundURL = "/404notfound";
const std::string kNotFoundResponseBody = "This is response body.";
server.RegisterRequestHandler(CreateBasicResponseHandler(
kNotFoundURL, net::HTTP_NOT_FOUND, base::StringPairs(), "text/html",
kNotFoundResponseBody));
ASSERT_TRUE(server.Start());
GURL url = server.GetURL(kNotFoundURL);
std::unique_ptr<DownloadUrlParameters> download_parameters(
DownloadUrlParameters::CreateForWebContentsMainFrame(
shell()->web_contents(), url, TRAFFIC_ANNOTATION_FOR_TESTS));
// Fetch non-successful response body.
download_parameters->set_fetch_error_body(true);
DownloadManager* download_manager = DownloadManagerForShell(shell());
std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(shell(), 1));
download_manager->DownloadUrl(std::move(download_parameters));
observer->WaitForFinished();
std::vector<DownloadItem*> items;
download_manager->GetAllDownloads(&items);
EXPECT_EQ(1u, items.size());
// Verify the error response body in the file.
{
base::ScopedAllowBlockingForTesting allow_blocking;
std::string file_content;
ASSERT_TRUE(
base::ReadFileToString(items[0]->GetTargetFilePath(), &file_content));
EXPECT_EQ(kNotFoundResponseBody, file_content);
}
}
// Verify the case that the first response is HTTP 200, and then interrupted,
// and the second response is HTTP 404, the response body of 404 should be
// fetched.
IN_PROC_BROWSER_TEST_F(DownloadContentTest, FetchErrorResponseBodyResumption) {
const std::string kNotFoundResponseBody = "This is 404 response body.";
GURL url;
TestDownloadRequestHandler server;
TestDownloadRequestHandler::Parameters server_params =
TestDownloadRequestHandler::Parameters::WithSingleInterruption();
url = server.url();
server.StartServing(server_params);
// Wait for an interrupted download.
std::unique_ptr<DownloadUrlParameters> download_parameters(
DownloadUrlParameters::CreateForWebContentsMainFrame(
shell()->web_contents(), url, TRAFFIC_ANNOTATION_FOR_TESTS));
download_parameters->set_fetch_error_body(true);
DownloadManager* download_manager = DownloadManagerForShell(shell());
std::unique_ptr<DownloadTestObserver> observer;
observer.reset(new content::DownloadTestObserverInterrupted(
download_manager, 1,
content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
download_manager->DownloadUrl(std::move(download_parameters));
observer->WaitForFinished();
std::vector<DownloadItem*> items;
download_manager->GetAllDownloads(&items);
EXPECT_EQ(1u, items.size());
// Now server will start to response 404 with empty body.
const std::string k404ResponseHeader = "HTTP/1.1 404 Not found\r\n\r\n";
server.StartServingStaticResponse(k404ResponseHeader);
DownloadItem* download = items[0];
// The fetch error body should be cached in download item. The download should
// start from beginning.
download->Resume();
WaitForCompletion(download);
// The file should be empty.
{
base::ScopedAllowBlockingForTesting allow_blocking;
std::string file_content;
ASSERT_TRUE(
base::ReadFileToString(items[0]->GetTargetFilePath(), &file_content));
EXPECT_EQ(std::string(), file_content);
}
}
} // namespace content
......@@ -144,6 +144,10 @@ struct CONTENT_EXPORT DownloadCreateInfo {
// The HTTP request method.
std::string method;
// Whether the download should fetch the response body for non successful HTTP
// response.
bool fetch_error_body = false;
private:
DISALLOW_COPY_AND_ASSIGN(DownloadCreateInfo);
};
......
......@@ -302,6 +302,7 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
etag_(info.etag),
net_log_(net_log),
is_updating_observers_(false),
fetch_error_body_(info.fetch_error_body),
weak_ptr_factory_(this) {
delegate_->Attach();
Init(true /* actively downloading */, SRC_ACTIVE_DOWNLOAD);
......@@ -2201,6 +2202,7 @@ void DownloadItemImpl::ResumeInterruptedDownload(
download_params->set_etag(GetETag());
download_params->set_hash_of_partial_file(GetHash());
download_params->set_hash_state(std::move(hash_state_));
download_params->set_fetch_error_body(fetch_error_body_);
// Note that resumed downloads disallow redirects. Hence the referrer URL
// (which is the contents of the Referer header for the last download request)
......
......@@ -759,6 +759,10 @@ class CONTENT_EXPORT DownloadItemImpl
// Check whether the download item is updating its observers.
bool is_updating_observers_;
// Whether the download should fetch the response body for non successful HTTP
// response.
bool fetch_error_body_ = false;
base::WeakPtrFactory<DownloadItemImpl> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(DownloadItemImpl);
......
......@@ -65,6 +65,7 @@ class DownloadRequestData : public base::SupportsUserData::Data {
uint32_t download_id() const { return download_id_; }
std::string guid() const { return guid_; }
bool is_transient() const { return transient_; }
bool fetch_error_body() const { return fetch_error_body_; }
const DownloadUrlParameters::OnStartedCallback& callback() const {
return on_started_callback_;
}
......@@ -75,6 +76,7 @@ class DownloadRequestData : public base::SupportsUserData::Data {
std::unique_ptr<DownloadSaveInfo> save_info_;
uint32_t download_id_ = DownloadItem::kInvalidId;
std::string guid_;
bool fetch_error_body_ = false;
bool transient_ = false;
DownloadUrlParameters::OnStartedCallback on_started_callback_;
};
......@@ -91,6 +93,7 @@ void DownloadRequestData::Attach(net::URLRequest* request,
new DownloadSaveInfo(parameters->GetSaveInfo()));
request_data->download_id_ = download_id;
request_data->guid_ = parameters->guid();
request_data->fetch_error_body_ = parameters->fetch_error_body();
request_data->transient_ = parameters->is_transient();
request_data->on_started_callback_ = parameters->callback();
request->SetUserData(&kKey, std::move(request_data));
......@@ -131,6 +134,7 @@ DownloadRequestCore::DownloadRequestCore(net::URLRequest* request,
: delegate_(delegate),
request_(request),
download_id_(DownloadItem::kInvalidId),
fetch_error_body_(false),
transient_(false),
bytes_read_(0),
pause_count_(0),
......@@ -165,6 +169,7 @@ DownloadRequestCore::DownloadRequestCore(net::URLRequest* request,
save_info_ = request_data->TakeSaveInfo();
download_id_ = request_data->download_id();
guid_ = request_data->guid();
fetch_error_body_ = request_data->fetch_error_body();
transient_ = request_data->is_transient();
on_started_callback_ = request_data->callback();
DownloadRequestData::Detach(request_);
......@@ -200,6 +205,7 @@ DownloadRequestCore::CreateDownloadCreateInfo(DownloadInterruptReason result) {
create_info->transient = transient_;
create_info->response_headers = request()->response_headers();
create_info->offset = create_info->save_info->offset;
create_info->fetch_error_body = fetch_error_body_;
return create_info;
}
......@@ -212,7 +218,7 @@ bool DownloadRequestCore::OnResponseStarted(
DownloadInterruptReason result =
request()->response_headers()
? HandleSuccessfulServerResponse(*request()->response_headers(),
save_info_.get())
save_info_.get(), fetch_error_body_)
: DOWNLOAD_INTERRUPT_REASON_NONE;
if (request()->response_headers()) {
......
......@@ -126,6 +126,7 @@ class CONTENT_EXPORT DownloadRequestCore
std::unique_ptr<DownloadSaveInfo> save_info_;
uint32_t download_id_;
std::string guid_;
bool fetch_error_body_;
bool transient_;
DownloadUrlParameters::OnStartedCallback on_started_callback_;
......
......@@ -50,7 +50,8 @@ DownloadResponseHandler::DownloadResponseHandler(
Delegate* delegate,
std::unique_ptr<DownloadSaveInfo> save_info,
bool is_parallel_request,
bool is_transient)
bool is_transient,
bool fetch_error_body)
: delegate_(delegate),
started_(false),
save_info_(std::move(save_info)),
......@@ -58,6 +59,7 @@ DownloadResponseHandler::DownloadResponseHandler(
method_(resource_request->method),
referrer_(resource_request->referrer),
is_transient_(is_transient),
fetch_error_body_(fetch_error_body),
has_strong_validators_(false) {
if (!is_parallel_request)
RecordDownloadCount(UNTHROTTLED_COUNT);
......@@ -109,9 +111,10 @@ DownloadResponseHandler::CreateDownloadCreateInfo(
base::Time::Now(), net::NetLogWithSource(), std::move(save_info_));
DownloadInterruptReason result =
head.headers ? HandleSuccessfulServerResponse(
*head.headers, create_info->save_info.get())
: DOWNLOAD_INTERRUPT_REASON_NONE;
head.headers
? HandleSuccessfulServerResponse(
*head.headers, create_info->save_info.get(), fetch_error_body_)
: DOWNLOAD_INTERRUPT_REASON_NONE;
create_info->result = result;
if (result == DOWNLOAD_INTERRUPT_REASON_NONE)
......
......@@ -38,7 +38,8 @@ class DownloadResponseHandler : public mojom::URLLoaderClient {
Delegate* delegate,
std::unique_ptr<DownloadSaveInfo> save_info,
bool is_parallel_request,
bool is_transient);
bool is_transient,
bool fetch_error_body);
~DownloadResponseHandler() override;
// mojom::URLLoaderClient
......@@ -77,6 +78,7 @@ class DownloadResponseHandler : public mojom::URLLoaderClient {
std::string method_;
GURL referrer_;
bool is_transient_;
bool fetch_error_body_;
net::CertStatus cert_status_;
bool has_strong_validators_;
GURL origin_;
......
......@@ -274,7 +274,9 @@ CreateURLRequestOnIOThread(DownloadUrlParameters* params) {
DownloadInterruptReason HandleSuccessfulServerResponse(
const net::HttpResponseHeaders& http_headers,
DownloadSaveInfo* save_info) {
DownloadSaveInfo* save_info,
bool fetch_error_body) {
DownloadInterruptReason result = DOWNLOAD_INTERRUPT_REASON_NONE;
switch (http_headers.response_code()) {
case -1: // Non-HTTP request.
case net::HTTP_OK:
......@@ -300,22 +302,22 @@ DownloadInterruptReason HandleSuccessfulServerResponse(
// resource not being found since there is no entity to download.
case net::HTTP_NOT_FOUND:
return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
result = DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
break;
case net::HTTP_REQUESTED_RANGE_NOT_SATISFIABLE:
// Retry by downloading from the start automatically:
// If we haven't received data when we get this error, we won't.
return DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE;
result = DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE;
break;
case net::HTTP_UNAUTHORIZED:
case net::HTTP_PROXY_AUTHENTICATION_REQUIRED:
// Server didn't authorize this request.
return DOWNLOAD_INTERRUPT_REASON_SERVER_UNAUTHORIZED;
result = DOWNLOAD_INTERRUPT_REASON_SERVER_UNAUTHORIZED;
break;
case net::HTTP_FORBIDDEN:
// Server forbids access to this resource.
return DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN;
result = DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN;
break;
default: // All other errors.
// Redirection and informational codes should have been handled earlier
......@@ -325,20 +327,26 @@ DownloadInterruptReason HandleSuccessfulServerResponse(
// This will change extensions::api::download::InterruptReason.
DCHECK_NE(3, http_headers.response_code() / 100);
DCHECK_NE(1, http_headers.response_code() / 100);
return DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED;
result = DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED;
}
if (result != DOWNLOAD_INTERRUPT_REASON_NONE && !fetch_error_body)
return result;
// The caller is expecting a partial response.
if (save_info && (save_info->offset > 0 || save_info->length > 0)) {
if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) {
// Server should send partial content when "If-Match" or
// "If-Unmodified-Since" check passes, and the range request header has
// last byte position. e.g. "Range:bytes=50-99".
if (save_info->length != DownloadSaveInfo::kLengthFullContent)
if (save_info->length != DownloadSaveInfo::kLengthFullContent &&
!fetch_error_body)
return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
// Requested a partial range, but received the entire response, when
// the range request header is "Range:bytes={offset}-".
// The response can be HTTP 200 or other error code when
// |fetch_error_body| is true.
save_info->offset = 0;
save_info->hash_of_partial_file.clear();
save_info->hash_state.reset();
......
......@@ -35,9 +35,13 @@ std::unique_ptr<ResourceRequest> CONTENT_EXPORT CreateResourceRequest(
std::unique_ptr<net::URLRequest> CONTENT_EXPORT CreateURLRequestOnIOThread(
DownloadUrlParameters* params);
// Parse the HTTP server response code.
// If |fetch_error_body| is true, most of HTTP response codes will be accepted
// as successful response.
DownloadInterruptReason CONTENT_EXPORT
HandleSuccessfulServerResponse(const net::HttpResponseHeaders& http_headers,
DownloadSaveInfo* save_info);
DownloadSaveInfo* save_info,
bool fetch_error_body);
// Parse response headers and update |create_info| accordingly.
CONTENT_EXPORT void HandleResponseHeaders(
......
......@@ -59,7 +59,8 @@ std::unique_ptr<ResourceDownloader> ResourceDownloader::BeginDownload(
auto downloader = base::MakeUnique<ResourceDownloader>(
delegate, std::move(request),
base::MakeUnique<DownloadSaveInfo>(params->GetSaveInfo()), download_id,
params->guid(), is_parallel_request, params->is_transient());
params->guid(), is_parallel_request, params->is_transient(),
params->fetch_error_body());
downloader->Start(factory, file_system_context, std::move(params));
return downloader;
}
......@@ -77,7 +78,7 @@ ResourceDownloader::InterceptNavigationResponse(
auto downloader = base::MakeUnique<ResourceDownloader>(
delegate, std::move(resource_request),
base::MakeUnique<DownloadSaveInfo>(), content::DownloadItem::kInvalidId,
std::string(), false, false);
std::string(), false, false, false);
downloader->InterceptResponse(std::move(url_loader), response,
std::move(consumer_handle), ssl_status,
std::move(completion_status));
......@@ -91,14 +92,16 @@ ResourceDownloader::ResourceDownloader(
uint32_t download_id,
std::string guid,
bool is_parallel_request,
bool is_transient)
bool is_transient,
bool fetch_error_body)
: delegate_(delegate),
resource_request_(std::move(resource_request)),
response_handler_(resource_request_.get(),
this,
std::move(save_info),
is_parallel_request,
is_transient),
is_transient,
fetch_error_body),
blob_client_binding_(&response_handler_),
download_id_(download_id),
guid_(guid),
......
......@@ -47,7 +47,8 @@ class ResourceDownloader : public UrlDownloadHandler,
uint32_t download_id,
std::string guid,
bool is_parallel_request,
bool is_transient);
bool is_transient,
bool fetch_error_body);
ResourceDownloader(base::WeakPtr<UrlDownloadHandler::Delegate> delegate,
std::unique_ptr<ThrottlingURLLoader> url_loader);
~ResourceDownloader() override;
......
......@@ -45,6 +45,7 @@ DownloadUrlParameters::DownloadUrlParameters(
url_request_context_getter_(url_request_context_getter),
url_(url),
do_not_prompt_for_login_(false),
fetch_error_body_(false),
transient_(false),
traffic_annotation_(traffic_annotation) {}
......
......@@ -209,6 +209,12 @@ class CONTENT_EXPORT DownloadUrlParameters {
do_not_prompt_for_login_ = do_not_prompt;
}
// Sets whether to download the response body even if the server returns
// non-successful HTTP response code, like "HTTP NOT FOUND".
void set_fetch_error_body(bool fetch_error_body) {
fetch_error_body_ = fetch_error_body;
}
// Sets whether the download is to be treated as transient. A transient
// download is short-lived and is not shown in the UI.
void set_transient(bool transient) { transient_ = transient; }
......@@ -270,6 +276,7 @@ class CONTENT_EXPORT DownloadUrlParameters {
bool prompt() const { return save_info_.prompt_for_save_location; }
const GURL& url() const { return url_; }
bool do_not_prompt_for_login() const { return do_not_prompt_for_login_; }
bool fetch_error_body() const { return fetch_error_body_; }
bool is_transient() const { return transient_; }
std::string guid() const { return guid_; }
......@@ -307,6 +314,7 @@ class CONTENT_EXPORT DownloadUrlParameters {
DownloadSaveInfo save_info_;
GURL url_;
bool do_not_prompt_for_login_;
bool fetch_error_body_;
bool transient_;
std::string guid_;
std::unique_ptr<storage::BlobDataHandle> blob_data_handle_;
......
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