This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 85daf4e9 overflow in toFileSize causing incorrect log rotation limits
(#646)
85daf4e9 is described below
commit 85daf4e9e24086dad15e25c30b5b83e1583ac776
Author: jmestwa-coder <[email protected]>
AuthorDate: Thu May 7 13:36:57 2026 +0530
overflow in toFileSize causing incorrect log rotation limits (#646)
---
src/main/cpp/optionconverter.cpp | 44 +++++++++++++++++++++-------
src/test/cpp/rollingfileappendertestcase.cpp | 12 ++++++++
2 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 2b43060e..5935400a 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -25,6 +25,8 @@
#include <log4cxx/helpers/stringhelper.h>
#include <log4cxx/helpers/exception.h>
#include <stdlib.h>
+#include <cerrno>
+#include <limits>
#include <log4cxx/helpers/properties.h>
#include <log4cxx/helpers/loglog.h>
#include <log4cxx/level.h>
@@ -281,35 +283,57 @@ int OptionConverter::toInt(const LogString& value, int
dEfault)
long OptionConverter::toFileSize(const LogString& s, long dEfault)
{
- if (s.empty())
+ LogString trimmed(StringHelper::trim(s));
+ if (trimmed.empty())
{
return dEfault;
}
- size_t index = s.find_first_of(LOG4CXX_STR("bB"));
+ size_t index = trimmed.find_first_of(LOG4CXX_STR("bB"));
+ long long multiplier = 1;
+ LogString numericPart = trimmed;
if (index != LogString::npos && index > 0)
{
- long multiplier = 1;
- index--;
-
- if (s[index] == 0x6B /* 'k' */ || s[index] == 0x4B /* 'K' */)
+ size_t suffixIndex = index - 1;
+ if (trimmed[suffixIndex] == 0x6B /* 'k' */ ||
trimmed[suffixIndex] == 0x4B /* 'K' */)
{
multiplier = 1024;
}
- else if (s[index] == 0x6D /* 'm' */ || s[index] == 0x4D /* 'M'
*/)
+ else if (trimmed[suffixIndex] == 0x6D /* 'm' */ ||
trimmed[suffixIndex] == 0x4D /* 'M' */)
{
multiplier = 1024 * 1024;
}
- else if (s[index] == 0x67 /* 'g'*/ || s[index] == 0x47 /* 'G'
*/)
+ else if (trimmed[suffixIndex] == 0x67 /* 'g'*/ ||
trimmed[suffixIndex] == 0x47 /* 'G' */)
{
multiplier = 1024 * 1024 * 1024;
}
- return toInt(s.substr(0, index), 1) * multiplier;
+ numericPart = trimmed.substr(0, suffixIndex);
+ }
+
+ LOG4CXX_ENCODE_CHAR(cvalue, numericPart);
+ char* endptr;
+ errno = 0;
+ long long parsed = strtoll(cvalue.c_str(), &endptr, 10);
+
+ if (endptr == cvalue.c_str() || errno == ERANGE || parsed < 0)
+ {
+ return dEfault;
+ }
+
+ if (multiplier != 0 && parsed > std::numeric_limits<long long>::max() /
multiplier)
+ {
+ return dEfault;
+ }
+
+ long long scaled = parsed * multiplier;
+ if (scaled > std::numeric_limits<long>::max())
+ {
+ return dEfault;
}
- return toInt(s, 1);
+ return static_cast<long>(scaled);
}
LogString OptionConverter::findAndSubst(const LogString& key, Properties&
props)
diff --git a/src/test/cpp/rollingfileappendertestcase.cpp
b/src/test/cpp/rollingfileappendertestcase.cpp
index f77f116b..f9c4d92b 100644
--- a/src/test/cpp/rollingfileappendertestcase.cpp
+++ b/src/test/cpp/rollingfileappendertestcase.cpp
@@ -32,11 +32,23 @@ class RollingFileAppenderTestCase : public
FileAppenderAbstractTestCase
//
LOGUNIT_TEST(testDefaultThreshold);
LOGUNIT_TEST(testSetOptionThreshold);
+ LOGUNIT_TEST(testMaxFileSizeOverflow);
LOGUNIT_TEST_SUITE_END();
public:
+ void testMaxFileSizeOverflow()
+ {
+ log4cxx::rolling::RollingFileAppender rfa;
+ // Set an extremely large value that would cause
overflow in the buggy version
+ rfa.setOption(LOG4CXX_STR("MaxFileSize"),
LOG4CXX_STR("9999999999999999999999GB"));
+ size_t currentMax = rfa.getMaximumFileSize();
+
+ // In the buggy version, currentMax would be a huge
positive value (size_t conversion of a negative long)
+ // In the fixed version, it should return the default
value or at least be reasonable.
+ LOGUNIT_ASSERT(currentMax <= (size_t)2000000000ULL);
+ }
FileAppender* createFileAppender() const
{
