From: Andrei Pavel <andrei@isc.org>
Date: Fri, 19 Nov 2021 12:52:29 +0000 (+0200)
Subject: [#2130] throw exception if maxsize is too lage
X-Git-Tag: Kea-2.1.1~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=efa7213ae1ed99e4abbb59c6157717805a3eba30;p=thirdparty%2Fkea.git

[#2130] throw exception if maxsize is too lage
---

diff --git a/src/lib/log/logger_manager_impl.cc b/src/lib/log/logger_manager_impl.cc
index cb74cb8f55..a29cd0e76c 100644
--- a/src/lib/log/logger_manager_impl.cc
+++ b/src/lib/log/logger_manager_impl.cc
@@ -151,9 +151,18 @@ LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
             ++i;
         }
         std::array<std::string, 3> const suffixes({"", "KB", "MB"});
-        std::string const maxFileSize(to_string(maxsize) + suffixes[i]);
-        properties.setProperty("MaxFileSize", maxFileSize);
+        std::string const max_file_size(to_string(maxsize) + suffixes[i]);
+
+        // If maxsize is still past INT_MAX, it will overflow in log4cplus,
+        // so stop here instead.
+        if (std::numeric_limits<int32_t>::max() < maxsize) {
+            isc_throw(BadValue, "expected maxsize < "
+                                    << std::numeric_limits<int32_t>::max()
+                                    << "MB , but instead got "
+                                    << max_file_size);
+        }
 
+        properties.setProperty("MaxFileSize", max_file_size);
         properties.setProperty("MaxBackupIndex",
                                lexical_cast<string>(opt.maxver));
         properties.setProperty("ImmediateFlush", opt.flush ? "true" : "false");
diff --git a/src/lib/log/tests/logger_manager_unittest.cc b/src/lib/log/tests/logger_manager_unittest.cc
index c569fa379a..809b693ded 100644
--- a/src/lib/log/tests/logger_manager_unittest.cc
+++ b/src/lib/log/tests/logger_manager_unittest.cc
@@ -28,8 +28,8 @@
 #include <log/logger_specification.h>
 #include <log/message_initializer.h>
 #include <log/output_option.h>
-
 #include <log/tests/tempdir.h>
+#include <testutils/gtest_utils.h>
 
 #include <sys/types.h>
 #include <regex.h>
@@ -254,7 +254,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     opt->maxsize = SIZE_LIMIT;    // Bytes
     opt->maxver = 2;
 
-    // The current current output file does not exist (the creation of file_spec
+    // The current output file does not exist (the creation of file_spec
     // ensures that.  Check that previous versions don't either.
     vector<string> prev_name;
     for (int i = 0; i < 3; ++i) {
@@ -320,6 +320,24 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     }
 }
 
+// Check if an exception is thrown if maxsize is too large.
+TEST_F(LoggerManagerTest, TooLargeMaxsize) {
+    // Set up the name of the file.
+    SpecificationForFileLogger file_spec;
+    LoggerSpecification& spec(file_spec.getSpecification());
+
+    // UINT64_MAX should be large enough.
+    LoggerSpecification::iterator opt = spec.begin();
+    EXPECT_TRUE(opt != spec.end());
+    opt->maxsize = std::numeric_limits<uint64_t>::max();  // bytes
+
+    // Set up the file logger.
+    LoggerManager manager;
+    EXPECT_THROW_MSG(manager.process(spec), BadValue,
+                     "expected maxsize < 2147483647MB , but instead got "
+                     "18446744073709MB");
+}
+
 namespace { // begin unnamed namespace
 
 // When we begin to use C++11, we could replace use of POSIX API with
diff --git a/src/lib/process/tests/log_parser_unittests.cc b/src/lib/process/tests/log_parser_unittests.cc
index 247536674b..f32580c4a8 100644
--- a/src/lib/process/tests/log_parser_unittests.cc
+++ b/src/lib/process/tests/log_parser_unittests.cc
@@ -487,7 +487,8 @@ void testMaxSize(uint64_t maxsize_candidate, uint64_t expected_maxsize) {
     ASSERT_NO_THROW(parser.parseConfiguration(config));
     ASSERT_NO_THROW(server_cfg->applyLoggingCfg());
 
-    EXPECT_EQ(server_cfg->getLoggingInfo()[0].destinations_[0].maxsize_, expected_maxsize);
+    EXPECT_EQ(server_cfg->getLoggingInfo()[0].destinations_[0].maxsize_,
+              expected_maxsize);
 }
 
 // Test that maxsize can be configured with high values.