diff --git a/client/crash_report_database.h b/client/crash_report_database.h index 1073d053..1897d2ec 100644 --- a/client/crash_report_database.h +++ b/client/crash_report_database.h @@ -165,14 +165,34 @@ class CrashReportDatabase { virtual ~CrashReportDatabase() {} - //! \brief Initializes a database of crash reports. + //! \brief Opens a database of crash reports, possibly creating it. //! - //! \param[in] path A path to the database to be created or opened. + //! \param[in] path A path to the database to be created or opened. If the + //! database does not yet exist, it will be created if possible. Note that + //! for databases implemented as directory structures, existence refers + //! solely to the outermost directory. //! //! \return A database object on success, `nullptr` on failure with an error //! logged. + //! + //! \sa InitializeWithoutCreating static scoped_ptr Initialize(const base::FilePath& path); + //! \brief Opens an existing database of crash reports. + //! + //! \param[in] path A path to the database to be opened. If the database does + //! not yet exist, it will not be created. Note that for databases + //! implemented as directory structures, existence refers solely to the + //! outermost directory. On such databases, as long as the outermost + //! directory is present, this method will create the inner structure. + //! + //! \return A database object on success, `nullptr` on failure with an error + //! logged. + //! + //! \sa Initialize + static scoped_ptr InitializeWithoutCreating( + const base::FilePath& path); + //! \brief Returns the Settings object for this database. //! //! \return A weak pointer to the Settings object, which is owned by the diff --git a/client/crash_report_database_mac.mm b/client/crash_report_database_mac.mm index 621d8a66..ca355c05 100644 --- a/client/crash_report_database_mac.mm +++ b/client/crash_report_database_mac.mm @@ -63,29 +63,35 @@ const char kXattrUploadAttemptCount[] = "upload_count"; const char kXattrDatabaseInitialized[] = "initialized"; +// Ensures that the node at |path| is a directory. If the |path| refers to a +// file, rather than a directory, returns false. Otherwise, returns true, +// indicating that |path| already was a directory. +bool EnsureDirectoryExists(const base::FilePath& path) { + struct stat st; + if (stat(path.value().c_str(), &st) != 0) { + PLOG(ERROR) << "stat " << path.value(); + return false; + } + if (!S_ISDIR(st.st_mode)) { + LOG(ERROR) << "stat " << path.value() << ": not a directory"; + return false; + } + return true; +} + // Ensures that the node at |path| is a directory, and creates it if it does -// not exist. If the |path| points to a file, rather than a directory, or the +// not exist. If the |path| refers to a file, rather than a directory, or the // directory could not be created, returns false. Otherwise, returns true, // indicating that |path| already was or now is a directory. bool CreateOrEnsureDirectoryExists(const base::FilePath& path) { if (mkdir(path.value().c_str(), 0755) == 0) { return true; - } else if (errno == EEXIST) { - struct stat st; - if (stat(path.value().c_str(), &st) != 0) { - PLOG(ERROR) << "stat"; - return false; - } - if (S_ISDIR(st.st_mode)) { - return true; - } else { - LOG(ERROR) << "not a directory"; - return false; - } - } else { - PLOG(ERROR) << "mkdir"; + } + if (errno != EEXIST) { + PLOG(ERROR) << "mkdir " << path.value(); return false; } + return EnsureDirectoryExists(path); } //! \brief A CrashReportDatabase that uses HFS+ extended attributes to store @@ -107,7 +113,7 @@ class CrashReportDatabaseMac : public CrashReportDatabase { explicit CrashReportDatabaseMac(const base::FilePath& path); virtual ~CrashReportDatabaseMac(); - bool Initialize(); + bool Initialize(bool may_create); // CrashReportDatabase: Settings* GetSettings() override; @@ -115,8 +121,7 @@ class CrashReportDatabaseMac : public CrashReportDatabase { OperationStatus FinishedWritingCrashReport(NewReport* report, UUID* uuid) override; OperationStatus ErrorWritingCrashReport(NewReport* report) override; - OperationStatus LookUpCrashReport(const UUID& uuid, - Report* report) override; + OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override; OperationStatus GetPendingReports(std::vector* reports) override; OperationStatus GetCompletedReports(std::vector* reports) override; OperationStatus GetReportForUploading(const UUID& uuid, @@ -181,7 +186,6 @@ class CrashReportDatabaseMac : public CrashReportDatabase { static OperationStatus ReportsInDirectory(const base::FilePath& path, std::vector* reports); - //! \brief Creates a database xattr name from the short constant name. //! //! \param[in] name The short name of the extended attribute. @@ -205,12 +209,17 @@ CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path) CrashReportDatabaseMac::~CrashReportDatabaseMac() {} -bool CrashReportDatabaseMac::Initialize() { +bool CrashReportDatabaseMac::Initialize(bool may_create) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); // Check if the database already exists. - if (!CreateOrEnsureDirectoryExists(base_dir_)) + if (may_create) { + if (!CreateOrEnsureDirectoryExists(base_dir_)) { + return false; + } + } else if (!EnsureDirectoryExists(base_dir_)) { return false; + } // Create the three processing directories for the database. for (size_t i = 0; i < arraysize(kReportDirectories); ++i) { @@ -501,8 +510,8 @@ base::FilePath CrashReportDatabaseMac::LocateCrashReport(const UUID& uuid) { const std::string target_uuid = uuid.ToString(); for (size_t i = 0; i < arraysize(kReportDirectories); ++i) { base::FilePath path = - base_dir_.Append(kReportDirectories[i]) - .Append(target_uuid + "." + kCrashReportFileExtension); + base_dir_.Append(kReportDirectories[i]) + .Append(target_uuid + "." + kCrashReportFileExtension); // Test if the path exists. struct stat st; @@ -616,17 +625,28 @@ std::string CrashReportDatabaseMac::XattrName(const base::StringPiece& name) { return base::StringPrintf("com.googlecode.crashpad.%s", name.data()); } -} // namespace - -// static -scoped_ptr CrashReportDatabase::Initialize( - const base::FilePath& path) { +scoped_ptr InitializeInternal(const base::FilePath& path, + bool may_create) { scoped_ptr database_mac( new CrashReportDatabaseMac(path)); - if (!database_mac->Initialize()) + if (!database_mac->Initialize(may_create)) database_mac.reset(); return scoped_ptr(database_mac.release()); } +} // namespace + +// static +scoped_ptr CrashReportDatabase::Initialize( + const base::FilePath& path) { + return InitializeInternal(path, true); +} + +// static +scoped_ptr CrashReportDatabase::InitializeWithoutCreating( + const base::FilePath& path) { + return InitializeInternal(path, false); +} + } // namespace crashpad diff --git a/client/crash_report_database_test.cc b/client/crash_report_database_test.cc index 83b0de0f..54dcdd75 100644 --- a/client/crash_report_database_test.cc +++ b/client/crash_report_database_test.cc @@ -75,8 +75,7 @@ class CrashReportDatabaseTest : public testing::Test { db_->GetReportForUploading(uuid, &report)); EXPECT_NE(UUID(), report->uuid); EXPECT_FALSE(report->file_path.empty()); - EXPECT_TRUE(FileExists(report->file_path)) - << report->file_path.value(); + EXPECT_TRUE(FileExists(report->file_path)) << report->file_path.value(); EXPECT_GT(report->creation_time, 0); EXPECT_EQ(CrashReportDatabase::kNoError, db_->RecordUploadAttempt(report, successful, id)); @@ -116,7 +115,7 @@ TEST_F(CrashReportDatabaseTest, Initialize) { Settings* settings = db()->GetSettings(); - UUID client_ids[2]; + UUID client_ids[3]; ASSERT_TRUE(settings->GetClientID(&client_ids[0])); EXPECT_NE(client_ids[0], UUID()); @@ -127,7 +126,7 @@ TEST_F(CrashReportDatabaseTest, Initialize) { // Close and reopen the database at the same path. ResetDatabase(); EXPECT_FALSE(db()); - auto db = CrashReportDatabase::Initialize(path()); + auto db = CrashReportDatabase::InitializeWithoutCreating(path()); ASSERT_TRUE(db); settings = db->GetSettings(); @@ -138,12 +137,31 @@ TEST_F(CrashReportDatabaseTest, Initialize) { ASSERT_TRUE(settings->GetLastUploadAttemptTime(&last_upload_attempt_time)); EXPECT_EQ(0, last_upload_attempt_time); + // Check that the database can also be opened by the method that is permitted + // to create it. + db = CrashReportDatabase::Initialize(path()); + ASSERT_TRUE(db); + + settings = db->GetSettings(); + + ASSERT_TRUE(settings->GetClientID(&client_ids[2])); + EXPECT_EQ(client_ids[0], client_ids[2]); + + ASSERT_TRUE(settings->GetLastUploadAttemptTime(&last_upload_attempt_time)); + EXPECT_EQ(0, last_upload_attempt_time); + std::vector reports; EXPECT_EQ(CrashReportDatabase::kNoError, db->GetPendingReports(&reports)); EXPECT_TRUE(reports.empty()); reports.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db->GetCompletedReports(&reports)); EXPECT_TRUE(reports.empty()); + + // InitializeWithoutCreating() shouldn’t create a nonexistent database. + base::FilePath non_database_path = + path().DirName().Append(FILE_PATH_LITERAL("not_a_database")); + db = CrashReportDatabase::InitializeWithoutCreating(non_database_path); + EXPECT_FALSE(db); } TEST_F(CrashReportDatabaseTest, NewCrashReport) { @@ -163,14 +181,12 @@ TEST_F(CrashReportDatabaseTest, NewCrashReport) { ExpectPreparedCrashReport(report); std::vector reports; - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&reports)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&reports)); ASSERT_EQ(1u, reports.size()); EXPECT_EQ(report.uuid, reports[0].uuid); reports.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetCompletedReports(&reports)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&reports)); EXPECT_TRUE(reports.empty()); } @@ -320,8 +336,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { const UUID& report_4_uuid = reports[4].uuid; std::vector pending; - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); std::vector completed; EXPECT_EQ(CrashReportDatabase::kNoError, @@ -334,8 +349,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { UploadReport(report_1_uuid, true, "report1"); pending.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); completed.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&completed)); @@ -357,8 +371,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { UploadReport(report_2_uuid, false, std::string()); pending.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); completed.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&completed)); @@ -380,8 +393,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { UploadReport(report_4_uuid, true, "report_4"); pending.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); completed.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&completed)); @@ -393,8 +405,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { UploadReport(report_2_uuid, true, "report 2"); pending.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); completed.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&completed)); @@ -403,8 +414,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { ASSERT_EQ(3u, completed.size()); for (const auto& report : pending) { - EXPECT_TRUE(report.uuid == report_0_uuid || - report.uuid == report_3_uuid); + EXPECT_TRUE(report.uuid == report_0_uuid || report.uuid == report_3_uuid); EXPECT_FALSE(report.file_path.empty()); } @@ -413,8 +423,7 @@ TEST_F(CrashReportDatabaseTest, GetCompletedAndNotUploadedReports) { db()->SkipReportUpload(report_3_uuid)); pending.clear(); - EXPECT_EQ(CrashReportDatabase::kNoError, - db()->GetPendingReports(&pending)); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetPendingReports(&pending)); completed.clear(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->GetCompletedReports(&completed)); diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index 6d2bf1af..95358b47 100644 --- a/client/crash_report_database_win.cc +++ b/client/crash_report_database_win.cc @@ -510,6 +510,21 @@ OperationStatus Metadata::VerifyReport(const ReportDisk& report_disk, : CrashReportDatabase::kBusyError; } +bool EnsureDirectory(const base::FilePath& path) { + DWORD fileattr = GetFileAttributes(path.value().c_str()); + if (fileattr == INVALID_FILE_ATTRIBUTES) { + PLOG(ERROR) << "GetFileAttributes " << base::UTF16ToUTF8(path.value()); + return false; + } + if ((fileattr & FILE_ATTRIBUTE_DIRECTORY) == 0) { + LOG(ERROR) << "GetFileAttributes " + << base::UTF16ToUTF8(path.value()) + << ": not a directory"; + return false; + } + return true; +} + //! \brief Ensures that the node at path is a directory, and creates it if it //! does not exist. //! @@ -520,18 +535,10 @@ bool CreateDirectoryIfNecessary(const base::FilePath& path) { if (CreateDirectory(path.value().c_str(), nullptr)) return true; if (GetLastError() != ERROR_ALREADY_EXISTS) { - PLOG(ERROR) << "CreateDirectory"; + PLOG(ERROR) << "CreateDirectory " << base::UTF16ToUTF8(path.value()); return false; } - DWORD fileattr = GetFileAttributes(path.value().c_str()); - if (fileattr == INVALID_FILE_ATTRIBUTES) { - PLOG(ERROR) << "GetFileAttributes"; - return false; - } - if ((fileattr & FILE_ATTRIBUTE_DIRECTORY) != 0) - return true; - LOG(ERROR) << "not a directory"; - return false; + return EnsureDirectory(path); } // CrashReportDatabaseWin ------------------------------------------------------ @@ -541,7 +548,7 @@ class CrashReportDatabaseWin : public CrashReportDatabase { explicit CrashReportDatabaseWin(const base::FilePath& path); ~CrashReportDatabaseWin() override; - bool Initialize(); + bool Initialize(bool may_create); // CrashReportDatabase: Settings* GetSettings() override; @@ -580,12 +587,19 @@ CrashReportDatabaseWin::CrashReportDatabaseWin(const base::FilePath& path) CrashReportDatabaseWin::~CrashReportDatabaseWin() { } -bool CrashReportDatabaseWin::Initialize() { +bool CrashReportDatabaseWin::Initialize(bool may_create) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); - // Ensure the database and report subdirectories exist. - if (!CreateDirectoryIfNecessary(base_dir_) || - !CreateDirectoryIfNecessary(base_dir_.Append(kReportsDirectory))) + // Ensure the database directory exists. + if (may_create) { + if (!CreateDirectoryIfNecessary(base_dir_)) + return false; + } else if (!EnsureDirectory(base_dir_)) { + return false; + } + + // Ensure that the report subdirectory exists. + if (!CreateDirectoryIfNecessary(base_dir_.Append(kReportsDirectory))) return false; if (!settings_.Initialize()) @@ -796,15 +810,27 @@ scoped_ptr CrashReportDatabaseWin::AcquireMetadata() { return Metadata::Create(metadata_file, base_dir_.Append(kReportsDirectory)); } +scoped_ptr InitializeInternal( + const base::FilePath& path, bool may_create) { + scoped_ptr database_win( + new CrashReportDatabaseWin(path)); + return database_win->Initialize(may_create) + ? database_win.Pass() + : scoped_ptr(); +} + } // namespace // static scoped_ptr CrashReportDatabase::Initialize( const base::FilePath& path) { - scoped_ptr database_win( - new CrashReportDatabaseWin(path)); - return database_win->Initialize() ? database_win.Pass() - : scoped_ptr(); + return InitializeInternal(path, true); +} + +// static +scoped_ptr CrashReportDatabase::InitializeWithoutCreating( + const base::FilePath& path) { + return InitializeInternal(path, false); } } // namespace crashpad diff --git a/handler/main.cc b/handler/main.cc index 261808c9..5f192f92 100644 --- a/handler/main.cc +++ b/handler/main.cc @@ -21,7 +21,6 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "client/crash_report_database.h" #include "client/crashpad_client.h" @@ -225,14 +224,9 @@ int HandlerMain(int argc, char* argv[]) { MACH_MSG_TYPE_MAKE_SEND); #endif // OS_MACOSX - scoped_ptr database( - CrashReportDatabase::Initialize(base::FilePath( -#if defined(OS_MACOSX) - options.database -#elif defined(OS_WIN) - base::UTF8ToUTF16(options.database) -#endif - ))); + scoped_ptr database(CrashReportDatabase::Initialize( + base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType( + options.database)))); if (!database) { return EXIT_FAILURE; } diff --git a/tools/crashpad_database_util.ad b/tools/crashpad_database_util.ad index 1eeb7db2..78107ee3 100644 --- a/tools/crashpad_database_util.ad +++ b/tools/crashpad_database_util.ad @@ -41,10 +41,13 @@ client library. This tool exists to allow developers to manipulate a Crashpad database. == Options +*--create*:: +Creates the database identified by *--database* if it does not exist, provided +that the parent directory of 'PATH' exists. + *-d*, *--database*='PATH':: Use 'PATH' as the path to the Crashpad crash report database. This option is -required. If the database does not exist, it will be created, provided that the -parent directory of 'PATH' exists. +required. The database must already exist unless *--create* is also specified. *--show-client-id*:: Show the client ID stored in the database’s settings. The client ID is formatted diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index 8f06aad9..e09f1ded 100644 --- a/tools/crashpad_database_util.cc +++ b/tools/crashpad_database_util.cc @@ -46,6 +46,7 @@ void Usage(const base::FilePath& me) { "Usage: %" PRFilePath " [OPTION]... PID\n" "Operate on Crashpad crash report databases.\n" "\n" +" --create allow database at PATH to be created\n" " -d, --database=PATH operate on the crash report database at PATH\n" " --show-client-id show the client ID\n" " --show-uploads-enabled show whether uploads are enabled\n" @@ -72,6 +73,7 @@ struct Options { const char* database; const char* set_last_upload_attempt_time_string; time_t set_last_upload_attempt_time; + bool create; bool show_client_id; bool show_uploads_enabled; bool show_last_upload_attempt_time; @@ -253,6 +255,7 @@ int DatabaseUtilMain(int argc, char* argv[]) { // Long options without short equivalents. kOptionLastChar = 255, + kOptionCreate, kOptionShowClientID, kOptionShowUploadsEnabled, kOptionShowLastUploadAttemptTime, @@ -271,6 +274,7 @@ int DatabaseUtilMain(int argc, char* argv[]) { }; const option long_options[] = { + {"create", no_argument, nullptr, kOptionCreate}, {"database", required_argument, nullptr, kOptionDatabase}, {"show-client-id", no_argument, nullptr, kOptionShowClientID}, {"show-uploads-enabled", no_argument, nullptr, kOptionShowUploadsEnabled}, @@ -305,6 +309,10 @@ int DatabaseUtilMain(int argc, char* argv[]) { int opt; while ((opt = getopt_long(argc, argv, "d:", long_options, nullptr)) != -1) { switch (opt) { + case kOptionCreate: { + options.create = true; + break; + } case kOptionDatabase: { options.database = optarg; break; @@ -410,14 +418,19 @@ int DatabaseUtilMain(int argc, char* argv[]) { options.has_set_uploads_enabled + (options.set_last_upload_attempt_time_string != nullptr); - if (show_operations + set_operations == 0) { + if ((options.create ? 1 : 0) + show_operations + set_operations == 0) { ToolSupport::UsageHint(me, "nothing to do"); return EXIT_FAILURE; } - scoped_ptr database(CrashReportDatabase::Initialize( - base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType( - options.database)))); + scoped_ptr database; + base::FilePath database_path = base::FilePath( + ToolSupport::CommandLineArgumentToFilePathStringType(options.database)); + if (options.create) { + database = CrashReportDatabase::Initialize(database_path); + } else { + database = CrashReportDatabase::InitializeWithoutCreating(database_path); + } if (!database) { return EXIT_FAILURE; }