From c8d8dd9ccfc96ad193191a1563242527e49240bb Mon Sep 17 00:00:00 2001 From: Olivier Robin Date: Tue, 2 Nov 2021 10:50:42 +0100 Subject: [PATCH] Add attachment support for Mac/iOS crash report database Copy the crash_report_database_generic implementation. bug: crashpad: 31 Change-Id: I582620ec8b22fecc7568d220c410c397948dfcb1 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3045405 Commit-Queue: Olivier Robin Reviewed-by: Joshua Peraza --- client/crash_report_database.cc | 109 +++++++++++++++++ client/crash_report_database.h | 34 +++++- client/crash_report_database_generic.cc | 154 ++---------------------- client/crash_report_database_mac.mm | 117 +++++++++++++++--- client/crash_report_database_test.cc | 42 ++++--- client/crash_report_database_win.cc | 153 ++++++----------------- client/prune_crash_reports_test.cc | 1 + util/file/filesystem.h | 16 +++ util/file/filesystem_posix.cc | 35 ++++++ util/file/filesystem_test.cc | 59 +++++++++ util/file/filesystem_win.cc | 37 ++++++ 11 files changed, 458 insertions(+), 299 deletions(-) diff --git a/client/crash_report_database.cc b/client/crash_report_database.cc index 68393333..5c8156e0 100644 --- a/client/crash_report_database.cc +++ b/client/crash_report_database.cc @@ -14,10 +14,29 @@ #include "client/crash_report_database.h" +#include + +#include "base/logging.h" +#include "base/strings/utf_string_conversions.h" #include "build/build_config.h" +#include "util/file/directory_reader.h" +#include "util/file/filesystem.h" namespace crashpad { +namespace { +constexpr base::FilePath::CharType kAttachmentsDirectory[] = + FILE_PATH_LITERAL("attachments"); + +bool AttachmentNameIsOK(const std::string& name) { + for (const char c : name) { + if (c != '_' && c != '-' && c != '.' && !isalnum(c)) + return false; + } + return true; +} +} // namespace + CrashReportDatabase::Report::Report() : uuid(), file_path(), @@ -73,6 +92,60 @@ FileReaderInterface* CrashReportDatabase::NewReport::Reader() { return reader_.get(); } +FileWriter* CrashReportDatabase::NewReport::AddAttachment( + const std::string& name) { + if (!AttachmentNameIsOK(name)) { + LOG(ERROR) << "invalid name for attachment " << name; + return nullptr; + } + base::FilePath report_attachments_dir = database_->AttachmentsPath(uuid_); + if (!LoggingCreateDirectory( + report_attachments_dir, FilePermissions::kOwnerOnly, true)) { + return nullptr; + } +#if defined(OS_WIN) + const std::wstring name_string = base::UTF8ToWide(name); +#else + const std::string name_string = name; +#endif + base::FilePath attachment_path = report_attachments_dir.Append(name_string); + auto writer = std::make_unique(); + if (!writer->Open(attachment_path, + FileWriteMode::kCreateOrFail, + FilePermissions::kOwnerOnly)) { + return nullptr; + } + attachment_writers_.emplace_back(std::move(writer)); + attachment_removers_.emplace_back(ScopedRemoveFile(attachment_path)); + return attachment_writers_.back().get(); +} + +void CrashReportDatabase::UploadReport::InitializeAttachments() { + base::FilePath report_attachments_dir = database_->AttachmentsPath(uuid); + DirectoryReader dir_reader; + if (!dir_reader.Open(report_attachments_dir)) { + return; + } + + base::FilePath filename; + DirectoryReader::Result dir_result; + while ((dir_result = dir_reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath filepath(report_attachments_dir.Append(filename)); + std::unique_ptr file_reader(std::make_unique()); + if (!file_reader->Open(filepath)) { + continue; + } + attachment_readers_.emplace_back(std::move(file_reader)); +#if defined(OS_WIN) + const std::string name_string = base::WideToUTF8(filename.value()); +#else + const std::string name_string = filename.value(); +#endif + attachment_map_[name_string] = attachment_readers_.back().get(); + } +} + CrashReportDatabase::UploadReport::UploadReport() : Report(), reader_(std::make_unique()), @@ -103,4 +176,40 @@ CrashReportDatabase::OperationStatus CrashReportDatabase::RecordUploadComplete( return RecordUploadAttempt(report, true, id); } +base::FilePath CrashReportDatabase::AttachmentsPath(const UUID& uuid) { +#if defined(OS_WIN) + const std::wstring uuid_string = uuid.ToWString(); +#else + const std::string uuid_string = uuid.ToString(); +#endif + + return DatabasePath().Append(kAttachmentsDirectory).Append(uuid_string); +} + +base::FilePath CrashReportDatabase::AttachmentsRootPath() { + return DatabasePath().Append(kAttachmentsDirectory); +} + +void CrashReportDatabase::RemoveAttachmentsByUUID(const UUID& uuid) { + base::FilePath report_attachment_dir = AttachmentsPath(uuid); + if (!IsDirectory(report_attachment_dir, /*allow_symlinks=*/false)) { + return; + } + DirectoryReader reader; + if (!reader.Open(report_attachment_dir)) { + return; + } + + base::FilePath filename; + DirectoryReader::Result result; + while ((result = reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath attachment_path( + report_attachment_dir.Append(filename)); + LoggingRemoveFile(attachment_path); + } + + LoggingRemoveDirectory(report_attachment_dir); +} + } // namespace crashpad diff --git a/client/crash_report_database.h b/client/crash_report_database.h index 19f1bf74..fea49853 100644 --- a/client/crash_report_database.h +++ b/client/crash_report_database.h @@ -129,8 +129,6 @@ class CrashReportDatabase { //! \brief Adds an attachment to the report. //! - //! \note This function is not yet implemented on macOS. - //! //! \param[in] name The key and name for the attachment, which will be //! included in the http upload. The attachment will not appear in the //! minidump report. \a name should only use characters from the set @@ -174,8 +172,6 @@ class CrashReportDatabase { //! \brief Obtains a mapping of names to file readers for any attachments //! for the report. - //! - //! This is not implemented on macOS. std::map GetAttachments() const { return attachment_map_; } @@ -400,9 +396,11 @@ class CrashReportDatabase { virtual OperationStatus RequestUpload(const UUID& uuid) = 0; //! \brief Cleans the database of expired lockfiles, metadata without report - //! files, and report files without metadata. + //! files, report files without metadata, and attachments without report + //! files. //! - //! This method does nothing on the macOS implementations of the database. + //! As the macOS implementation does not use lock or metadata files, the + //! cleaning is limited to attachments without report files. //! //! \param[in] lockfile_ttl The number of seconds at which lockfiles or new //! report files are considered expired. @@ -412,6 +410,30 @@ class CrashReportDatabase { protected: CrashReportDatabase() {} + //! \brief The path to the database passed to Initialize. + //! + //! \return The filepath of the database; + virtual base::FilePath DatabasePath() = 0; + + //! \brief Build a filepath for the root attachments directory. + //! + //! \return The filepath to the attachments directory. + base::FilePath AttachmentsRootPath(); + + //! \brief Build a filepath for the directory for the report to hold + //! attachments. + //! + //! \param[in] uuid The unique identifier for the crash report record. + //! + //! \return The filepath to the report attachments directory. + base::FilePath AttachmentsPath(const UUID& uuid); + + //! \brief Attempts to remove any attachments associated with the given + //! report UUID. There may not be any, so failing is not an error. + //! + //! \param[in] uuid The unique identifier for the crash report record. + void RemoveAttachmentsByUUID(const UUID& uuid); + private: //! \brief Adjusts a crash report record’s metadata to account for an upload //! attempt, and updates the last upload attempt time as returned by diff --git a/client/crash_report_database_generic.cc b/client/crash_report_database_generic.cc index 046e82d8..734a6c48 100644 --- a/client/crash_report_database_generic.cc +++ b/client/crash_report_database_generic.cc @@ -45,14 +45,6 @@ UUID UUIDFromReportPath(const base::FilePath& path) { return uuid; } -bool AttachmentNameIsOK(const std::string& name) { - for (const char c : name) { - if (c != '_' && c != '-' && c != '.' && !isalnum(c)) - return false; - } - return true; -} - using OperationStatus = CrashReportDatabase::OperationStatus; constexpr base::FilePath::CharType kSettings[] = @@ -70,8 +62,6 @@ constexpr base::FilePath::CharType kPendingDirectory[] = FILE_PATH_LITERAL("pending"); constexpr base::FilePath::CharType kCompletedDirectory[] = FILE_PATH_LITERAL("completed"); -constexpr base::FilePath::CharType kAttachmentsDirectory[] = - FILE_PATH_LITERAL("attachments"); constexpr const base::FilePath::CharType* kReportDirectories[] = { kNewDirectory, @@ -164,35 +154,6 @@ class ScopedLockFile { private: ScopedRemoveFile lock_file_; }; - -off_t GetFileSize(const base::FilePath& filepath) { - struct stat statbuf; - if (stat(filepath.value().c_str(), &statbuf) == 0) { - return statbuf.st_size; - } - PLOG(ERROR) << "stat " << filepath.value(); - return 0; -} - -void AddAttachmentSize(const base::FilePath& attachments_dir, uint64_t* size) { - // Early return if the attachment directory does not exist. - if (!IsDirectory(attachments_dir, /*allow_symlinks=*/false)) { - return; - } - DirectoryReader reader; - if (!reader.Open(attachments_dir)) { - return; - } - base::FilePath attachment_filename; - DirectoryReader::Result result; - while ((result = reader.NextFile(&attachment_filename)) == - DirectoryReader::Result::kSuccess) { - const base::FilePath attachment_filepath( - attachments_dir.Append(attachment_filename)); - *size += GetFileSize(attachment_filepath); - } -} - } // namespace class CrashReportDatabaseGeneric : public CrashReportDatabase { @@ -225,9 +186,7 @@ class CrashReportDatabaseGeneric : public CrashReportDatabase { OperationStatus DeleteReport(const UUID& uuid) override; OperationStatus RequestUpload(const UUID& uuid) override; int CleanDatabase(time_t lockfile_ttl) override; - - // Build a filepath for the directory for the report to hold attachments. - base::FilePath AttachmentsPath(const UUID& uuid); + base::FilePath DatabasePath() override; private: struct LockfileUploadReport : public UploadReport { @@ -286,10 +245,6 @@ class CrashReportDatabaseGeneric : public CrashReportDatabase { // Cleans any attachments that have no associated report in any state. void CleanOrphanedAttachments(); - // Attempt to remove any attachments associated with the given report UUID. - // There may not be any, so failing is not an error. - void RemoveAttachmentsByUUID(const UUID& uuid); - // Reads the metadata for a report from path and returns it in report. bool ReadMetadata(const base::FilePath& path, Report* report); @@ -307,62 +262,6 @@ class CrashReportDatabaseGeneric : public CrashReportDatabase { InitializationStateDcheck initialized_; }; -FileWriter* CrashReportDatabase::NewReport::AddAttachment( - const std::string& name) { - if (!AttachmentNameIsOK(name)) { - LOG(ERROR) << "invalid name for attachment " << name; - return nullptr; - } - - base::FilePath attachments_dir = - static_cast(database_)->AttachmentsPath( - uuid_); - if (!LoggingCreateDirectory( - attachments_dir, FilePermissions::kOwnerOnly, true)) { - return nullptr; - } - - base::FilePath path = attachments_dir.Append(name); - - auto writer = std::make_unique(); - if (!writer->Open( - path, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly)) { - LOG(ERROR) << "could not open " << path.value(); - return nullptr; - } - attachment_writers_.emplace_back(std::move(writer)); - attachment_removers_.emplace_back(ScopedRemoveFile(path)); - return attachment_writers_.back().get(); -} - -void CrashReportDatabase::UploadReport::InitializeAttachments() { - base::FilePath attachments_dir = - static_cast(database_)->AttachmentsPath( - uuid); - if (!IsDirectory(attachments_dir, /*allow_symlinks=*/false)) { - return; - } - DirectoryReader directory_reader; - if (!directory_reader.Open(attachments_dir)) { - return; - } - - base::FilePath filename; - DirectoryReader::Result dir_result; - while ((dir_result = directory_reader.NextFile(&filename)) == - DirectoryReader::Result::kSuccess) { - const base::FilePath filepath(attachments_dir.Append(filename)); - std::unique_ptr file_reader(std::make_unique()); - if (!file_reader->Open(filepath)) { - LOG(ERROR) << "attachment " << filepath.value() - << " couldn't be opened, skipping"; - continue; - } - attachment_readers_.emplace_back(std::move(file_reader)); - attachment_map_[filename.value()] = attachment_readers_.back().get(); - } -} - CrashReportDatabaseGeneric::CrashReportDatabaseGeneric() = default; CrashReportDatabaseGeneric::~CrashReportDatabaseGeneric() = default; @@ -385,9 +284,8 @@ bool CrashReportDatabaseGeneric::Initialize(const base::FilePath& path, } } - if (!LoggingCreateDirectory(base_dir_.Append(kAttachmentsDirectory), - FilePermissions::kOwnerOnly, - true)) { + if (!LoggingCreateDirectory( + AttachmentsRootPath(), FilePermissions::kOwnerOnly, true)) { return false; } @@ -413,6 +311,10 @@ CrashReportDatabase::InitializeWithoutCreating(const base::FilePath& path) { return database->Initialize(path, false) ? std::move(database) : nullptr; } +base::FilePath CrashReportDatabaseGeneric::DatabasePath() { + return base_dir_; +} + Settings* CrashReportDatabaseGeneric::GetSettings() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return &settings_; @@ -699,7 +601,7 @@ base::FilePath CrashReportDatabaseGeneric::ReportPath(const UUID& uuid, DCHECK_NE(state, kSearchable); #if defined(OS_WIN) - const std::wstring uuid_string = uuid.ToString16(); + const std::wstring uuid_string = uuid.ToWString(); #else const std::string uuid_string = uuid.ToString(); #endif @@ -708,16 +610,6 @@ base::FilePath CrashReportDatabaseGeneric::ReportPath(const UUID& uuid, .Append(uuid_string + kCrashReportExtension); } -base::FilePath CrashReportDatabaseGeneric::AttachmentsPath(const UUID& uuid) { -#if defined(OS_WIN) - const std::wstring uuid_string = uuid.ToString16(); -#else - const std::string uuid_string = uuid.ToString(); -#endif - - return base_dir_.Append(kAttachmentsDirectory).Append(uuid_string); -} - OperationStatus CrashReportDatabaseGeneric::LocateAndLockReport( const UUID& uuid, ReportState desired_state, @@ -878,7 +770,7 @@ int CrashReportDatabaseGeneric::CleanReportsInState(ReportState state, } void CrashReportDatabaseGeneric::CleanOrphanedAttachments() { - base::FilePath root_attachments_dir(base_dir_.Append(kAttachmentsDirectory)); + base::FilePath root_attachments_dir(AttachmentsRootPath()); DirectoryReader reader; if (!reader.Open(root_attachments_dir)) { return; @@ -888,8 +780,9 @@ void CrashReportDatabaseGeneric::CleanOrphanedAttachments() { DirectoryReader::Result result; while ((result = reader.NextFile(&filename)) == DirectoryReader::Result::kSuccess) { - const base::FilePath path(root_attachments_dir.Append(filename)); - if (IsDirectory(path, false)) { + const base::FilePath report_attachment_dir( + root_attachments_dir.Append(filename)); + if (IsDirectory(report_attachment_dir, false)) { UUID uuid; if (!uuid.InitializeFromString(filename.value())) { LOG(ERROR) << "unexpected attachment dir name " << filename.value(); @@ -919,27 +812,6 @@ void CrashReportDatabaseGeneric::CleanOrphanedAttachments() { } } -void CrashReportDatabaseGeneric::RemoveAttachmentsByUUID(const UUID& uuid) { - base::FilePath attachments_dir = AttachmentsPath(uuid); - if (!IsDirectory(attachments_dir, /*allow_symlinks=*/false)) { - return; - } - DirectoryReader reader; - if (!reader.Open(attachments_dir)) { - return; - } - - base::FilePath filename; - DirectoryReader::Result result; - while ((result = reader.NextFile(&filename)) == - DirectoryReader::Result::kSuccess) { - const base::FilePath filepath(attachments_dir.Append(filename)); - LoggingRemoveFile(filepath); - } - - LoggingRemoveDirectory(attachments_dir); -} - bool CrashReportDatabaseGeneric::ReadMetadata(const base::FilePath& path, Report* report) { const base::FilePath metadata_path( @@ -974,7 +846,7 @@ bool CrashReportDatabaseGeneric::ReadMetadata(const base::FilePath& path, // Seed the total size with the main report size and then add the sizes of any // potential attachments. uint64_t total_size = GetFileSize(path); - AddAttachmentSize(AttachmentsPath(uuid), &total_size); + total_size += GetDirectorySize(AttachmentsPath(uuid)); report->uuid = uuid; report->upload_attempts = metadata.upload_attempts; diff --git a/client/crash_report_database_mac.mm b/client/crash_report_database_mac.mm index 839cfc8f..ce0ab186 100644 --- a/client/crash_report_database_mac.mm +++ b/client/crash_report_database_mac.mm @@ -35,7 +35,9 @@ #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" #include "client/settings.h" +#include "util/file/directory_reader.h" #include "util/file/file_io.h" +#include "util/file/filesystem.h" #include "util/mac/xattr.h" #include "util/misc/initialization_state_dcheck.h" #include "util/misc/metrics.h" @@ -153,6 +155,8 @@ class CrashReportDatabaseMac : public CrashReportDatabase { Metrics::CrashSkippedReason reason) override; OperationStatus DeleteReport(const UUID& uuid) override; OperationStatus RequestUpload(const UUID& uuid) override; + int CleanDatabase(time_t lockfile_ttl) override; + base::FilePath DatabasePath() override; private: // CrashReportDatabase: @@ -244,21 +248,15 @@ class CrashReportDatabaseMac : public CrashReportDatabase { const base::FilePath& report_path, base::FilePath* out_path); + // Cleans any attachments that have no associated report in any state. + void CleanOrphanedAttachments(); + base::FilePath base_dir_; Settings settings_; bool xattr_new_names_; InitializationStateDcheck initialized_; }; -FileWriter* CrashReportDatabase::NewReport::AddAttachment( - const std::string& name) { - // Attachments aren't implemented in the Mac database yet. - return nullptr; -} - -void CrashReportDatabase::UploadReport::InitializeAttachments() { - // Attachments aren't implemented in the Mac database yet. -} CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path) : CrashReportDatabase(), @@ -288,6 +286,10 @@ bool CrashReportDatabaseMac::Initialize(bool may_create) { return false; } + if (!CreateOrEnsureDirectoryExists(AttachmentsRootPath())) { + return false; + } + if (!settings_.Initialize(base_dir_.Append(kSettings))) return false; @@ -315,6 +317,10 @@ bool CrashReportDatabaseMac::Initialize(bool may_create) { return true; } +base::FilePath CrashReportDatabaseMac::DatabasePath() { + return base_dir_; +} + Settings* CrashReportDatabaseMac::GetSettings() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return &settings_; @@ -381,6 +387,14 @@ CrashReportDatabaseMac::FinishedWritingCrashReport( } ignore_result(report->file_remover_.release()); + // Close all the attachments and disarm their removers too. + for (auto& writer : report->attachment_writers_) { + writer->Close(); + } + for (auto& remover : report->attachment_removers_) { + ignore_result(remover.release()); + } + Metrics::CrashReportPending(Metrics::PendingReportReason::kNewlyCreated); Metrics::CrashReportSize(size); @@ -444,7 +458,7 @@ CrashReportDatabaseMac::GetReportForUploading( if (!ReadReportMetadataLocked(upload_report->file_path, upload_report.get())) return kDatabaseError; - if (!upload_report->reader_->Open(upload_report->file_path)) { + if (!upload_report->Initialize(upload_report->file_path, this)) { return kFileSystemError; } @@ -544,6 +558,8 @@ CrashReportDatabase::OperationStatus CrashReportDatabaseMac::DeleteReport( return kFileSystemError; } + RemoveAttachmentsByUUID(uuid); + return kNoError; } @@ -628,6 +644,34 @@ CrashReportDatabase::OperationStatus CrashReportDatabaseMac::RequestUpload( return kNoError; } +int CrashReportDatabaseMac::CleanDatabase(time_t lockfile_ttl) { + int removed = 0; + time_t now = time(nullptr); + + DirectoryReader reader; + const base::FilePath new_dir(base_dir_.Append(kWriteDirectory)); + if (reader.Open(new_dir)) { + base::FilePath filename; + DirectoryReader::Result result; + while ((result = reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath filepath(new_dir.Append(filename)); + timespec filetime; + if (!FileModificationTime(filepath, &filetime)) { + continue; + } + if (filetime.tv_sec <= now - lockfile_ttl) { + if (LoggingRemoveFile(filepath)) { + ++removed; + } + } + } + } + + CleanOrphanedAttachments(); + return removed; +} + // static base::ScopedFD CrashReportDatabaseMac::ObtainReportLock( const base::FilePath& path) { @@ -685,13 +729,11 @@ bool CrashReportDatabaseMac::ReadReportMetadataLocked( return false; } - // There are no attachments on Mac so the total size is the main report size. - struct stat statbuf; - if (stat(path.value().c_str(), &statbuf) != 0) { - PLOG(ERROR) << "stat " << path.value(); - return false; - } - report->total_size = statbuf.st_size; + // Seed the total size with the main report size and then add the sizes of any + // potential attachments. + uint64_t total_size = GetFileSize(path); + total_size += GetDirectorySize(AttachmentsPath(report->uuid)); + report->total_size = total_size; return true; } @@ -758,6 +800,47 @@ CrashReportDatabaseMac::MarkReportCompletedLocked( return kNoError; } +void CrashReportDatabaseMac::CleanOrphanedAttachments() { + base::FilePath root_attachments_dir(AttachmentsRootPath()); + DirectoryReader reader; + if (!reader.Open(root_attachments_dir)) { + return; + } + + base::FilePath filename; + DirectoryReader::Result result; + while ((result = reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath report_attachment_dir( + root_attachments_dir.Append(filename)); + if (IsDirectory(report_attachment_dir, false)) { + UUID uuid; + if (!uuid.InitializeFromString(filename.value())) { + LOG(ERROR) << "unexpected attachment dir name " << filename.value(); + continue; + } + + // Check to see if the report is being created in "new". + base::FilePath new_dir_path = + base_dir_.Append(kWriteDirectory) + .Append(uuid.ToString() + "." + kCrashReportFileExtension); + if (IsRegularFile(new_dir_path)) { + continue; + } + + // Check to see if the report is in "pending" or "completed". + base::FilePath local_path = + LocateCrashReport(uuid, kReportStatePending | kReportStateCompleted); + if (!local_path.empty()) { + continue; + } + + // Couldn't find a report, assume these attachments are orphaned. + RemoveAttachmentsByUUID(uuid); + } + } +} + std::unique_ptr InitializeInternal( const base::FilePath& path, bool may_create) { diff --git a/client/crash_report_database_test.cc b/client/crash_report_database_test.cc index 9d3f1fc9..357702b0 100644 --- a/client/crash_report_database_test.cc +++ b/client/crash_report_database_test.cc @@ -675,10 +675,6 @@ TEST_F(CrashReportDatabaseTest, RequestUpload) { } TEST_F(CrashReportDatabaseTest, Attachments) { -#if defined(OS_APPLE) || defined(OS_WIN) - // Attachments aren't supported on Mac and Windows yet. - GTEST_SKIP(); -#else std::unique_ptr new_report; ASSERT_EQ(db()->PrepareNewCrashReport(&new_report), CrashReportDatabase::kNoError); @@ -717,16 +713,9 @@ TEST_F(CrashReportDatabaseTest, Attachments) { char result_buffer[sizeof(test_data)]; result_attachments["some_file"]->Read(result_buffer, sizeof(result_buffer)); EXPECT_EQ(memcmp(test_data, result_buffer, sizeof(test_data)), 0); -#endif } TEST_F(CrashReportDatabaseTest, OrphanedAttachments) { -#if defined(OS_APPLE) || defined(OS_WIN) - // Attachments aren't supported on Mac and Windows yet. - GTEST_SKIP(); -#else - // TODO: This is using paths that are specific to the generic implementation - // and will need to be generalized for Mac and Windows. std::unique_ptr new_report; ASSERT_EQ(db()->PrepareNewCrashReport(&new_report), CrashReportDatabase::kNoError); @@ -748,25 +737,43 @@ TEST_F(CrashReportDatabaseTest, OrphanedAttachments) { ASSERT_TRUE(LoggingRemoveFile(report.file_path)); +#if !defined(OS_APPLE) && !defined(OS_WIN) + // CrashReportDatabaseMac stores metadata in xattrs and does not have .meta + // files. + // CrashReportDatabaseWin stores metadata in a global metadata file and not + // per report. ASSERT_TRUE(LoggingRemoveFile(base::FilePath( report.file_path.RemoveFinalExtension().value() + ".meta"))); +#endif ASSERT_EQ(db()->LookUpCrashReport(uuid, &report), CrashReportDatabase::kReportNotFound); +#if defined(OS_WIN) + const std::wstring uuid_string = uuid.ToWString(); +#else + const std::string uuid_string = uuid.ToString(); +#endif base::FilePath report_attachments_dir( - path().Append("attachments").Append(uuid.ToString())); - base::FilePath file_path1(report_attachments_dir.Append("file1")); - base::FilePath file_path2(report_attachments_dir.Append("file2")); + path().Append(FILE_PATH_LITERAL("attachments")).Append(uuid_string)); + base::FilePath file_path1( + report_attachments_dir.Append(FILE_PATH_LITERAL("file1"))); + base::FilePath file_path2( + report_attachments_dir.Append(FILE_PATH_LITERAL("file2"))); EXPECT_TRUE(FileExists(file_path1)); EXPECT_TRUE(FileExists(file_path1)); +#if defined(OS_WIN) + // On Windows, reports removed from metadata are counted, even if the file + // is not on the disk. + EXPECT_EQ(db()->CleanDatabase(0), 1); +#else EXPECT_EQ(db()->CleanDatabase(0), 0); +#endif EXPECT_FALSE(FileExists(file_path1)); EXPECT_FALSE(FileExists(file_path2)); EXPECT_FALSE(FileExists(report_attachments_dir)); -#endif } // This test uses knowledge of the database format to break it, so it only @@ -860,10 +867,6 @@ TEST_F(CrashReportDatabaseTest, TotalSize_MainReportOnly) { } TEST_F(CrashReportDatabaseTest, GetReportSize_RightSizeWithAttachments) { -#if defined(OS_APPLE) || defined(OS_WIN) - // Attachments aren't supported on Mac and Windows yet. - return; -#else std::unique_ptr new_report; ASSERT_EQ(db()->PrepareNewCrashReport(&new_report), CrashReportDatabase::kNoError); @@ -895,7 +898,6 @@ TEST_F(CrashReportDatabaseTest, GetReportSize_RightSizeWithAttachments) { EXPECT_EQ(report.total_size, sizeof(main_report_data) + sizeof(attachment_1_data) + sizeof(attachment_2_data)); -#endif } } // namespace diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index b8ecadb7..33d729bb 100644 --- a/client/crash_report_database_win.cc +++ b/client/crash_report_database_win.cc @@ -48,8 +48,6 @@ constexpr wchar_t kCrashReportFileExtension[] = L"dmp"; constexpr uint32_t kMetadataFileHeaderMagic = 'CPAD'; constexpr uint32_t kMetadataFileVersion = 1; -constexpr base::FilePath::CharType kAttachmentsDirectory[] = L"attachments"; - using OperationStatus = CrashReportDatabase::OperationStatus; // Helpers --------------------------------------------------------------------- @@ -216,8 +214,10 @@ class Metadata { //! handle. ~Metadata(); - static std::unique_ptr Create(const base::FilePath& metadata_file, - const base::FilePath& report_dir); + static std::unique_ptr Create( + const base::FilePath& metadata_file, + const base::FilePath& report_dir, + const base::FilePath& attachments_dir); //! \brief Adds a new report to the set. //! @@ -280,7 +280,9 @@ class Metadata { int CleanDatabase(); private: - Metadata(FileHandle handle, const base::FilePath& report_dir); + Metadata(FileHandle handle, + const base::FilePath& report_dir, + const base::FilePath& attachments_dir); bool Rewind(); @@ -298,6 +300,7 @@ class Metadata { ScopedFileHandle handle_; const base::FilePath report_dir_; + const base::FilePath attachments_dir_; bool dirty_; //! \brief `true` when a Write() is required on destruction. std::vector reports_; }; @@ -312,8 +315,10 @@ Metadata::~Metadata() { } // static -std::unique_ptr Metadata::Create(const base::FilePath& metadata_file, - const base::FilePath& report_dir) { +std::unique_ptr Metadata::Create( + const base::FilePath& metadata_file, + const base::FilePath& report_dir, + const base::FilePath& attachments_dir) { // It is important that dwShareMode be non-zero so that concurrent access to // this file results in a successful open. This allows us to get to LockFileEx // which then blocks to guard access. @@ -338,7 +343,8 @@ std::unique_ptr Metadata::Create(const base::FilePath& metadata_file, return std::unique_ptr(); } - std::unique_ptr metadata(new Metadata(handle, report_dir)); + std::unique_ptr metadata( + new Metadata(handle, report_dir, attachments_dir)); // If Read() fails, for whatever reason (corruption, etc.) metadata will not // have been modified and will be in a clean empty state. We continue on and // return an empty database to hopefully recover. This means that existing @@ -427,9 +433,14 @@ int Metadata::CleanDatabase() { return removed; } -Metadata::Metadata(FileHandle handle, const base::FilePath& report_dir) - : handle_(handle), report_dir_(report_dir), dirty_(false), reports_() { -} +Metadata::Metadata(FileHandle handle, + const base::FilePath& report_dir, + const base::FilePath& attachments_dir) + : handle_(handle), + report_dir_(report_dir), + attachments_dir_(attachments_dir), + dirty_(false), + reports_() {} bool Metadata::Rewind() { FileOffset result = LoggingSeekFile(handle_.get(), 0, SEEK_SET); @@ -488,15 +499,10 @@ void Metadata::Read() { } ReportDisk report_disk(record, report_dir_, string_table); - // There are no attachments on Windows so the total size is the main - // report size. - struct _stati64 statbuf; - if (_wstat64(report_disk.file_path.value().c_str(), &statbuf) == 0) { - report_disk.total_size = statbuf.st_size; - } else { - LOG(ERROR) << "failed to stat report"; - } - + report_disk.total_size = GetFileSize(report_disk.file_path); + base::FilePath report_attachment_dir = + attachments_dir_.Append(report_disk.uuid.ToWString()); + report_disk.total_size += GetDirectorySize(report_attachment_dir); reports.push_back(report_disk); } } @@ -643,12 +649,7 @@ class CrashReportDatabaseWin : public CrashReportDatabase { OperationStatus DeleteReport(const UUID& uuid) override; OperationStatus RequestUpload(const UUID& uuid) override; int CleanDatabase(time_t lockfile_ttl) override; - - // Build a filepath for the root attachments directory. - base::FilePath AttachmentsRootPath(); - - // Build a filepath for the directory for the report to hold attachments. - base::FilePath AttachmentsPath(const UUID& uuid); + base::FilePath DatabasePath() override; private: // CrashReportDatabase: @@ -659,10 +660,6 @@ class CrashReportDatabaseWin : public CrashReportDatabase { // Cleans any attachments that have no associated report. void CleanOrphanedAttachments(); - // Attempt to remove any attachments associated with the given report UUID. - // There may not be any, so failing is not an error. - void RemoveAttachmentsByUUID(const UUID& uuid); - std::unique_ptr AcquireMetadata(); base::FilePath base_dir_; @@ -670,68 +667,6 @@ class CrashReportDatabaseWin : public CrashReportDatabase { InitializationStateDcheck initialized_; }; -base::FilePath CrashReportDatabaseWin::AttachmentsRootPath() { - return base_dir_.Append(kAttachmentsDirectory); -} - -base::FilePath CrashReportDatabaseWin::AttachmentsPath(const UUID& uuid) { - const std::wstring uuid_string = uuid.ToWString(); - return base_dir_.Append(kAttachmentsDirectory).Append(uuid_string); -} - -FileWriter* CrashReportDatabase::NewReport::AddAttachment( - const std::string& name) { - auto database_win = static_cast(database_); - base::FilePath attachments_root_dir = database_win->AttachmentsRootPath(); - base::FilePath attachments_dir = database_win->AttachmentsPath(uuid_); - if (!LoggingCreateDirectory( - attachments_root_dir, FilePermissions::kOwnerOnly, true) || - !LoggingCreateDirectory( - attachments_dir, FilePermissions::kOwnerOnly, true)) { - return nullptr; - } - - base::FilePath path = attachments_dir.Append(base::UTF8ToWide(name)); - - auto writer = std::make_unique(); - if (!writer->Open( - path, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly)) { - LOG(ERROR) << "could not open " << path.value().c_str(); - return nullptr; - } - attachment_writers_.emplace_back(std::move(writer)); - attachment_removers_.emplace_back(ScopedRemoveFile(path)); - return attachment_writers_.back().get(); -} - -void CrashReportDatabase::UploadReport::InitializeAttachments() { - base::FilePath attachments_dir = - static_cast(database_)->AttachmentsPath(uuid); - if (!IsDirectory(attachments_dir, /*allow_symlinks=*/false)) { - return; - } - DirectoryReader dir_reader; - if (!dir_reader.Open(attachments_dir)) { - return; - } - - base::FilePath filename; - DirectoryReader::Result dir_result; - while ((dir_result = dir_reader.NextFile(&filename)) == - DirectoryReader::Result::kSuccess) { - const base::FilePath filepath(attachments_dir.Append(filename)); - std::unique_ptr file_reader(std::make_unique()); - if (!file_reader->Open(filepath)) { - LOG(ERROR) << "attachment " << filepath.value().c_str() - << " couldn't be opened, skipping"; - continue; - } - attachment_readers_.emplace_back(std::move(file_reader)); - attachment_map_[base::WideToUTF8(filename.value())] = - attachment_readers_.back().get(); - } -} - CrashReportDatabaseWin::CrashReportDatabaseWin(const base::FilePath& path) : CrashReportDatabase(), base_dir_(path), settings_(), initialized_() {} @@ -753,6 +688,9 @@ bool CrashReportDatabaseWin::Initialize(bool may_create) { if (!CreateDirectoryIfNecessary(base_dir_.Append(kReportsDirectory))) return false; + if (!CreateDirectoryIfNecessary(AttachmentsRootPath())) + return false; + if (!settings_.Initialize(base_dir_.Append(kSettings))) return false; @@ -760,6 +698,10 @@ bool CrashReportDatabaseWin::Initialize(bool may_create) { return true; } +base::FilePath CrashReportDatabaseWin::DatabasePath() { + return base_dir_; +} + Settings* CrashReportDatabaseWin::GetSettings() { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return &settings_; @@ -934,27 +876,6 @@ OperationStatus CrashReportDatabaseWin::DeleteReport(const UUID& uuid) { return kNoError; } -void CrashReportDatabaseWin::RemoveAttachmentsByUUID(const UUID& uuid) { - base::FilePath attachments_dir = AttachmentsPath(uuid); - if (!IsDirectory(attachments_dir, /*allow_symlinks=*/false)) { - return; - } - DirectoryReader reader; - if (!reader.Open(attachments_dir)) { - return; - } - - base::FilePath filename; - DirectoryReader::Result result; - while ((result = reader.NextFile(&filename)) == - DirectoryReader::Result::kSuccess) { - const base::FilePath filepath(attachments_dir.Append(filename)); - LoggingRemoveFile(filepath); - } - - LoggingRemoveDirectory(attachments_dir); -} - OperationStatus CrashReportDatabaseWin::SkipReportUpload( const UUID& uuid, Metrics::CrashSkippedReason reason) { @@ -977,7 +898,9 @@ OperationStatus CrashReportDatabaseWin::SkipReportUpload( std::unique_ptr CrashReportDatabaseWin::AcquireMetadata() { base::FilePath metadata_file = base_dir_.Append(kMetadataFileName); - return Metadata::Create(metadata_file, base_dir_.Append(kReportsDirectory)); + return Metadata::Create(metadata_file, + base_dir_.Append(kReportsDirectory), + AttachmentsRootPath()); } std::unique_ptr InitializeInternal( @@ -1073,7 +996,7 @@ int CrashReportDatabaseWin::CleanDatabase(time_t lockfile_ttl) { } void CrashReportDatabaseWin::CleanOrphanedAttachments() { - base::FilePath root_attachments_dir(base_dir_.Append(kAttachmentsDirectory)); + base::FilePath root_attachments_dir = AttachmentsRootPath(); DirectoryReader reader; if (!reader.Open(root_attachments_dir)) { return; diff --git a/client/prune_crash_reports_test.cc b/client/prune_crash_reports_test.cc index b989d2f6..cce26aa0 100644 --- a/client/prune_crash_reports_test.cc +++ b/client/prune_crash_reports_test.cc @@ -69,6 +69,7 @@ class MockDatabase : public CrashReportDatabase { (override)); MOCK_METHOD(OperationStatus, DeleteReport, (const UUID&), (override)); MOCK_METHOD(OperationStatus, RequestUpload, (const UUID&), (override)); + MOCK_METHOD(base::FilePath, DatabasePath, (), (override)); // Google Mock doesn't support mocking methods with non-copyable types such as // unique_ptr. diff --git a/util/file/filesystem.h b/util/file/filesystem.h index e12f1271..e2affcc6 100644 --- a/util/file/filesystem.h +++ b/util/file/filesystem.h @@ -111,6 +111,22 @@ bool LoggingRemoveFile(const base::FilePath& path); //! \return `true` if the directory was removed. Otherwise, `false`. bool LoggingRemoveDirectory(const base::FilePath& path); +//! \brief Returns the size of the file at |filepath|. +//! The function will ignore symlinks (not follow them, not add them to +//! the returned size). +//! +//! \return The size of the file pointed by |filepath|. +uint64_t GetFileSize(const base::FilePath& filepath); + +//! \brief Returns the recursive sum of the size of the files in |dirPath|. +//! The function will ignore symlinks (not follow them, not add them to +//! the returned size). +//! +//! \param[in] dirPath The path to the directory +//! +//! \return The sum of the size of the files in |dirPath|. +uint64_t GetDirectorySize(const base::FilePath& dirPath); + } // namespace crashpad #endif // CRASHPAD_UTIL_FILE_FILESYSTEM_H_ diff --git a/util/file/filesystem_posix.cc b/util/file/filesystem_posix.cc index 60799eb4..c4133974 100644 --- a/util/file/filesystem_posix.cc +++ b/util/file/filesystem_posix.cc @@ -22,6 +22,7 @@ #include "base/logging.h" #include "build/build_config.h" +#include "util/file/directory_reader.h" namespace crashpad { @@ -112,4 +113,38 @@ bool LoggingRemoveDirectory(const base::FilePath& path) { return true; } +uint64_t GetFileSize(const base::FilePath& filepath) { + if (!IsRegularFile(filepath)) { + return 0; + } + struct stat statbuf; + if (stat(filepath.value().c_str(), &statbuf) == 0) { + return statbuf.st_size; + } + PLOG(ERROR) << "stat " << filepath.value().c_str(); + return 0; +} + +uint64_t GetDirectorySize(const base::FilePath& dirpath) { + if (!IsDirectory(dirpath, /*allow_symlinks=*/false)) { + return 0; + } + DirectoryReader reader; + if (!reader.Open(dirpath)) { + return 0; + } + base::FilePath filename; + DirectoryReader::Result result; + uint64_t size = 0; + while ((result = reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath filepath(dirpath.Append(filename)); + if (IsDirectory(filepath, /*allow_symlinks=*/false)) { + size += GetDirectorySize(filepath); + } else { + size += GetFileSize(filepath); + } + } + return size; +} } // namespace crashpad diff --git a/util/file/filesystem_test.cc b/util/file/filesystem_test.cc index 30cd0d12..9338cdcc 100644 --- a/util/file/filesystem_test.cc +++ b/util/file/filesystem_test.cc @@ -21,12 +21,15 @@ #include "test/errors.h" #include "test/filesystem.h" #include "test/scoped_temp_dir.h" +#include "util/file/file_writer.h" #include "util/misc/time.h" namespace crashpad { namespace test { namespace { +constexpr char kTestFileContent[] = "file_content"; + bool CurrentTime(timespec* now) { #if defined(OS_APPLE) timeval now_tv; @@ -471,6 +474,62 @@ TEST(Filesystem, RemoveDirectory_SymbolicLinks) { #endif // !OS_FUCHSIA +TEST(Filesystem, GetFileSize) { + ScopedTempDir temp_dir; + base::FilePath filepath(temp_dir.path().Append(FILE_PATH_LITERAL("file"))); + FileWriter writer; + bool is_created = writer.Open( + filepath, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly); + ASSERT_TRUE(is_created); + writer.Write(kTestFileContent, sizeof(kTestFileContent)); + writer.Close(); + uint64_t filesize = GetFileSize(filepath); + EXPECT_EQ(filesize, sizeof(kTestFileContent)); + +#if !defined(OS_FUCHSIA) + // Create a link to a file. + base::FilePath link(temp_dir.path().Append(FILE_PATH_LITERAL("link"))); + ASSERT_TRUE(CreateSymbolicLink(filepath, link)); + uint64_t filesize_link = GetFileSize(link); + EXPECT_EQ(filesize_link, 0u); +#endif // !OS_FUCHSIA +} + +TEST(Filesystem, GetDirectorySize) { + ScopedTempDir temp_dir; + base::FilePath dir(temp_dir.path().Append(FILE_PATH_LITERAL("dir"))); + ASSERT_TRUE( + LoggingCreateDirectory(dir, FilePermissions::kWorldReadable, false)); + base::FilePath filepath1(temp_dir.path().Append(FILE_PATH_LITERAL("file"))); + FileWriter writer1; + bool is_created1 = writer1.Open( + filepath1, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly); + ASSERT_TRUE(is_created1); + writer1.Write(kTestFileContent, sizeof(kTestFileContent)); + writer1.Close(); + + base::FilePath filepath2(dir.Append(FILE_PATH_LITERAL("file"))); + FileWriter writer2; + bool is_created2 = writer2.Open( + filepath2, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly); + ASSERT_TRUE(is_created2); + writer2.Write(kTestFileContent, sizeof(kTestFileContent)); + writer2.Close(); + +#if !defined(OS_FUCHSIA) + // Create a link to a file. + base::FilePath link(dir.Append(FILE_PATH_LITERAL("link"))); + ASSERT_TRUE(CreateSymbolicLink(filepath2, link)); + + // Create a link to a dir. + base::FilePath linkdir(temp_dir.path().Append(FILE_PATH_LITERAL("link"))); + ASSERT_TRUE(CreateSymbolicLink(dir, linkdir)); +#endif // !OS_FUCHSIA + + uint64_t filesize = GetDirectorySize(temp_dir.path()); + EXPECT_EQ(filesize, 2 * sizeof(kTestFileContent)); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/file/filesystem_win.cc b/util/file/filesystem_win.cc index d0270fc6..8edb2956 100644 --- a/util/file/filesystem_win.cc +++ b/util/file/filesystem_win.cc @@ -19,6 +19,7 @@ #include "base/logging.h" #include "base/strings/utf_string_conversions.h" +#include "util/file/directory_reader.h" #include "util/misc/time.h" namespace crashpad { @@ -159,4 +160,40 @@ bool LoggingRemoveDirectory(const base::FilePath& path) { return LoggingRemoveDirectoryImpl(path); } +uint64_t GetFileSize(const base::FilePath& filepath) { + struct _stati64 statbuf; + if (!IsRegularFile(filepath)) { + return 0; + } + int ret_value = _wstat64(filepath.value().c_str(), &statbuf); + if (ret_value == 0) { + return statbuf.st_size; + } + PLOG(ERROR) << "stat " << filepath.value().c_str(); + return 0; +} + +uint64_t GetDirectorySize(const base::FilePath& dirpath) { + if (!IsDirectory(dirpath, /*allow_symlinks=*/false)) { + return 0; + } + DirectoryReader reader; + if (!reader.Open(dirpath)) { + return 0; + } + base::FilePath filename; + DirectoryReader::Result result; + uint64_t size = 0; + while ((result = reader.NextFile(&filename)) == + DirectoryReader::Result::kSuccess) { + const base::FilePath filepath(dirpath.Append(filename)); + if (IsDirectory(filepath, /*allow_symlinks=*/false)) { + size += GetDirectorySize(filepath); + } else { + size += GetFileSize(filepath); + } + } + return size; +} + } // namespace crashpad