You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2025/11/23 08:05:04 UTC
(logging-log4cxx) 01/01: Prevent abnormal termination when a variable is recursively referenced
This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch prevent_recursive_vars
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 6d4931bdeb652ac1ded44ca845ad82dda15abd23
Author: Stephen Webb <sw...@gmail.com>
AuthorDate: Sun Nov 23 19:04:44 2025 +1100
Prevent abnormal termination when a variable is recursively referenced
---
src/main/cpp/optionconverter.cpp | 180 ++++++++++++++---------
src/test/cpp/helpers/optionconvertertestcase.cpp | 19 +++
2 files changed, 128 insertions(+), 71 deletions(-)
diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp
index 2b6cd619..17fc221d 100644
--- a/src/main/cpp/optionconverter.cpp
+++ b/src/main/cpp/optionconverter.cpp
@@ -44,6 +44,114 @@
#include <log4cxx/helpers/filewatchdog.h>
#include <log4cxx/helpers/singletonholder.h>
+namespace
+{
+using namespace LOG4CXX_NS;
+
+/// For recursion checking
+struct LogStringChain
+{
+ const LogString& item;
+ const LogStringChain* parent;
+};
+
+/// Is \c item referenced in \c path
+bool isRecursiveReference(const LogString& newkey, const LogStringChain* path)
+{
+ bool result = false;
+ if (path->item == newkey)
+ result = true;
+ else if (path->parent)
+ result = isRecursiveReference(newkey, path->parent);
+ return result;
+}
+
+LogString substVarsSafely(const LogString& val, helpers::Properties& props, const LogStringChain* path = 0)
+{
+ LogString sbuf;
+ const logchar delimStartArray[] = { 0x24, 0x7B, 0 }; // '$', '{'
+ const LogString delimStart(delimStartArray);
+ const logchar delimStop = 0x7D; // '}';
+ const size_t DELIM_START_LEN = 2;
+ const size_t DELIM_STOP_LEN = 1;
+
+ size_t i = 0;
+
+ while (true)
+ {
+ size_t j = val.find(delimStart, i);
+
+ if (j == val.npos)
+ {
+ // no more variables
+ if (i == 0)
+ {
+ // this is a simple string
+ return val;
+ }
+ else
+ {
+ // add the tail string which contails no variables and return the result.
+ sbuf.append(val.substr(i, val.length() - i));
+ return sbuf;
+ }
+ }
+ else
+ {
+ sbuf.append(val.substr(i, j - i));
+ size_t k = val.find(delimStop, j);
+
+ if (k == val.npos)
+ {
+ LogString msg(1, (logchar) 0x22 /* '\"' */);
+ msg.append(val);
+ msg.append(LOG4CXX_STR("\" has no closing brace. Opening brace at position "));
+ helpers::Pool p;
+ helpers::StringHelper::toString(j, p, msg);
+ msg.append(1, (logchar) 0x2E /* '.' */);
+ throw helpers::IllegalArgumentException(msg);
+ }
+ else
+ {
+ j += DELIM_START_LEN;
+ LogString key = val.substr(j, k - j);
+ if (path && isRecursiveReference(key, path))
+ {
+ LogString msg(LOG4CXX_STR("The variable ${"));
+ msg.append(key);
+ msg.append(LOG4CXX_STR("} is used recursively"));
+ throw helpers::IllegalArgumentException(msg);
+ }
+
+ // first try in System properties
+ LogString replacement(helpers::OptionConverter::getSystemProperty(key, LogString()));
+
+ // then try props parameter
+ if (replacement.empty())
+ {
+ replacement = props.getProperty(key);
+ }
+
+ if (!replacement.empty())
+ {
+ // Do variable substitution on the replacement string
+ // such that we can solve "Hello ${x2}" as "Hello p1"
+ // the where the properties are
+ // x1=p1
+ // x2=${x1}
+ LogStringChain current{ key, path };
+ LogString recursiveReplacement = substVarsSafely(replacement, props, ¤t);
+ sbuf.append(recursiveReplacement);
+ }
+
+ i = k + DELIM_STOP_LEN;
+ }
+ }
+ }
+}
+
+} // namespace
+
namespace LOG4CXX_NS
{
@@ -227,77 +335,7 @@ LogString OptionConverter::findAndSubst(const LogString& key, Properties& props)
LogString OptionConverter::substVars(const LogString& val, Properties& props)
{
- LogString sbuf;
- const logchar delimStartArray[] = { 0x24, 0x7B, 0 }; // '$', '{'
- const LogString delimStart(delimStartArray);
- const logchar delimStop = 0x7D; // '}';
- const size_t DELIM_START_LEN = 2;
- const size_t DELIM_STOP_LEN = 1;
-
- size_t i = 0;
-
- while (true)
- {
- size_t j = val.find(delimStart, i);
-
- if (j == val.npos)
- {
- // no more variables
- if (i == 0)
- {
- // this is a simple string
- return val;
- }
- else
- {
- // add the tail string which contails no variables and return the result.
- sbuf.append(val.substr(i, val.length() - i));
- return sbuf;
- }
- }
- else
- {
- sbuf.append(val.substr(i, j - i));
- size_t k = val.find(delimStop, j);
-
- if (k == val.npos)
- {
- LogString msg(1, (logchar) 0x22 /* '\"' */);
- msg.append(val);
- msg.append(LOG4CXX_STR("\" has no closing brace. Opening brace at position "));
- Pool p;
- StringHelper::toString(j, p, msg);
- msg.append(1, (logchar) 0x2E /* '.' */);
- throw IllegalArgumentException(msg);
- }
- else
- {
- j += DELIM_START_LEN;
- LogString key = val.substr(j, k - j);
- // first try in System properties
- LogString replacement(getSystemProperty(key, LogString()));
-
- // then try props parameter
- if (replacement.empty())
- {
- replacement = props.getProperty(key);
- }
-
- if (!replacement.empty())
- {
- // Do variable substitution on the replacement string
- // such that we can solve "Hello ${x2}" as "Hello p1"
- // the where the properties are
- // x1=p1
- // x2=${x1}
- LogString recursiveReplacement = substVars(replacement, props);
- sbuf.append(recursiveReplacement);
- }
-
- i = k + DELIM_STOP_LEN;
- }
- }
- }
+ return substVarsSafely(val, props);
}
LogString OptionConverter::getSystemProperty(const LogString& key, const LogString& def)
diff --git a/src/test/cpp/helpers/optionconvertertestcase.cpp b/src/test/cpp/helpers/optionconvertertestcase.cpp
index d3c128a8..e152fc9a 100644
--- a/src/test/cpp/helpers/optionconvertertestcase.cpp
+++ b/src/test/cpp/helpers/optionconvertertestcase.cpp
@@ -20,6 +20,7 @@
#include <log4cxx/helpers/system.h>
#include <log4cxx/helpers/transcoder.h>
#include <log4cxx/helpers/pool.h>
+#include <log4cxx/helpers/loglog.h>
#include "../testchar.h"
#include "../insertwide.h"
#include "../logunit.h"
@@ -44,6 +45,7 @@ LOGUNIT_CLASS(OptionConverterTestCase)
LOGUNIT_TEST(varSubstTest3);
LOGUNIT_TEST(varSubstTest4);
LOGUNIT_TEST(varSubstTest5);
+ LOGUNIT_TEST(varSubstRecursiveReferenceTest);
LOGUNIT_TEST(testTmpDir);
#if APR_HAS_USER
LOGUNIT_TEST(testUserHome);
@@ -144,6 +146,23 @@ public:
LOGUNIT_ASSERT_EQUAL((LogString) LOG4CXX_STR("x1"), res);
}
+ void varSubstRecursiveReferenceTest()
+ {
+ Properties props;
+ props.setProperty(LOG4CXX_STR("p1"), LOG4CXX_STR("${p2}"));
+ props.setProperty(LOG4CXX_STR("p2"), LOG4CXX_STR("${p1}"));
+ try
+ {
+ OptionConverter::substVars(LOG4CXX_STR("${p1}"), props);
+ LOGUNIT_ASSERT(false);
+ }
+ catch (IllegalArgumentException& ex)
+ {
+ LOG4CXX_ENCODE_CHAR(lsMsg, ex.what());
+ LogLog::debug(lsMsg);
+ }
+ }
+
void testTmpDir()
{
LogString actual(OptionConverter::substVars(