fix: Add more constraints to config (#1831)

Log file size and rotation should also not allowed to be 0.

Author: PeterChen13579

src/util/newconfig/ConfigConstraints.hpp

@@ -362,6 +362,10 @@ static constinit NumberValueConstraint<uint16_t> gValidateUint16{
     std::numeric_limits<uint16_t>::min(),
     std::numeric_limits<uint16_t>::max()
 };
+
+// log file size minimum is 1mb, log rotation time minimum is 1hr
+static constinit NumberValueConstraint<uint32_t> gValidateLogSize{1, std::numeric_limits<uint32_t>::max()};
+static constinit NumberValueConstraint<uint32_t> gValidateLogRotationTime{1, std::numeric_limits<uint32_t>::max()};
 static constinit NumberValueConstraint<uint32_t> gValidateUint32{
     std::numeric_limits<uint32_t>::min(),
     std::numeric_limits<uint32_t>::max()

src/util/newconfig/ConfigDefinition.hpp

@@ -350,8 +350,9 @@ static ClioConfigDefinition gClioConfig = ClioConfigDefinition{
      {"server.admin_password", ConfigValue{ConfigType::String}.optional()},
      {"server.processing_policy",
       ConfigValue{ConfigType::String}.defaultValue("parallel").withConstraint(gValidateProcessingPolicy)},
-     {"server.parallel_requests_limit", ConfigValue{ConfigType::Integer}.optional()},
-     {"server.ws_max_sending_queue_size", ConfigValue{ConfigType::Integer}.defaultValue(1500)},
+     {"server.parallel_requests_limit", ConfigValue{ConfigType::Integer}.optional().withConstraint(gValidateUint16)},
+     {"server.ws_max_sending_queue_size",
+      ConfigValue{ConfigType::Integer}.defaultValue(1500).withConstraint(gValidateUint32)},
      {"server.__ng_web_server", ConfigValue{ConfigType::Boolean}.defaultValue(false)},
 
      {"prometheus.enabled", ConfigValue{ConfigType::Boolean}.defaultValue(true)},
@@ -387,12 +388,13 @@ static ClioConfigDefinition gClioConfig = ClioConfigDefinition{
 
      {"log_directory", ConfigValue{ConfigType::String}.optional()},
 
-     {"log_rotation_size", ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(gValidateUint32)},
+     {"log_rotation_size", ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(gValidateLogSize)},
 
-     {"log_directory_max_size", ConfigValue{ConfigType::Integer}.defaultValue(50 * 1024).withConstraint(gValidateUint32)
-     },
+     {"log_directory_max_size",
+      ConfigValue{ConfigType::Integer}.defaultValue(50 * 1024).withConstraint(gValidateLogSize)},
 
-     {"log_rotation_hour_interval", ConfigValue{ConfigType::Integer}.defaultValue(12).withConstraint(gValidateUint32)},
+     {"log_rotation_hour_interval",
+      ConfigValue{ConfigType::Integer}.defaultValue(12).withConstraint(gValidateLogRotationTime)},
 
      {"log_tag_style", ConfigValue{ConfigType::String}.defaultValue("none").withConstraint(gValidateLogTag)},
 

tests/unit/LoggerTests.cpp

@@ -31,8 +31,12 @@
 #include <fmt/core.h>
 #include <gtest/gtest.h>
 
+#include <cstddef>
+#include <cstdint>
+#include <limits>
 #include <string>
 #include <string_view>
+#include <vector>
 using namespace util;
 
 // Used as a fixture for tests with enabled logging
@@ -90,11 +94,14 @@ struct LoggerInitTest : LoggerTest {
 
         {"log_directory", ConfigValue{ConfigType::String}.optional()},
 
-        {"log_rotation_size", ConfigValue{ConfigType::Integer}.defaultValue(2048)},
+        {"log_rotation_size",
+         ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(config::gValidateLogSize)},
 
-        {"log_directory_max_size", ConfigValue{ConfigType::Integer}.defaultValue(50 * 1024)},
+        {"log_directory_max_size",
+         ConfigValue{ConfigType::Integer}.defaultValue(50 * 1024).withConstraint(config::gValidateLogSize)},
 
-        {"log_rotation_hour_interval", ConfigValue{ConfigType::Integer}.defaultValue(12)},
+        {"log_rotation_hour_interval",
+         ConfigValue{ConfigType::Integer}.defaultValue(12).withConstraint(config::gValidateLogRotationTime)},
 
         {"log_tag_style", ConfigValue{ConfigType::String}.defaultValue("none")},
     };
@@ -168,6 +175,23 @@ TEST_F(LoggerInitTest, ChannelLogLevel)
     }
 }
 
+TEST_F(LoggerInitTest, LogSizeAndHourRotationCannotBeZero)
+{
+    std::vector<std::string_view> const keys{
+        "log_rotation_hour_interval", "log_directory_max_size", "log_rotation_size"
+    };
+
+    auto const parsingErrors =
+        config_.parse(ConfigFileJson{boost::json::object{{keys[0], 0}, {keys[1], 0}, {keys[2], 0}}});
+    ASSERT_TRUE(parsingErrors->size() == 3);
+    for (std::size_t i = 0; i < parsingErrors->size(); ++i) {
+        EXPECT_EQ(
+            (*parsingErrors)[i].error,
+            fmt::format("{} Number must be between 1 and {}", keys[i], std::numeric_limits<uint32_t>::max())
+        );
+    }
+}
+
 #ifndef COVERAGE_ENABLED
 TEST_F(LoggerTest, LOGMacro)
 {