diff --git a/internal/platform/implementation/windows/preferences_manager.cc b/internal/platform/implementation/windows/preferences_manager.cc index c588bce5..0e89d26d 100644 --- a/internal/platform/implementation/windows/preferences_manager.cc +++ b/internal/platform/implementation/windows/preferences_manager.cc @@ -194,6 +194,12 @@ bool PreferencesManager::Commit() { } bool PreferencesManager::SetValue(absl::string_view key, const json& value) { + if (!value_.is_object()) { + NEARBY_LOGS(ERROR) << "Preferences is no longer an object! value_=" + << value_.dump(4); + value_ = json::object(); + } + if (value_[absl::StrCat(key)] == value) { return false; } @@ -205,6 +211,12 @@ bool PreferencesManager::SetValue(absl::string_view key, const json& value) { template T PreferencesManager::GetValue(absl::string_view key, const T& default_value) const { + if (!value_.is_object()) { + NEARBY_LOGS(ERROR) << "Preferences is no longer an object! value_=" + << value_.dump(4); + return default_value; + } + auto it = value_.find(absl::StrCat(key)); if (it == value_.end()) { return default_value; @@ -215,6 +227,12 @@ T PreferencesManager::GetValue(absl::string_view key, template bool PreferencesManager::SetArrayValue(absl::string_view key, absl::Span value) { + if (!value_.is_object()) { + NEARBY_LOGS(ERROR) << "Preferences is no longer an object! value_=" + << value_.dump(4); + value_ = json::object(); + } + json array_value = json::array(); for (const T& item_value : value) { array_value.push_back(item_value); @@ -233,6 +251,16 @@ std::vector PreferencesManager::GetArrayValue( absl::string_view key, absl::Span default_value) const { std::vector result; + if (!value_.is_object()) { + NEARBY_LOGS(ERROR) << "Preferences is no longer an object! value_=" + << value_.dump(4); + + for (const T& value : default_value) { + result.push_back(value); + } + return result; + } + auto array_value = value_.find(absl::StrCat(key)); if (array_value == value_.end() || !array_value->is_array()) { for (const T& value : default_value) { diff --git a/internal/platform/implementation/windows/preferences_manager_test.cc b/internal/platform/implementation/windows/preferences_manager_test.cc index 04a7a799..243182ae 100644 --- a/internal/platform/implementation/windows/preferences_manager_test.cc +++ b/internal/platform/implementation/windows/preferences_manager_test.cc @@ -16,8 +16,10 @@ #include +#include #include // NOLINT(build/c++17) #include +#include #include #include #include @@ -29,6 +31,7 @@ #include "absl/types/span.h" #include "nlohmann/json.hpp" #include "nlohmann/json_fwd.hpp" +#include "internal/platform/logging.h" namespace nearby { namespace windows { @@ -38,27 +41,27 @@ constexpr absl::Duration kTimeOut = absl::Milliseconds(200); constexpr char kPreferencesFilePath[] = "Google/Nearby/Sharing"; } // namespace + TEST(PreferencesManager, CorruptedConfigFile) { std::filesystem::path settingsPath = - std::filesystem::temp_directory_path() / "settings.json"; - std::ofstream output_stream{settingsPath}; - output_stream << "{\"data\":8, \"names\": [\"In valid\"}" << std::endl; - output_stream.close(); + std::filesystem::temp_directory_path(); + std::ofstream output_stream{settingsPath / "preferences.json"}; + output_stream << "CORRUPTED" << std::endl; - // Should use an empty setting for a corrupted configuration file. - EXPECT_EQ(PreferencesManager(kPreferencesFilePath).GetInteger("data", 100), + NEARBY_LOGS(INFO) << "Loading preferences from: " << settingsPath.string(); + EXPECT_EQ(PreferencesManager(settingsPath.string()).GetInteger("data", 100), 100); } TEST(PreferencesManager, ValidConfigFile) { std::filesystem::path settingsPath = - std::filesystem::temp_directory_path() / "settings.json"; - std::ofstream output_stream{settingsPath}; + std::filesystem::temp_directory_path(); + std::ofstream output_stream{settingsPath / "preferences.json"}; output_stream << "{\"data\":8, \"name\": \"Valid\"}" << std::endl; output_stream.close(); - // Should use an empty setting for a corrupted configuration file. - EXPECT_EQ(PreferencesManager(kPreferencesFilePath).GetInteger("data", 100), + NEARBY_LOGS(INFO) << "Loading preferences from: " << settingsPath.string(); + EXPECT_EQ(PreferencesManager(settingsPath.string()).GetInteger("data", 100), 8); } diff --git a/internal/platform/implementation/windows/preferences_repository.cc b/internal/platform/implementation/windows/preferences_repository.cc index 5c323c20..9f7e1fd2 100644 --- a/internal/platform/implementation/windows/preferences_repository.cc +++ b/internal/platform/implementation/windows/preferences_repository.cc @@ -37,6 +37,15 @@ json PreferencesRepository::LoadPreferences() { absl::MutexLock lock(&mutex_); std::optional preferences = AttemptLoad(); if (preferences.has_value()) { + // The top level root should be an object, if it's not then something went + // wrong or the file was corrupted. + if (!preferences.value().is_object()) { + NEARBY_LOGS(ERROR) << "Preferences loaded was not a valid object: " + << preferences.value().dump(4); + + return json::object(); + } + return preferences.value(); } diff --git a/internal/platform/implementation/windows/preferences_repository_test.cc b/internal/platform/implementation/windows/preferences_repository_test.cc index 78794a00..d1732de8 100644 --- a/internal/platform/implementation/windows/preferences_repository_test.cc +++ b/internal/platform/implementation/windows/preferences_repository_test.cc @@ -40,6 +40,25 @@ TEST(PreferencesRepository, LoadWithBadPath) { EXPECT_TRUE(result.empty()); } +TEST(PreferencesRepository, RecoverFromBadPreferences) { + std::optional app_data_path = + api::ImplementationPlatform::CreateDeviceInfo()->GetLocalAppDataPath(); + ASSERT_TRUE(app_data_path.has_value()); + std::filesystem::path full_path = *app_data_path / kPreferencesPath; + std::filesystem::path full_name = full_path / kPreferencesFileName; + + if (std::filesystem::exists(full_name)) { + std::filesystem::remove(full_name); + } + + std::ofstream pref_file(full_name.c_str()); + pref_file << "\"Bad top level object\""; + pref_file.close(); + + PreferencesRepository preferences_repository{full_path.string()}; + EXPECT_EQ(preferences_repository.LoadPreferences(), json::object()); +} + TEST(PreferencesRepository, SaveAndLoadPreferences) { std::optional app_data_path = api::ImplementationPlatform::CreateDeviceInfo()->GetLocalAppDataPath();