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 b4cde331 Harden OptionConverter::toFileSize input validation (#654)
b4cde331 is described below
commit b4cde3316372bf9fe21b3c803278601985afff0e
Author: jmestwa-coder <[email protected]>
AuthorDate: Sun May 10 08:33:42 2026 +0530
Harden OptionConverter::toFileSize input validation (#654)
---
src/main/cpp/optionconverter.cpp | 57 ++++++++++++-------
src/test/cpp/helpers/optionconvertertestcase.cpp | 70 ++++++++++++++++++++++++
2 files changed, 108 insertions(+), 19 deletions(-)
diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 70efa29c..961c304b 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -21,6 +21,7 @@
#include <log4cxx/appenderskeleton.h>
#include <log4cxx/helpers/optionconverter.h>
#include <algorithm>
+#include <cctype>
#include <ctype.h>
#include <log4cxx/helpers/stringhelper.h>
#include <log4cxx/helpers/exception.h>
@@ -50,6 +51,14 @@ namespace
{
using namespace LOG4CXX_NS;
+void skipWhitespace(char*& p)
+{
+ while (std::isspace(static_cast<unsigned char>(*p)))
+ {
+ ++p;
+ }
+}
+
/// For recursion checking
struct LogStringChain
{
@@ -300,37 +309,47 @@ long OptionConverter::toFileSize(const LogString& s, long
dEfault)
return dEfault;
}
- size_t index = trimmed.find_first_of(LOG4CXX_STR("bB"));
-
long long multiplier = 1;
- LogString numericPart = trimmed;
- if (index != LogString::npos && index > 0)
+ LOG4CXX_ENCODE_CHAR(cvalue, trimmed);
+ char* endptr;
+ errno = 0;
+ long long parsed = strtoll(cvalue.c_str(), &endptr, 10);
+
+ if (endptr == cvalue.c_str() || errno == ERANGE || parsed < 0)
{
- size_t suffixIndex = index - 1;
- if (trimmed[suffixIndex] == 0x6B /* 'k' */ ||
trimmed[suffixIndex] == 0x4B /* 'K' */)
+ return dEfault;
+ }
+
+ skipWhitespace(endptr);
+ if (*endptr != '\0')
+ {
+ const char unit =
static_cast<char>(std::toupper(static_cast<unsigned char>(*endptr)));
+ if (unit == 'K')
{
multiplier = 1024;
+ ++endptr;
}
- else if (trimmed[suffixIndex] == 0x6D /* 'm' */ ||
trimmed[suffixIndex] == 0x4D /* 'M' */)
+ else if (unit == 'M')
{
multiplier = 1024 * 1024;
+ ++endptr;
}
- else if (trimmed[suffixIndex] == 0x67 /* 'g'*/ ||
trimmed[suffixIndex] == 0x47 /* 'G' */)
+ else if (unit == 'G')
{
multiplier = 1024 * 1024 * 1024;
+ ++endptr;
}
- 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 (static_cast<char>(std::toupper(static_cast<unsigned
char>(*endptr))) != 'B')
+ {
+ return dEfault;
+ }
+ ++endptr;
+ skipWhitespace(endptr);
+ if (*endptr != '\0')
+ {
+ return dEfault;
+ }
}
if (multiplier != 0 && parsed > std::numeric_limits<long long>::max() /
multiplier)
diff --git a/src/test/cpp/helpers/optionconvertertestcase.cpp
b/src/test/cpp/helpers/optionconvertertestcase.cpp
index d2be9a71..565699e0 100644
--- a/src/test/cpp/helpers/optionconvertertestcase.cpp
+++ b/src/test/cpp/helpers/optionconvertertestcase.cpp
@@ -49,6 +49,12 @@ LOGUNIT_CLASS(OptionConverterTestCase)
LOGUNIT_TEST(varSubstRecursiveReferenceTest);
LOGUNIT_TEST(toIntReturnsDefaultOnOverflow);
LOGUNIT_TEST(toIntReturnsDefaultOnMalformedInput);
+ LOGUNIT_TEST(toFileSizeAcceptsValidValues);
+ LOGUNIT_TEST(toFileSizeAcceptsWhitespaceSeparatedValues);
+ LOGUNIT_TEST(toFileSizeAcceptsLowercaseSuffixes);
+ LOGUNIT_TEST(toFileSizeReturnsDefaultOnMalformedInput);
+ LOGUNIT_TEST(toFileSizeReturnsDefaultOnOverflow);
+ LOGUNIT_TEST(toFileSizeBoundaryValues);
LOGUNIT_TEST(testTmpDir);
#if APR_HAS_USER
LOGUNIT_TEST(testUserHome);
@@ -184,6 +190,70 @@ public:
LOGUNIT_ASSERT_EQUAL((std::numeric_limits<int>::min)(),
OptionConverter::toInt(LOG4CXX_STR("-2147483648"), 7));
}
+ void toFileSizeAcceptsValidValues()
+ {
+ LOGUNIT_ASSERT_EQUAL(10L,
OptionConverter::toFileSize(LOG4CXX_STR("10"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L,
OptionConverter::toFileSize(LOG4CXX_STR("10B"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10 KB"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10MB"), 7));
+ }
+
+ void toFileSizeAcceptsWhitespaceSeparatedValues()
+ {
+ LOGUNIT_ASSERT_EQUAL(10L,
OptionConverter::toFileSize(LOG4CXX_STR("10 B"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10 KB"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10 MB"), 7));
+ const long tenGBExpected = ((std::numeric_limits<long>::max)()
>= (10LL * 1024LL * 1024LL * 1024LL))
+ ? static_cast<long>(10LL * 1024LL * 1024LL * 1024LL)
+ : 7L;
+ LOGUNIT_ASSERT_EQUAL(tenGBExpected,
OptionConverter::toFileSize(LOG4CXX_STR("10GB "), 7));
+ }
+
+ void toFileSizeAcceptsLowercaseSuffixes()
+ {
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10kb"), 7));
+ LOGUNIT_ASSERT_EQUAL(10L * 1024L * 1024L,
OptionConverter::toFileSize(LOG4CXX_STR("10mb"), 7));
+ const long tenGbLowerExpected =
((std::numeric_limits<long>::max)() >= (10LL * 1024LL * 1024LL * 1024LL))
+ ? static_cast<long>(10LL * 1024LL * 1024LL * 1024LL)
+ : 7L;
+ LOGUNIT_ASSERT_EQUAL(tenGbLowerExpected,
OptionConverter::toFileSize(LOG4CXX_STR("10gb"), 7));
+ }
+
+ void toFileSizeReturnsDefaultOnMalformedInput()
+ {
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("not-a-size"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("123abc"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("10XB"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("10QB"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("10K"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("10MBjunk"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("10 B junk"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("-1MB"), 7));
+ }
+
+ void toFileSizeReturnsDefaultOnOverflow()
+ {
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("9999999999999999999999"), 7));
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(LOG4CXX_STR("9999999999999999999999GB"), 7));
+ }
+
+ void toFileSizeBoundaryValues()
+ {
+ auto maxLongString =
std::to_string((std::numeric_limits<long>::max)());
+ LOG4CXX_DECODE_CHAR(lsMaxLong, maxLongString);
+ LOGUNIT_ASSERT_EQUAL((std::numeric_limits<long>::max)(),
OptionConverter::toFileSize(lsMaxLong, 7));
+
+ const unsigned long long onePastMaxLong = static_cast<unsigned
long long>((std::numeric_limits<long>::max)()) + 1ULL;
+ auto onePastMaxLongString = std::to_string(onePastMaxLong);
+ LOG4CXX_DECODE_CHAR(lsOnePastMaxLong, onePastMaxLongString);
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(lsOnePastMaxLong, 7));
+
+ const unsigned long long overflowBase = static_cast<unsigned
long long>((std::numeric_limits<long>::max)() / (1024LL * 1024LL * 1024LL)) +
1ULL;
+ auto overflowSizeString = std::to_string(overflowBase) + "GB";
+ LOG4CXX_DECODE_CHAR(lsOverflowAfterMultiply,
overflowSizeString);
+ LOGUNIT_ASSERT_EQUAL(7L,
OptionConverter::toFileSize(lsOverflowAfterMultiply, 7));
+ }
+
void testTmpDir()
{
LogString actual(OptionConverter::substVars(
