Error when no prefix is set before retrieve setting

rcaelers · Sep 17, 2024 · d4c84d5 · d4c84d5
1 parent abf2730
commit d4c84d5
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 7 deletions.
diff --git a/src/UpgradeControl.cc b/src/UpgradeControl.cc
@@ -77,7 +77,6 @@ UpgradeControl::UpgradeControl(std::shared_ptr<Platform> platform,
   http->options().set_max_redirects(5);
   http->options().set_follow_redirects(true);
   init_periodic_update_check();
-  init_priority();
 }
 
 UpgradeControl::UpgradeControl(std::shared_ptr<Platform> platform,
@@ -100,7 +99,6 @@ UpgradeControl::UpgradeControl(std::shared_ptr<Platform> platform,
   , check_timer(io_context.get_io_context())
 {
   init_periodic_update_check();
-  init_priority();
 }
 
 outcome::std_result<void>
@@ -144,6 +142,7 @@ UpgradeControl::set_periodic_update_check_enabled(bool enabled)
 {
   if (periodic_update_check_enabled != enabled)
     {
+      init_priority();
       periodic_update_check_enabled = enabled;
       update_check_timer();
     }

diff --git a/src/windows/WindowsSettingsStorage.cc b/src/windows/WindowsSettingsStorage.cc
@@ -47,6 +47,12 @@ WindowsSettingsStorage::set_prefix(const std::string &prefix)
 outcome::std_result<void>
 WindowsSettingsStorage::remove_key(const std::string &name)
 {
+  if (subkey_.empty())
+    {
+      logger->error("no prefix set for registry settings");
+      return outcome::failure(UnfoldInternalErrc::InvalidSetting);
+    }
+
   HKEY key = nullptr;
   LONG err = RegOpenKeyEx(HKEY_CURRENT_USER, subkey_.c_str(), 0, KEY_ALL_ACCESS, &key);
   if (err != ERROR_SUCCESS)
@@ -71,6 +77,12 @@ WindowsSettingsStorage::remove_key(const std::string &name)
 outcome::std_result<SettingValue>
 WindowsSettingsStorage::get_value(const std::string &name, SettingType type) const
 {
+  if (subkey_.empty())
+    {
+      logger->error("no prefix set for registry settings");
+      return outcome::failure(UnfoldInternalErrc::InvalidSetting);
+    }
+
   HKEY key = nullptr;
 
   LONG err = RegOpenKeyEx(HKEY_CURRENT_USER, subkey_.c_str(), 0, KEY_ALL_ACCESS, &key);
@@ -135,6 +147,12 @@ WindowsSettingsStorage::get_value(const std::string &name, SettingType type) con
 outcome::std_result<void>
 WindowsSettingsStorage::set_value(const std::string &name, const SettingValue &value)
 {
+  if (subkey_.empty())
+    {
+      logger->error("no prefix set for registry settings");
+      return outcome::failure(UnfoldInternalErrc::InvalidSetting);
+    }
+
   return std::visit(
     [this, name](auto &&arg) -> outcome::std_result<void> {
       using T = std::decay_t<decltype(arg)>;

diff --git a/test/IntegrationTests.cc b/test/IntegrationTests.cc
@@ -240,6 +240,7 @@ TEST_F(IntegrationTest, CheckAlpha)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);
@@ -439,7 +440,7 @@ TEST_F(IntegrationTest, UpgradeLastUpgradeTime)
   UpgradeControl control(platform, time_source, io_context);
 
   control.set_configuration_prefix("Software\\Unfold\\Test");
-
+  
   auto rc = control.set_appcast("https://127.0.0.1:1337/appcast.xml");
   EXPECT_EQ(rc.has_error(), false);
 
@@ -491,6 +492,7 @@ TEST_F(IntegrationTest, PeriodicCheckLater)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);
@@ -532,6 +534,7 @@ TEST_F(IntegrationTest, PeriodicCheckSkip)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);
@@ -575,6 +578,7 @@ TEST_F(IntegrationTest, PeriodicCheckCanaryLowPrio)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);
@@ -651,6 +655,7 @@ TEST_F(IntegrationTest, PeriodicCheckCanaryHighPrio)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);
@@ -727,6 +732,7 @@ TEST_F(IntegrationTest, PeriodicCheckInstallNow)
   EXPECT_EQ(rc.has_error(), false);
 
   control.set_certificate(cert);
+  control.set_configuration_prefix("Software\\Unfold\\Test");
 
   rc = control.set_signature_verification_key("MCowBQYDK2VwAyEA0vkFT/GcU/NEM9xoDqhiYK3/EaTXVAI95MOt+SnjCpM=");
   EXPECT_EQ(rc.has_error(), false);

diff --git a/test/UpgradeControlTests.cc b/test/UpgradeControlTests.cc
@@ -89,8 +89,6 @@ struct UpgradeControlTest : public ::testing::Test
     time_source = std::make_shared<TimeSourceMock>();
     installer = std::make_shared<InstallerMock>();
 
-    EXPECT_CALL(*storage, set_value("Priority", _)).Times(1).WillRepeatedly(Return(outcome::success()));
-
     EXPECT_CALL(*storage, get_value(::testing::_, ::testing::_)).WillRepeatedly(::testing::DoDefault());
     ON_CALL(*storage, get_value("Priority", SettingType::Int32)).WillByDefault(Return(outcome::success(0)));
 
@@ -164,6 +162,8 @@ TEST_F(UpgradeControlTest, PeriodicCheckLater)
   control->set_configuration_prefix("some\\prefix");
   control->set_configuration_prefix("some\\wrong\\prefix");
 
+  EXPECT_CALL(*storage, set_value("Priority", _)).Times(1).WillRepeatedly(Return(outcome::success()));
+
   EXPECT_CALL(*storage, get_value("LastUpdateCheckTime", SettingType::Int64))
     .Times(AtLeast(1))
     .WillRepeatedly(Return(outcome::success(32LL)));
@@ -244,6 +244,7 @@ TEST_F(UpgradeControlTest, PeriodicCheckLaterLastCheckInFuture)
     .WillRepeatedly(Return(outcome::success(
       static_cast<int64_t>(std::chrono::duration_cast<std::chrono::microseconds>(now.time_since_epoch()).count() + 3600))));
 
+  EXPECT_CALL(*storage, set_value("Priority", _)).Times(1).WillRepeatedly(Return(outcome::success()));
   EXPECT_CALL(*storage, set_value("LastUpdateCheckTime", _)).Times(AtLeast(1)).WillRepeatedly(Return(outcome::success()));
   EXPECT_CALL(*storage, get_value("SkipVersion", SettingType::String))
     .Times(AtLeast(1))
@@ -318,6 +319,7 @@ TEST_F(UpgradeControlTest, PeriodicCheckError)
     .Times(AtLeast(1))
     .WillRepeatedly(Return(outcome::success(32LL)));
 
+  EXPECT_CALL(*storage, set_value("Priority", _)).Times(1).WillRepeatedly(Return(outcome::success()));
   EXPECT_CALL(*storage, set_value("LastUpdateCheckTime", _)).Times(AtLeast(1)).WillRepeatedly(Return(outcome::success()));
   EXPECT_CALL(*storage, set_value("SkipVersion", SettingValue{""})).Times(1).WillRepeatedly(Return(outcome::success()));
 

diff --git a/test/WindowsTests.cc b/test/WindowsTests.cc
@@ -26,7 +26,7 @@
 #include "windows/WindowsSettingsStorage.hh"
 #include "windows/WindowsPlatform.hh"
 
-TEST(Windows, WindowsSettingsString)
+TEST(WindowsTest, WindowsSettingsString)
 {
   WindowsSettingsStorage storage;
 
@@ -62,7 +62,7 @@ TEST(WindowsTest, SettingsInt64)
   EXPECT_EQ(SettingValueToType(s.value()), SettingType::Int64);
 }
 
-TEST(WindowsTest,SettingsInt32)
+TEST(WindowsTest, SettingsInt32)
 {
   WindowsSettingsStorage storage;
 
@@ -214,6 +214,18 @@ TEST(WindowsTest, PlatformIsSupported)
 #endif
 }
 
+TEST(WindowsTest, SettingsNoPrefix)
+{
+  WindowsSettingsStorage storage;
+
+  auto rc = storage.set_value("foo", 42LL);
+  EXPECT_TRUE(rc.has_error());
+  auto s = storage.get_value("foo", SettingType::Int64);
+  EXPECT_TRUE(s.has_error());
+  rc = storage.remove_key("foo");
+  EXPECT_TRUE(rc.has_error());
+}
+
 // TEST(WindowsTest, PlatformIsSupportedOsVersion)
 // {
 //   WindowsPlatform platform;