Commit 8010592e authored by Takashi Sakamoto's avatar Takashi Sakamoto Committed by Commit Bot

Revert "Report the number of files within ZIP file to SafeBrowsing"

This reverts commit e55866bf.

Reason for revert: suspect non_single_process_mash_unit_tests failing on chromium.memory/Linux ChromiumOS MSan Tests

E.g.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/11404
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8920951203278062672/+/steps/non_single_process_mash_unit_tests/0/logs/FileAnalyzerTest.TypeInvalidZip__status_CRASH_/0


Original change's description:
> Report the number of files within ZIP file to SafeBrowsing
> 
> This CL adds code to count the number of files and directories within a
> ZIP archive and add it to download pings.
> 
> Bug: 926586
> Change-Id: I6cc48f6e02f7f5a8d99271f21bb7b0330070812c
> Reviewed-on: https://chromium-review.googlesource.com/c/1479585
> Reviewed-by: Varun Khaneja <vakh@chromium.org>
> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
> Commit-Queue: Daniel Rubery <drubery@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#633969}

TBR=meacer@chromium.org,vakh@chromium.org,drubery@chromium.org

Change-Id: I19d08a66ae7b020e3999fb262df2cac9d74e6f39
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 926586
Reviewed-on: https://chromium-review.googlesource.com/c/1480360Reviewed-by: default avatarTakashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#634069}
parent 00b91cbc
...@@ -445,8 +445,6 @@ void CheckClientDownloadRequest::OnFileFeatureExtractionDone( ...@@ -445,8 +445,6 @@ void CheckClientDownloadRequest::OnFileFeatureExtractionDone(
signature_info_ = results.signature_info; signature_info_ = results.signature_info;
image_headers_.reset(new ClientDownloadRequest_ImageHeaders()); image_headers_.reset(new ClientDownloadRequest_ImageHeaders());
*image_headers_ = results.image_headers; *image_headers_ = results.image_headers;
file_count_ = results.file_count;
directory_count_ = results.directory_count;
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
if (!results.disk_image_signature.empty()) if (!results.disk_image_signature.empty())
...@@ -702,8 +700,6 @@ void CheckClientDownloadRequest::SendRequest() { ...@@ -702,8 +700,6 @@ void CheckClientDownloadRequest::SendRequest() {
request->set_allocated_image_headers(image_headers_.release()); request->set_allocated_image_headers(image_headers_.release());
if (!archived_binaries_.empty()) if (!archived_binaries_.empty())
request->mutable_archived_binary()->Swap(&archived_binaries_); request->mutable_archived_binary()->Swap(&archived_binaries_);
request->set_archive_file_count(file_count_);
request->set_archive_directory_count(directory_count_);
if (!request->SerializeToString(&client_download_request_data_)) { if (!request->SerializeToString(&client_download_request_data_)) {
FinishRequest(DownloadCheckResult::UNKNOWN, REASON_INVALID_REQUEST_PROTO); FinishRequest(DownloadCheckResult::UNKNOWN, REASON_INVALID_REQUEST_PROTO);
return; return;
......
...@@ -134,8 +134,6 @@ class CheckClientDownloadRequest : public download::DownloadItem::Observer { ...@@ -134,8 +134,6 @@ class CheckClientDownloadRequest : public download::DownloadItem::Observer {
bool is_extended_reporting_; bool is_extended_reporting_;
bool is_incognito_; bool is_incognito_;
bool is_under_advanced_protection_; bool is_under_advanced_protection_;
int file_count_;
int directory_count_;
base::WeakPtrFactory<CheckClientDownloadRequest> weakptr_factory_; base::WeakPtrFactory<CheckClientDownloadRequest> weakptr_factory_;
......
...@@ -69,7 +69,7 @@ FileAnalyzer::Results ExtractFileFeatures( ...@@ -69,7 +69,7 @@ FileAnalyzer::Results ExtractFileFeatures(
} // namespace } // namespace
FileAnalyzer::Results::Results() : file_count(0), directory_count(0) {} FileAnalyzer::Results::Results() {}
FileAnalyzer::Results::~Results() {} FileAnalyzer::Results::~Results() {}
FileAnalyzer::Results::Results(const FileAnalyzer::Results& other) = default; FileAnalyzer::Results::Results(const FileAnalyzer::Results& other) = default;
...@@ -208,9 +208,6 @@ void FileAnalyzer::OnZipAnalysisFinished( ...@@ -208,9 +208,6 @@ void FileAnalyzer::OnZipAnalysisFinished(
results_.type = ClientDownloadRequest::ZIPPED_EXECUTABLE; results_.type = ClientDownloadRequest::ZIPPED_EXECUTABLE;
} }
results_.file_count = archive_results.file_count;
results_.directory_count = archive_results.directory_count;
std::move(callback_).Run(std::move(results_)); std::move(callback_).Run(std::move(results_));
} }
......
...@@ -72,12 +72,6 @@ class FileAnalyzer { ...@@ -72,12 +72,6 @@ class FileAnalyzer {
ClientDownloadRequest::DetachedCodeSignature> ClientDownloadRequest::DetachedCodeSignature>
detached_code_signatures; detached_code_signatures;
#endif #endif
// For archive files, the number of contained files.
int file_count;
// For archive files, the number of contained directories.
int directory_count;
}; };
explicit FileAnalyzer( explicit FileAnalyzer(
......
...@@ -768,65 +768,4 @@ TEST_F(FileAnalyzerTest, LargeRarSkipsContentInspection) { ...@@ -768,65 +768,4 @@ TEST_F(FileAnalyzerTest, LargeRarSkipsContentInspection) {
EXPECT_TRUE(result_.archived_binaries.Get(0).digests().sha256().empty()); EXPECT_TRUE(result_.archived_binaries.Get(0).digests().sha256().empty());
} }
TEST_F(FileAnalyzerTest, ZipFilesGetFileCount) {
scoped_refptr<MockBinaryFeatureExtractor> extractor =
new testing::StrictMock<MockBinaryFeatureExtractor>();
FileAnalyzer analyzer(extractor);
base::RunLoop run_loop;
base::FilePath target_path(FILE_PATH_LITERAL("target.zip"));
base::FilePath tmp_path =
temp_dir_.GetPath().Append(FILE_PATH_LITERAL("tmp.crdownload"));
base::ScopedTempDir zip_source_dir;
ASSERT_TRUE(zip_source_dir.CreateUniqueTempDir());
std::string file_contents = "dummy file";
ASSERT_EQ(static_cast<int>(file_contents.size()),
base::WriteFile(
zip_source_dir.GetPath().Append(FILE_PATH_LITERAL("file.exe")),
file_contents.data(), file_contents.size()));
ASSERT_TRUE(zip::Zip(zip_source_dir.GetPath(), tmp_path,
/* include_hidden_files= */
false));
analyzer.Start(
target_path, tmp_path,
base::BindOnce(&FileAnalyzerTest::DoneCallback, base::Unretained(this),
run_loop.QuitClosure()));
run_loop.Run();
ASSERT_TRUE(has_result_);
EXPECT_EQ(1, result_.file_count);
EXPECT_EQ(0, result_.directory_count);
}
TEST_F(FileAnalyzerTest, ZipFilesGetDirectoryCount) {
scoped_refptr<MockBinaryFeatureExtractor> extractor =
new testing::StrictMock<MockBinaryFeatureExtractor>();
FileAnalyzer analyzer(extractor);
base::RunLoop run_loop;
base::FilePath target_path(FILE_PATH_LITERAL("target.zip"));
base::FilePath tmp_path =
temp_dir_.GetPath().Append(FILE_PATH_LITERAL("tmp.crdownload"));
base::ScopedTempDir zip_source_dir;
ASSERT_TRUE(zip_source_dir.CreateUniqueTempDir());
ASSERT_TRUE(base::CreateDirectory(
zip_source_dir.GetPath().Append(FILE_PATH_LITERAL("direcotry"))));
ASSERT_TRUE(zip::Zip(zip_source_dir.GetPath(), tmp_path,
/* include_hidden_files= */
false));
analyzer.Start(
target_path, tmp_path,
base::BindOnce(&FileAnalyzerTest::DoneCallback, base::Unretained(this),
run_loop.QuitClosure()));
run_loop.Run();
ASSERT_TRUE(has_result_);
EXPECT_EQ(0, result_.file_count);
EXPECT_EQ(1, result_.directory_count);
}
} // namespace safe_browsing } // namespace safe_browsing
...@@ -33,8 +33,6 @@ struct ArchiveAnalyzerResults { ...@@ -33,8 +33,6 @@ struct ArchiveAnalyzerResults {
ClientDownloadRequest_DetachedCodeSignature> ClientDownloadRequest_DetachedCodeSignature>
detached_code_signatures; detached_code_signatures;
#endif // OS_MACOSX #endif // OS_MACOSX
int file_count;
int directory_count;
ArchiveAnalyzerResults(); ArchiveAnalyzerResults();
ArchiveAnalyzerResults(const ArchiveAnalyzerResults& other); ArchiveAnalyzerResults(const ArchiveAnalyzerResults& other);
~ArchiveAnalyzerResults(); ~ArchiveAnalyzerResults();
......
...@@ -71,8 +71,6 @@ void AnalyzeZipFile(base::File zip_file, ...@@ -71,8 +71,6 @@ void AnalyzeZipFile(base::File zip_file,
bool contains_zip = false; bool contains_zip = false;
bool advanced = true; bool advanced = true;
int zip_entry_count = 0; int zip_entry_count = 0;
results->file_count = 0;
results->directory_count = 0;
for (; reader.HasMore(); advanced = reader.AdvanceToNextEntry()) { for (; reader.HasMore(); advanced = reader.AdvanceToNextEntry()) {
if (!advanced) { if (!advanced) {
DVLOG(1) << "Could not advance to next entry, aborting zip scan."; DVLOG(1) << "Could not advance to next entry, aborting zip scan.";
...@@ -97,11 +95,6 @@ void AnalyzeZipFile(base::File zip_file, ...@@ -97,11 +95,6 @@ void AnalyzeZipFile(base::File zip_file,
FILE_PATH_LITERAL(".zip")) FILE_PATH_LITERAL(".zip"))
contains_zip = true; contains_zip = true;
zip_entry_count++; zip_entry_count++;
if (reader.current_entry_info()->is_directory())
results->directory_count++;
else
results->file_count++;
} }
results->success = true; results->success = true;
......
...@@ -3,8 +3,6 @@ ...@@ -3,8 +3,6 @@
// found in the LICENSE file. // found in the LICENSE file.
// Multiply-included param traits file, so no include guard. // Multiply-included param traits file, so no include guard.
// Disabling the presubmit warning with:
// no-include-guard-because-multiply-included
#if !defined(FULL_SAFE_BROWSING) #if !defined(FULL_SAFE_BROWSING)
#error FULL_SAFE_BROWSING should be defined. #error FULL_SAFE_BROWSING should be defined.
...@@ -105,6 +103,4 @@ IPC_STRUCT_TRAITS_BEGIN(safe_browsing::ArchiveAnalyzerResults) ...@@ -105,6 +103,4 @@ IPC_STRUCT_TRAITS_BEGIN(safe_browsing::ArchiveAnalyzerResults)
IPC_STRUCT_TRAITS_MEMBER(signature_blob) IPC_STRUCT_TRAITS_MEMBER(signature_blob)
IPC_STRUCT_TRAITS_MEMBER(detached_code_signatures) IPC_STRUCT_TRAITS_MEMBER(detached_code_signatures)
#endif // OS_MACOSX #endif // OS_MACOSX
IPC_STRUCT_TRAITS_MEMBER(file_count)
IPC_STRUCT_TRAITS_MEMBER(directory_count)
IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_END()
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