Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(191)

Unified Diff: chrome/browser/safe_browsing/download_protection_service.cc

Issue 2029903002: Add token field to ClientSafeBrowsingReportReqeust (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: tweak comments Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/safe_browsing/download_protection_service.cc
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index f5c82c7e83e35119809606e2c1fc8d6f008f74ce..d973f923059b0ff8a95ad176e0007cc2bfa8e6b7 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -279,7 +279,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
public:
CheckClientDownloadRequest(
content::DownloadItem* item,
- const CheckDownloadCallback& callback,
+ const CheckDownloadContentCallback& callback,
DownloadProtectionService* service,
const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
BinaryFeatureExtractor* binary_feature_extractor)
@@ -422,7 +422,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
// reference to this object. We'll eventually wind up in some method on
// the UI thread that will call FinishRequest() again. If FinishRequest()
// is called a second time, it will be a no-op.
- FinishRequest(UNKNOWN, REASON_REQUEST_CANCELED);
+ FinishRequest(UNKNOWN, REASON_REQUEST_CANCELED, base::EmptyString());
// Calling FinishRequest might delete this object, we may be deleted by
// this point.
}
@@ -451,6 +451,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
-source->GetStatus().error());
DownloadCheckResultReason reason = REASON_SERVER_PING_FAILED;
DownloadCheckResult result = UNKNOWN;
+ std::string token;
if (source->GetStatus().is_success() &&
net::HTTP_OK == source->GetResponseCode()) {
ClientDownloadResponse response;
@@ -475,16 +476,20 @@ class DownloadProtectionService::CheckClientDownloadRequest
} else if (response.verdict() == ClientDownloadResponse::DANGEROUS) {
reason = REASON_DOWNLOAD_DANGEROUS;
result = DANGEROUS;
+ token = response.has_token() ? response.token() : base::EmptyString();
} else if (response.verdict() == ClientDownloadResponse::UNCOMMON) {
reason = REASON_DOWNLOAD_UNCOMMON;
result = UNCOMMON;
+ token = response.has_token() ? response.token() : base::EmptyString();
} else if (response.verdict() == ClientDownloadResponse::DANGEROUS_HOST) {
reason = REASON_DOWNLOAD_DANGEROUS_HOST;
result = DANGEROUS_HOST;
+ token = response.has_token() ? response.token() : base::EmptyString();
} else if (
response.verdict() == ClientDownloadResponse::POTENTIALLY_UNWANTED) {
reason = REASON_DOWNLOAD_POTENTIALLY_UNWANTED;
result = POTENTIALLY_UNWANTED;
+ token = response.has_token() ? response.token() : base::EmptyString();
} else {
LOG(DFATAL) << "Unknown download response verdict: "
<< response.verdict();
@@ -501,7 +506,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
UMA_HISTOGRAM_TIMES("SBClientDownload.DownloadRequestNetworkDuration",
base::TimeTicks::Now() - request_start_time_);
- FinishRequest(result, reason);
+ FinishRequest(result, reason, token);
}
static bool IsSupportedDownload(const content::DownloadItem& item,
@@ -938,7 +943,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
if (archived_executable_)
request.mutable_archived_binary()->Swap(&archived_binary_);
if (!request.SerializeToString(&client_download_request_data_)) {
- FinishRequest(UNKNOWN, REASON_INVALID_REQUEST_PROTO);
+ FinishRequest(UNKNOWN, REASON_INVALID_REQUEST_PROTO, base::EmptyString());
return;
}
@@ -977,11 +982,12 @@ class DownloadProtectionService::CheckClientDownloadRequest
BrowserThread::UI,
FROM_HERE,
base::Bind(&CheckClientDownloadRequest::FinishRequest, this, result,
- reason));
+ reason, base::EmptyString()));
}
void FinishRequest(DownloadCheckResult result,
- DownloadCheckResultReason reason) {
+ DownloadCheckResultReason reason,
+ const std::string& token) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (finished_) {
return;
@@ -1012,7 +1018,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
reason,
REASON_MAX);
- callback_.Run(result);
+ callback_.Run(result, token);
item_->RemoveObserver(this);
item_ = NULL;
DownloadProtectionService* service = service_;
@@ -1021,7 +1027,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
// DownloadProtectionService::RequestFinished will decrement our refcount,
// so we may be deleted now.
} else {
- callback_.Run(UNKNOWN);
+ callback_.Run(UNKNOWN, base::EmptyString());
}
}
@@ -1087,7 +1093,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
std::unique_ptr<ClientDownloadRequest_ImageHeaders> image_headers_;
google::protobuf::RepeatedPtrField<ClientDownloadRequest_ArchivedBinary>
archived_binary_;
- CheckDownloadCallback callback_;
+ CheckDownloadContentCallback callback_;
// Will be NULL if the request has been canceled.
DownloadProtectionService* service_;
scoped_refptr<BinaryFeatureExtractor> binary_feature_extractor_;
@@ -1150,7 +1156,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
const GURL& requestor_url,
const base::FilePath& default_file_path,
const std::vector<base::FilePath::StringType>& alternate_extensions,
- const CheckDownloadCallback& callback,
+ const CheckDownloadContentCallback& callback,
DownloadProtectionService* service,
scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
: requestor_url_(requestor_url),
@@ -1166,7 +1172,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
~PPAPIDownloadRequest() override {
if (fetcher_ && !callback_.is_null())
- Finish(RequestOutcome::REQUEST_DESTROYED, UNKNOWN);
+ Finish(RequestOutcome::REQUEST_DESTROYED, UNKNOWN, base::EmptyString());
}
// Start the process of checking the download request. The callback passed as
@@ -1189,7 +1195,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
if (supported_path_.empty()) {
// Neither the default_file_path_ nor any path resulting of trying out
// |alternate_extensions_| are supported by SafeBrowsing.
- Finish(RequestOutcome::UNSUPPORTED_FILE_TYPE, SAFE);
+ Finish(RequestOutcome::UNSUPPORTED_FILE_TYPE, SAFE, base::EmptyString());
return;
}
@@ -1234,7 +1240,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
if (was_on_whitelist) {
// TODO(asanka): Should sample whitelisted downloads based on
// service_->whitelist_sample_rate(). http://crbug.com/610924
- Finish(RequestOutcome::WHITELIST_HIT, SAFE);
+ Finish(RequestOutcome::WHITELIST_HIT, SAFE, base::EmptyString());
return;
}
@@ -1271,7 +1277,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
if (!request.SerializeToString(&client_download_request_data_)) {
// More of an internal error than anything else. Note that the UNKNOWN
// verdict gets interpreted as "allowed".
- Finish(RequestOutcome::REQUEST_MALFORMED, UNKNOWN);
+ Finish(RequestOutcome::REQUEST_MALFORMED, UNKNOWN, base::EmptyString());
return;
}
@@ -1291,30 +1297,35 @@ class DownloadProtectionService::PPAPIDownloadRequest
if (!source->GetStatus().is_success() ||
net::HTTP_OK != source->GetResponseCode()) {
- Finish(RequestOutcome::FETCH_FAILED, UNKNOWN);
+ Finish(RequestOutcome::FETCH_FAILED, UNKNOWN, base::EmptyString());
return;
}
ClientDownloadResponse response;
std::string response_body;
+ std::string token;
bool got_data = source->GetResponseAsString(&response_body);
DCHECK(got_data);
if (response.ParseFromString(response_body)) {
+ token = response.has_token() ? response.token() : base::EmptyString();
Finish(RequestOutcome::SUCCEEDED,
- DownloadCheckResultFromClientDownloadResponse(response.verdict()));
+ DownloadCheckResultFromClientDownloadResponse(response.verdict()),
+ token);
} else {
- Finish(RequestOutcome::RESPONSE_MALFORMED, UNKNOWN);
+ Finish(RequestOutcome::RESPONSE_MALFORMED, UNKNOWN, base::EmptyString());
}
}
void OnRequestTimedOut() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DVLOG(2) << __FUNCTION__;
- Finish(RequestOutcome::TIMEDOUT, UNKNOWN);
+ Finish(RequestOutcome::TIMEDOUT, UNKNOWN, base::EmptyString());
}
- void Finish(RequestOutcome reason, DownloadCheckResult response) {
+ void Finish(RequestOutcome reason,
+ DownloadCheckResult response,
+ const std::string& token) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DVLOG(2) << __FUNCTION__ << " response: " << response;
UMA_HISTOGRAM_SPARSE_SLOWLY(
@@ -1325,7 +1336,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
UMA_HISTOGRAM_TIMES("SBClientDownload.PPAPIDownloadRequest.RequestDuration",
start_time_ - base::TimeTicks::Now());
if (!callback_.is_null())
- base::ResetAndReturn(&callback_).Run(response);
+ base::ResetAndReturn(&callback_).Run(response, token);
fetcher_.reset();
weakptr_factory_.InvalidateWeakPtrs();
service_->PPAPIDownloadCheckRequestFinished(this);
@@ -1385,7 +1396,7 @@ class DownloadProtectionService::PPAPIDownloadRequest
const std::vector<base::FilePath::StringType> alternate_extensions_;
// Callback to invoke with the result of the PPAPI download request check.
- CheckDownloadCallback callback_;
+ CheckDownloadContentCallback callback_;
DownloadProtectionService* service_;
const scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
@@ -1466,7 +1477,7 @@ bool DownloadProtectionService::IsHashManuallyBlacklisted(
void DownloadProtectionService::CheckClientDownload(
content::DownloadItem* item,
- const CheckDownloadCallback& callback) {
+ const CheckDownloadContentCallback& callback) {
scoped_refptr<CheckClientDownloadRequest> request(
new CheckClientDownloadRequest(item, callback, this,
database_manager_,
@@ -1503,7 +1514,7 @@ void DownloadProtectionService::CheckPPAPIDownloadRequest(
const GURL& requestor_url,
const base::FilePath& default_file_path,
const std::vector<base::FilePath::StringType>& alternate_extensions,
- const CheckDownloadCallback& callback) {
+ const CheckDownloadContentCallback& callback) {
DVLOG(1) << __FUNCTION__ << " url:" << requestor_url
<< " default_file_path:" << default_file_path.value();
std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest(

Powered by Google App Engine
This is Rietveld 408576698