From 1a635e3a799c9c93b39d1a9f507b3c9eebbd873b Mon Sep 17 00:00:00 2001 From: Robert Sesek Date: Mon, 9 Mar 2015 18:47:52 -0400 Subject: [PATCH] Define the Settings interface for a CrashReportDatabase and provide a Mac implementation. R=mark@chromium.org Review URL: https://codereview.chromium.org/988063003 --- client/client.gyp | 15 ++ client/crash_report_database.h | 8 + client/crash_report_database_mac.mm | 19 ++- client/crash_report_database_win.cc | 7 + client/settings.cc | 250 ++++++++++++++++++++++++++++ client/settings.h | 147 ++++++++++++++++ client/settings_test.cc | 175 +++++++++++++++++++ 7 files changed, 620 insertions(+), 1 deletion(-) create mode 100644 client/settings.cc create mode 100644 client/settings.h create mode 100644 client/settings_test.cc diff --git a/client/client.gyp b/client/client.gyp index 65b33486..27057110 100644 --- a/client/client.gyp +++ b/client/client.gyp @@ -39,6 +39,8 @@ 'crashpad_client_mac.cc', 'crashpad_info.cc', 'crashpad_info.h', + 'settings.cc', + 'settings.h', 'simple_string_dictionary.cc', 'simple_string_dictionary.h', 'simulate_crash.h', @@ -52,6 +54,10 @@ '-lrpcrt4.lib', ], }, + 'sources!': [ + # Port to Win https://code.google.com/p/crashpad/issues/detail?id=13 + 'settings.cc', + ], }], ], }, @@ -73,9 +79,18 @@ 'sources': [ 'capture_context_mac_test.cc', 'crash_report_database_test.cc', + 'settings_test.cc', 'simple_string_dictionary_test.cc', 'simulate_crash_mac_test.cc', ], + 'conditions': [ + ['OS=="win"', { + 'sources!': [ + # Port to Win https://code.google.com/p/crashpad/issues/detail?id=13 + 'settings_test.cc', + ], + }], + ], }, ], } diff --git a/client/crash_report_database.h b/client/crash_report_database.h index a266b212..a7e2c200 100644 --- a/client/crash_report_database.h +++ b/client/crash_report_database.h @@ -28,6 +28,8 @@ namespace crashpad { +class Settings; + //! \brief An interface for managing a collection of crash report files and //! metadata associated with the crash reports. //! @@ -141,6 +143,12 @@ class CrashReportDatabase { //! logged. static scoped_ptr Initialize(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 + //! database. + virtual Settings* GetSettings() = 0; + //! \brief Creates a record of a new crash report. //! //! Callers can then write the crash report using the file handle provided. diff --git a/client/crash_report_database_mac.mm b/client/crash_report_database_mac.mm index e4c904af..75f63f47 100644 --- a/client/crash_report_database_mac.mm +++ b/client/crash_report_database_mac.mm @@ -24,11 +24,13 @@ #include #include "base/logging.h" +#include "base/mac/scoped_nsautorelease_pool.h" #include "base/posix/eintr_wrapper.h" #include "base/scoped_generic.h" #include "base/strings/string_piece.h" #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" +#include "client/settings.h" #include "util/file/file_io.h" #include "util/mac/xattr.h" @@ -42,6 +44,8 @@ const char kWriteDirectory[] = "new"; const char kUploadPendingDirectory[] = "pending"; const char kCompletedDirectory[] = "completed"; +const char kSettings[] = "settings.dat"; + const char* const kReportDirectories[] = { kWriteDirectory, kUploadPendingDirectory, @@ -106,6 +110,7 @@ class CrashReportDatabaseMac : public CrashReportDatabase { bool Initialize(); // CrashReportDatabase: + Settings* GetSettings() override; OperationStatus PrepareNewCrashReport(NewReport** report) override; OperationStatus FinishedWritingCrashReport(NewReport* report, UUID* uuid) override; @@ -184,12 +189,15 @@ class CrashReportDatabaseMac : public CrashReportDatabase { static std::string XattrName(const base::StringPiece& name); base::FilePath base_dir_; + Settings settings_; DISALLOW_COPY_AND_ASSIGN(CrashReportDatabaseMac); }; CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path) - : CrashReportDatabase(), base_dir_(path) { + : CrashReportDatabase(), + base_dir_(path), + settings_(base_dir_.Append(kSettings)) { } CrashReportDatabaseMac::~CrashReportDatabaseMac() {} @@ -205,11 +213,18 @@ bool CrashReportDatabaseMac::Initialize() { return false; } + if (!settings_.Initialize()) + return false; + // Write an xattr as the last step, to ensure the filesystem has support for // them. This attribute will never be read. return WriteXattrBool(base_dir_, XattrName(kXattrDatabaseInitialized), true); } +Settings* CrashReportDatabaseMac::GetSettings() { + return &settings_; +} + CrashReportDatabase::OperationStatus CrashReportDatabaseMac::PrepareNewCrashReport(NewReport** out_report) { uuid_t uuid_gen; @@ -505,6 +520,8 @@ bool CrashReportDatabaseMac::ReadReportMetadataLocked( CrashReportDatabase::OperationStatus CrashReportDatabaseMac::ReportsInDirectory( const base::FilePath& path, std::vector* reports) { + base::mac::ScopedNSAutoreleasePool pool; + DCHECK(reports->empty()); NSError* error = nil; diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index 84ed6e22..a80046c1 100644 --- a/client/crash_report_database_win.cc +++ b/client/crash_report_database_win.cc @@ -524,6 +524,7 @@ class CrashReportDatabaseWin : public CrashReportDatabase { bool Initialize(); // CrashReportDatabase: + Settings* GetSettings() override; OperationStatus PrepareNewCrashReport(NewReport** report) override; OperationStatus FinishedWritingCrashReport(NewReport* report, UUID* uuid) override; @@ -565,6 +566,12 @@ bool CrashReportDatabaseWin::Initialize() { return true; } +Settings* CrashReportDatabaseWin::GetSettings() { + // Port to Win https://code.google.com/p/crashpad/issues/detail?id=13. + NOTREACHED(); + return nullptr; +} + OperationStatus CrashReportDatabaseWin::PrepareNewCrashReport( NewReport** report) { ::UUID system_uuid; diff --git a/client/settings.cc b/client/settings.cc new file mode 100644 index 00000000..8a087b97 --- /dev/null +++ b/client/settings.cc @@ -0,0 +1,250 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "client/settings.h" + +#include + +#include +#include +#include + +#include "base/compiler_specific.h" +#include "base/logging.h" +#include "base/posix/eintr_wrapper.h" +#include "util/numeric/in_range_cast.h" + +namespace crashpad { + +struct ALIGNAS(4) Settings::Data { + static const uint16_t kSettingsVersion = 1; + + Data() : version(kSettingsVersion), + options(0), + last_upload_attempt_time(0), + client_id() {} + + enum Options : uint32_t { + kUploadsEnabled = 1 << 0, + }; + + uint32_t version; + uint32_t options; + uint64_t last_upload_attempt_time; // time_t + UUID client_id; +}; + +Settings::Settings(const base::FilePath& file_path) + : file_path_(file_path), + initialized_() { +} + +Settings::~Settings() { +} + +bool Settings::Initialize() { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + + ScopedFileHandle handle(HANDLE_EINTR( + open(file_path(), + O_CREAT | O_EXCL | O_WRONLY | O_EXLOCK, + 0644))); + + // The file was created, so this is a new database that needs to be + // initialized with a client ID. + if (handle.is_valid()) { + bool initialized = InitializeSettings(handle.get()); + if (initialized) + INITIALIZATION_STATE_SET_VALID(initialized_); + return initialized; + } + + // The file wasn't created, try opening it for a write operation. If the file + // needs to be recovered, writing is necessary. This also ensures that the + // process has permission to write the file. + Data settings; + if (!OpenForWritingAndReadSettings(&settings).is_valid()) + return false; + + INITIALIZATION_STATE_SET_VALID(initialized_); + return true; +} + +bool Settings::GetClientID(UUID* client_id) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + Data settings; + if (!OpenAndReadSettings(&settings)) + return false; + + *client_id = settings.client_id; + return true; +} + +bool Settings::GetUploadsEnabled(bool* enabled) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + Data settings; + if (!OpenAndReadSettings(&settings)) + return false; + + *enabled = (settings.options & Data::Options::kUploadsEnabled) != 0; + return true; +} + +bool Settings::SetUploadsEnabled(bool enabled) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + Data settings; + ScopedFileHandle handle = OpenForWritingAndReadSettings(&settings); + if (!handle.is_valid()) + return false; + + if (enabled) + settings.options |= Data::Options::kUploadsEnabled; + else + settings.options &= ~Data::Options::kUploadsEnabled; + + return WriteSettings(handle.get(), settings); +} + +bool Settings::GetLastUploadAttemptTime(time_t* time) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + Data settings; + if (!OpenAndReadSettings(&settings)) + return false; + + *time = InRangeCast(settings.last_upload_attempt_time, + std::numeric_limits::max()); + return true; +} + +bool Settings::SetLastUploadAttemptTime(time_t time) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + Data settings; + ScopedFileHandle handle = OpenForWritingAndReadSettings(&settings); + if (!handle.is_valid()) + return false; + + settings.last_upload_attempt_time = InRangeCast(time, 0); + + return WriteSettings(handle.get(), settings); +} + +ScopedFileHandle Settings::OpenForReading() { + ScopedFileHandle handle(HANDLE_EINTR(open(file_path(), O_RDONLY | O_SHLOCK))); + PLOG_IF(ERROR, !handle.is_valid()) << "open for reading"; + return handle.Pass(); +} + +ScopedFileHandle Settings::OpenForReadingAndWriting() { + ScopedFileHandle handle(HANDLE_EINTR( + open(file_path(), O_RDWR | O_EXLOCK | O_CREAT))); + PLOG_IF(ERROR, !handle.is_valid()) << "open for writing"; + return handle.Pass(); +} + +bool Settings::OpenAndReadSettings(Data* out_data) { + ScopedFileHandle handle = OpenForReading(); + if (!handle.is_valid()) + return false; + + if (ReadSettings(handle.get(), out_data)) + return true; + + // The settings file is corrupt, so reinitialize it. + handle.reset(); + + // The settings failed to be read, so re-initialize them. + return RecoverSettings(kInvalidFileHandle, out_data); +} + +ScopedFileHandle Settings::OpenForWritingAndReadSettings(Data* out_data) { + ScopedFileHandle handle = OpenForReadingAndWriting(); + if (!handle.is_valid()) + return ScopedFileHandle(); + + if (!ReadSettings(handle.get(), out_data)) { + if (!RecoverSettings(handle.get(), out_data)) + return ScopedFileHandle(); + } + + return handle.Pass(); +} + +bool Settings::ReadSettings(FileHandle handle, Data* out_data) { + if (LoggingSeekFile(handle, 0, SEEK_SET) != 0) + return false; + + if (!LoggingReadFile(handle, out_data, sizeof(*out_data))) + return false; + + if (out_data->version != Data::kSettingsVersion) { + LOG(ERROR) << "Settings version is not " << Data::kSettingsVersion; + return false; + } + + return true; +} + +bool Settings::WriteSettings(FileHandle handle, const Data& data) { + if (LoggingSeekFile(handle, 0, SEEK_SET) != 0) + return false; + + if (HANDLE_EINTR(ftruncate(handle, 0)) != 0) { + PLOG(ERROR) << "ftruncate settings file"; + return false; + } + + return LoggingWriteFile(handle, &data, sizeof(Data)); +} + +bool Settings::RecoverSettings(FileHandle handle, Data* out_data) { + ScopedFileHandle scoped_handle; + if (handle == kInvalidFileHandle) { + scoped_handle.reset(OpenForReadingAndWriting().release()); + handle = scoped_handle.get(); + + // Test if the file has already been recovered now that the exclusive lock + // is held. + if (ReadSettings(handle, out_data)) + return true; + } + + LOG(INFO) << "Recovering settings file " << file_path(); + + if (handle == kInvalidFileHandle) { + LOG(ERROR) << "Invalid file handle"; + return false; + } + + if (!InitializeSettings(handle)) + return false; + + return ReadSettings(handle, out_data); +} + +bool Settings::InitializeSettings(FileHandle handle) { + uuid_t uuid; + uuid_generate(uuid); + + Data settings; + settings.client_id.InitializeFromBytes(uuid); + + return WriteSettings(handle, settings); +} + +} // namespace crashpad diff --git a/client/settings.h b/client/settings.h new file mode 100644 index 00000000..741561be --- /dev/null +++ b/client/settings.h @@ -0,0 +1,147 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_CLIENT_SETTINGS_H_ +#define CRASHPAD_CLIENT_SETTINGS_H_ + +#include + +#include + +#include "base/basictypes.h" +#include "base/files/file_path.h" +#include "util/file/file_io.h" +#include "util/misc/initialization_state_dcheck.h" +#include "util/misc/uuid.h" + +namespace crashpad { + +//! \brief An interface for accessing and modifying the settings of a +//! CrashReportDatabase. +//! +//! This class must not be instantiated directly, but rather an instance of it +//! should be retrieved via CrashReportDatabase::GetSettings(). +class Settings { + public: + explicit Settings(const base::FilePath& file_path); + ~Settings(); + + bool Initialize(); + + //! \brief Retrieves the immutable identifier for this client, which is used + //! on a server to locate all crash reports from a specific Crashpad + //! database. + //! + //! This is automatically initialized when the database is created. + //! + //! \param[out] client_id The unique client identifier. + //! + //! \return On success, returns `true`, otherwise returns `false` with an + //! error logged. + bool GetClientID(UUID* client_id); + + //! \brief Retrieves the user’s preference for submitting crash reports to a + //! collection server. + //! + //! The default value is `false`. + //! + //! \param[out] enabled Whether crash reports should be uploaded. + //! + //! \return On success, returns `true`, otherwise returns `false` with an + //! error logged. + bool GetUploadsEnabled(bool* enabled); + + //! \brief Sets the user’s preference for submitting crash reports to a + //! collection server. + //! + //! \param[in] enabled Whether crash reports should be uploaded. + //! + //! \return On success, returns `true`, otherwise returns `false` with an + //! error logged. + bool SetUploadsEnabled(bool enabled); + + //! \brief Retrieves the last time at which a report was attempted to be + //! uploaded. + //! + //! The default value is `0` if it has never been set before. + //! + //! \param[out] time The last time at which a report was uploaded. + //! + //! \return On success, returns `true`, otherwise returns `false` with an + //! error logged. + bool GetLastUploadAttemptTime(time_t* time); + + //! \brief Sets the last time at which a report was attempted to be uploaded. + //! + //! This is only meant to be used internally by the CrashReportDatabase. + //! + //! \param[in] time The last time at which a report was uploaded. + //! + //! \return On success, returns `true`, otherwise returns `false` with an + //! error logged. + bool SetLastUploadAttemptTime(time_t time); + + private: + struct Data; + + // Opens the settings file for reading. On error, logs a message and returns + // the invalid handle. + ScopedFileHandle OpenForReading(); + + // Opens the settings file for reading and writing. On error, logs a message + // and returns the invalid handle. + ScopedFileHandle OpenForReadingAndWriting(); + + // Opens the settings file and reads the data. If that fails, an error will + // be logged and the settings will be recovered and re-initialized. If that + // also fails, returns false with additional log data from recovery. + bool OpenAndReadSettings(Data* out_data); + + // Opens the settings file for writing and reads the data. If reading fails, + // recovery is attempted. Returns the opened file handle on success, or the + // invalid file handle on failure, with an error logged. + ScopedFileHandle OpenForWritingAndReadSettings(Data* out_data); + + // Reads the settings from |handle|. Logs an error and returns false on + // failure. This does not perform recovery. + bool ReadSettings(FileHandle handle, Data* out_data); + + // Writes the settings to |handle|. Logs an error and returns false on + // failure. This does not perform recovery. + bool WriteSettings(FileHandle handle, const Data& data); + + // Recovers the settings file by re-initializing the data. If |handle| is the + // invalid handle, this will open the file; if it is not, then it must be the + // result of OpenForReadingAndWriting(). If the invalid handle is passed, the + // caller must not be holding the handle. The new settings data are stored in + // |out_data|. Returns true on success and false on failure, with an error + // logged. + bool RecoverSettings(FileHandle handle, Data* out_data); + + // Initializes a settings file and writes the data to |handle|. Returns true + // on success and false on failure, with an error logged. + bool InitializeSettings(FileHandle handle); + + const char* file_path() { return file_path_.value().c_str(); } + + base::FilePath file_path_; + + InitializationStateDcheck initialized_; + + DISALLOW_COPY_AND_ASSIGN(Settings); +}; + +} // namespace crashpad + +#endif // CRASHPAD_CLIENT_SETTINGS_H_ diff --git a/client/settings_test.cc b/client/settings_test.cc new file mode 100644 index 00000000..debb7c19 --- /dev/null +++ b/client/settings_test.cc @@ -0,0 +1,175 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "client/settings.h" + +#include "client/crash_report_database.h" +#include "gtest/gtest.h" +#include "util/file/file_io.h" +#include "util/test/scoped_temp_dir.h" + +namespace crashpad { +namespace test { +namespace { + +class SettingsTest : public testing::Test { + public: + SettingsTest() : settings_(settings_path()) {} + + base::FilePath settings_path() { + return temp_dir_.path().Append("settings"); + } + + Settings* settings() { return &settings_; } + + void InitializeBadFile() { + ScopedFileHandle handle( + LoggingOpenFileForWrite(settings_path(), + FileWriteMode::kTruncateOrCreate, + FilePermissions::kWorldReadable)); + ASSERT_TRUE(handle.is_valid()); + + const char kBuf[] = "test bad file"; + ASSERT_TRUE(LoggingWriteFile(handle.get(), kBuf, sizeof(kBuf))); + handle.reset(); + } + + protected: + // testing::Test: + void SetUp() override { + ASSERT_TRUE(settings()->Initialize()); + } + + private: + ScopedTempDir temp_dir_; + Settings settings_; + + DISALLOW_COPY_AND_ASSIGN(SettingsTest); +}; + +TEST_F(SettingsTest, ClientID) { + UUID client_id; + EXPECT_TRUE(settings()->GetClientID(&client_id)); + EXPECT_NE(UUID(), client_id); + + Settings local_settings(settings_path()); + EXPECT_TRUE(local_settings.Initialize()); + UUID actual; + EXPECT_TRUE(local_settings.GetClientID(&actual)); + EXPECT_EQ(client_id, actual); +} + +TEST_F(SettingsTest, UploadsEnabled) { + bool enabled = true; + // Default value is false. + EXPECT_TRUE(settings()->GetUploadsEnabled(&enabled)); + EXPECT_FALSE(enabled); + + EXPECT_TRUE(settings()->SetUploadsEnabled(true)); + EXPECT_TRUE(settings()->GetUploadsEnabled(&enabled)); + EXPECT_TRUE(enabled); + + Settings local_settings(settings_path()); + EXPECT_TRUE(local_settings.Initialize()); + enabled = false; + EXPECT_TRUE(local_settings.GetUploadsEnabled(&enabled)); + EXPECT_TRUE(enabled); + + EXPECT_TRUE(settings()->SetUploadsEnabled(false)); + EXPECT_TRUE(settings()->GetUploadsEnabled(&enabled)); + EXPECT_FALSE(enabled); + + enabled = true; + EXPECT_TRUE(local_settings.GetUploadsEnabled(&enabled)); + EXPECT_FALSE(enabled); +} + +TEST_F(SettingsTest, LastUploadAttemptTime) { + time_t actual = -1; + EXPECT_TRUE(settings()->GetLastUploadAttemptTime(&actual)); + // Default value is 0. + EXPECT_EQ(0, actual); + + const time_t expected = time(nullptr); + EXPECT_TRUE(settings()->SetLastUploadAttemptTime(expected)); + EXPECT_TRUE(settings()->GetLastUploadAttemptTime(&actual)); + EXPECT_EQ(expected, actual); + + Settings local_settings(settings_path()); + EXPECT_TRUE(local_settings.Initialize()); + actual = -1; + EXPECT_TRUE(local_settings.GetLastUploadAttemptTime(&actual)); + EXPECT_EQ(expected, actual); +} + +// The following tests write a corrupt settings file and test the recovery +// operation. + +TEST_F(SettingsTest, BadFileOnInitialize) { + InitializeBadFile(); + + Settings settings(settings_path()); + EXPECT_TRUE(settings.Initialize()); +} + +TEST_F(SettingsTest, BadFileOnGet) { + InitializeBadFile(); + + UUID client_id; + EXPECT_TRUE(settings()->GetClientID(&client_id)); + EXPECT_NE(UUID(), client_id); + + Settings local_settings(settings_path()); + EXPECT_TRUE(local_settings.Initialize()); + UUID actual; + EXPECT_TRUE(local_settings.GetClientID(&actual)); + EXPECT_EQ(client_id, actual); +} + +TEST_F(SettingsTest, BadFileOnSet) { + InitializeBadFile(); + + EXPECT_TRUE(settings()->SetUploadsEnabled(true)); + bool enabled = false; + EXPECT_TRUE(settings()->GetUploadsEnabled(&enabled)); + EXPECT_TRUE(enabled); +} + +TEST_F(SettingsTest, UnlinkFile) { + UUID client_id; + EXPECT_TRUE(settings()->GetClientID(&client_id)); + EXPECT_TRUE(settings()->SetUploadsEnabled(true)); + EXPECT_TRUE(settings()->SetLastUploadAttemptTime(time(nullptr))); + + EXPECT_EQ(0, unlink(settings_path().value().c_str())); + + Settings local_settings(settings_path()); + EXPECT_TRUE(local_settings.Initialize()); + UUID new_client_id; + EXPECT_TRUE(local_settings.GetClientID(&new_client_id)); + EXPECT_NE(client_id, new_client_id); + + // Check that all values are reset. + bool enabled = true; + EXPECT_TRUE(local_settings.GetUploadsEnabled(&enabled)); + EXPECT_FALSE(enabled); + + time_t time = -1; + EXPECT_TRUE(local_settings.GetLastUploadAttemptTime(&time)); + EXPECT_EQ(0, time); +} + +} // namespace +} // namespace test +} // namespace crashpad