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 7af0ac4a Bound multiprocess map-file filename reads (#653)
7af0ac4a is described below
commit 7af0ac4a01d71c77f37bd2a52671e0cb7d54210c
Author: jmestwa-coder <[email protected]>
AuthorDate: Sat May 9 09:34:21 2026 +0530
Bound multiprocess map-file filename reads (#653)
---
src/main/cpp/timebasedrollingpolicy.cpp | 47 +++++++++++++++++----
src/test/cpp/rolling/timebasedrollingtest.cpp | 60 +++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/src/main/cpp/timebasedrollingpolicy.cpp
b/src/main/cpp/timebasedrollingpolicy.cpp
index 96893227..4ea8b22f 100644
--- a/src/main/cpp/timebasedrollingpolicy.cpp
+++ b/src/main/cpp/timebasedrollingpolicy.cpp
@@ -29,6 +29,7 @@
#include <log4cxx/helpers/optionconverter.h>
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/fileappender.h>
+#include <algorithm>
#include <iostream>
#include <apr_mmap.h>
@@ -121,6 +122,29 @@ struct
TimeBasedRollingPolicy::TimeBasedRollingPolicyPrivate{
#define MAX_FILE_LEN 2048
#if LOG4CXX_HAS_MULTIPROCESS_ROLLING_FILE_APPENDER
+namespace
+{
+LogString readMappedFileName(apr_mmap_t* mmap)
+{
+ if (!mmap || !mmap->mm)
+ {
+ return LogString();
+ }
+
+ const auto* first = static_cast<const logchar*>(mmap->mm);
+ const auto* last = first + (MAX_FILE_LEN / sizeof(logchar));
+ const auto* terminator = std::find(first, last, logchar(0));
+
+ if (terminator == last)
+ {
+ LogLog::warn(LOG4CXX_STR("Ignoring invalid multiprocess rolling
map file: missing string terminator"));
+ return LogString();
+ }
+
+ return LogString(first, terminator);
+}
+}
+
bool TimeBasedRollingPolicy::isMapFileEmpty(LOG4CXX_NS::helpers::Pool& pool)
{
apr_finfo_t finfo;
@@ -406,9 +430,10 @@ RolloverDescriptionPtr TimeBasedRollingPolicy::rollover(
if (m_priv->_mmap && !isMapFileEmpty(m_priv->_mmapPool))
{
lockMMapFile(APR_FLOCK_SHARED);
- LogString
mapLastFile(static_cast<logchar*>(m_priv->_mmap->mm));
- m_priv->lastFileName = mapLastFile;
+ LogString
mapLastFile(readMappedFileName(m_priv->_mmap));
unLockMMapFile();
+ if (!mapLastFile.empty())
+ m_priv->lastFileName = mapLastFile;
}
else
{
@@ -508,14 +533,18 @@ bool TimeBasedRollingPolicy::isTriggeringEvent(
if (m_priv->bRefreshCurFile && m_priv->_mmap &&
!isMapFileEmpty(m_priv->_mmapPool))
{
lockMMapFile(APR_FLOCK_SHARED);
- LogString
mapCurrent(static_cast<logchar*>(m_priv->_mmap->mm));
+ LogString mapCurrent(readMappedFileName(m_priv->_mmap));
unLockMMapFile();
- LogString mapCurrentBase(mapCurrent.substr(0,
mapCurrent.length() - m_priv->suffixLength));
- if (!mapCurrentBase.empty() && mapCurrentBase !=
filename)
+ if (!mapCurrent.empty() &&
static_cast<size_t>(m_priv->suffixLength) <= mapCurrent.length())
{
- if (auto fappend =
dynamic_cast<FileAppender*>(appender))
- fappend->setFile(mapCurrentBase);
+ LogString mapCurrentBase(mapCurrent.substr(0,
mapCurrent.length() - m_priv->suffixLength));
+
+ if (!mapCurrentBase.empty() && mapCurrentBase
!= filename)
+ {
+ if (auto fappend =
dynamic_cast<FileAppender*>(appender))
+
fappend->setFile(mapCurrentBase);
+ }
}
}
@@ -559,7 +588,7 @@ bool TimeBasedRollingPolicy::isLastFileNameUnchanged()
if (m_priv->_mmap)
{
lockMMapFile(APR_FLOCK_SHARED);
- LogString
mapCurrent(static_cast<logchar*>(m_priv->_mmap->mm));
+ LogString mapCurrent(readMappedFileName(m_priv->_mmap));
unLockMMapFile();
result = (mapCurrent == m_priv->lastFileName);
}
@@ -578,7 +607,7 @@ void TimeBasedRollingPolicy::loadLastFileName()
if (m_priv->_mmap)
{
lockMMapFile(APR_FLOCK_SHARED);
- LogString
mapLastFile(static_cast<logchar*>(m_priv->_mmap->mm));
+ LogString
mapLastFile(readMappedFileName(m_priv->_mmap));
unLockMMapFile();
if (!mapLastFile.empty())
m_priv->lastFileName = mapLastFile;
diff --git a/src/test/cpp/rolling/timebasedrollingtest.cpp
b/src/test/cpp/rolling/timebasedrollingtest.cpp
index 4c78d6aa..2c523956 100644
--- a/src/test/cpp/rolling/timebasedrollingtest.cpp
+++ b/src/test/cpp/rolling/timebasedrollingtest.cpp
@@ -31,8 +31,12 @@
#include "../util/compare.h"
#include "../logunit.h"
+#include <fstream>
#include <apr_strings.h>
#include <apr_time.h>
+#if LOG4CXX_HAS_MULTIPROCESS_ROLLING_FILE_APPENDER
+#include <apr_user.h>
+#endif
#include <random>
#ifndef INT64_C
#define INT64_C(x) x ## LL
@@ -83,6 +87,9 @@ LOGUNIT_CLASS(TimeBasedRollingTest)
LOGUNIT_TEST(test6);
LOGUNIT_TEST(test7);
LOGUNIT_TEST(rollIntoDir);
+#if LOG4CXX_HAS_MULTIPROCESS_ROLLING_FILE_APPENDER
+ LOGUNIT_TEST(unterminatedMultiprocessMapFile);
+#endif
LOGUNIT_TEST_SUITE_END();
private:
@@ -684,6 +691,59 @@ public:
this->checkFilesExist( pool, LOG4CXX_STR("test6."), fnames, 0,
__LINE__);
}
+#if LOG4CXX_HAS_MULTIPROCESS_ROLLING_FILE_APPENDER
+ void unterminatedMultiprocessMapFile()
+ {
+ Pool pool;
+ char uidBuffer[64] = "0000";
+#ifndef _WIN32
+ apr_uid_t uid;
+ apr_gid_t groupid;
+ if (APR_SUCCESS == apr_uid_current(&uid, &groupid,
pool.getAPRPool()))
+ apr_snprintf(uidBuffer, sizeof(uidBuffer), "%u", uid);
+#endif
+ std::string mapFile("output/rolling/tbr-map-corrupt-");
+ mapFile += uidBuffer;
+ mapFile += ".map";
+ std::string lockFile("output/rolling/tbr-map-corrupt-");
+ lockFile += uidBuffer;
+ lockFile += ".maplck";
+
+ File("output/rolling").mkdirs();
+ File(mapFile).deleteFile();
+ File(lockFile).deleteFile();
+
+ std::ofstream corruptMap(mapFile, std::ios::binary |
std::ios::trunc);
+ LOGUNIT_ASSERT(corruptMap.is_open());
+ std::string unterminatedMap(4096, 'x');
+ corruptMap.write(unterminatedMap.data(),
unterminatedMap.size());
+ corruptMap.close();
+
+ PatternLayoutPtr layout(new PatternLayout(PATTERN_LAYOUT));
+ RollingFileAppenderPtr rfa(new RollingFileAppender());
+ rfa->setAppend(false);
+ rfa->setLayout(layout);
+ rfa->setFile(LOG4CXX_STR("output/rolling/tbr-map-corrupt.log"));
+
+ TimeBasedRollingPolicyPtr tbrp(new TimeBasedRollingPolicy());
+ tbrp->setMultiprocess(true);
+
tbrp->setFileNamePattern(LOG4CXX_STR("output/rolling/tbr-map-corrupt-%d{"
DATE_PATTERN "}"));
+ tbrp->activateOptions();
+ rfa->setRollingPolicy(tbrp);
+ rfa->activateOptions();
+ logger->addAppender(rfa);
+
+ this->delayUntilNextSecond();
+ LOG4CXX_DEBUG(logger, "roll despite corrupt map");
+
LOGUNIT_ASSERT(File("output/rolling/tbr-map-corrupt.log").exists());
+
+ logger->removeAppender(rfa);
+ rfa->close();
+ File(mapFile).deleteFile();
+ File(lockFile).deleteFile();
+ }
+#endif
+
};
LOGUNIT_TEST_SUITE_REGISTRATION(TimeBasedRollingTest);
