This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch improve_large_string_handling in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit e44ffe1f1b3bc0a3f56bc3b04759265c88ec4676 Author: Stephen Webb <[email protected]> AuthorDate: Wed Feb 11 16:19:37 2026 +1100 Improve robustness when logging large message strings --- src/fuzzers/cpp/PatternParserFuzzer.cpp | 3 +-- src/main/cpp/formattinginfo.cpp | 4 ++- src/main/cpp/messagepatternconverter.cpp | 7 +++++- src/main/cpp/patternconverter.cpp | 9 +++++++ src/main/cpp/patternlayout.cpp | 26 +++++-------------- src/main/cpp/patternparser.cpp | 29 +++++++++++++--------- src/main/cpp/rollingpolicybase.cpp | 19 +++----------- .../include/log4cxx/pattern/patternconverter.h | 11 ++++++++ src/main/include/log4cxx/pattern/patternparser.h | 17 ++++++++++--- .../log4cxx/private/patternconverter_priv.h | 6 +++++ .../log4cxx/private/rollingpolicybase_priv.h | 5 ---- .../include/log4cxx/rolling/rollingpolicybase.h | 1 - src/test/cpp/pattern/patternparsertestcase.cpp | 13 +++------- src/test/cpp/patternlayouttest.cpp | 11 ++++++++ src/test/cpp/rolling/filenamepatterntestcase.cpp | 13 +++------- 15 files changed, 96 insertions(+), 78 deletions(-) diff --git a/src/fuzzers/cpp/PatternParserFuzzer.cpp b/src/fuzzers/cpp/PatternParserFuzzer.cpp index 4b1f2865..551aed7c 100644 --- a/src/fuzzers/cpp/PatternParserFuzzer.cpp +++ b/src/fuzzers/cpp/PatternParserFuzzer.cpp @@ -136,9 +136,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { log4cxx::helpers::Pool p; PatternMap patternMap = getFormatSpecifiers(); std::vector<PatternConverterPtr> converters; - std::vector<FormattingInfoPtr> fields; - PatternParser::parse(patternLogString, converters, fields, patternMap); + auto converters = PatternParser::parse(patternLogString, patternMap); return 0; } diff --git a/src/main/cpp/formattinginfo.cpp b/src/main/cpp/formattinginfo.cpp index 2f62545a..56ef54a8 100644 --- a/src/main/cpp/formattinginfo.cpp +++ b/src/main/cpp/formattinginfo.cpp @@ -79,7 +79,9 @@ FormattingInfoPtr FormattingInfo::getDefault() */ void FormattingInfo::format(const int fieldStart, LogString& buffer) const { - int rawLength = int(buffer.length() - fieldStart); + if (INT_MAX < buffer.length()) + return; + int rawLength = static_cast<int>(buffer.length()) - fieldStart; if (rawLength > m_priv->maxLength) { diff --git a/src/main/cpp/messagepatternconverter.cpp b/src/main/cpp/messagepatternconverter.cpp index 6b0712ff..3bf1697b 100644 --- a/src/main/cpp/messagepatternconverter.cpp +++ b/src/main/cpp/messagepatternconverter.cpp @@ -84,6 +84,11 @@ void MessagePatternConverter::format , helpers::Pool& /* p */ ) const { - toAppendTo.append(event->getRenderedMessage()); + auto& msg = event->getRenderedMessage(); + auto& info = getFormattingInfo(); + if (info.getMaxLength() < msg.length()) + toAppendTo.append(msg.substr(msg.length() - info.getMaxLength())); + else + toAppendTo.append(msg); } diff --git a/src/main/cpp/patternconverter.cpp b/src/main/cpp/patternconverter.cpp index 5fe92c3e..c1120da0 100644 --- a/src/main/cpp/patternconverter.cpp +++ b/src/main/cpp/patternconverter.cpp @@ -58,3 +58,12 @@ void PatternConverter::append(LogString& toAppendTo, const std::string& src) toAppendTo.append(decoded); } +const FormattingInfo& PatternConverter::getFormattingInfo() const +{ + return *m_priv->info; +} + +void PatternConverter::setFormattingInfo(const FormattingInfoPtr& newValue) +{ + m_priv->info = newValue; +} diff --git a/src/main/cpp/patternlayout.cpp b/src/main/cpp/patternlayout.cpp index 68d43c5e..6a29183d 100644 --- a/src/main/cpp/patternlayout.cpp +++ b/src/main/cpp/patternlayout.cpp @@ -75,11 +75,6 @@ struct PatternLayout::PatternLayoutPrivate */ LoggingEventPatternConverterList patternConverters; - /** - * Field widths and alignment corresponding to pattern converters. - */ - FormattingInfoList patternFields; - LogString m_fatalColor = LOG4CXX_STR("\\x1B[35m"); //magenta LogString m_errorColor = LOG4CXX_STR("\\x1B[31m"); //red LogString m_warnColor = LOG4CXX_STR("\\x1B[33m"); //yellow @@ -124,17 +119,13 @@ void PatternLayout::format(LogString& output, { auto& lsMsg = event->getRenderedMessage(); output.reserve(m_priv->expectedPatternLength + lsMsg.size()); - std::vector<FormattingInfoPtr>::const_iterator formatterIter = - m_priv->patternFields.begin(); - for (std::vector<LoggingEventPatternConverterPtr>::const_iterator - converterIter = m_priv->patternConverters.begin(); - converterIter != m_priv->patternConverters.end(); - converterIter++, formatterIter++) + for (auto item : m_priv->patternConverters) { - int startField = (int)output.length(); - (*converterIter)->format(event, output, pool); - (*formatterIter)->format(startField, output); + auto startField = output.length(); + item->format(event, output, pool); + if (startField < INT_MAX) + item->getFormattingInfo().format(static_cast<int>(startField), output); } } @@ -189,12 +180,7 @@ void PatternLayout::activateOptions(Pool&) } m_priv->patternConverters.erase(m_priv->patternConverters.begin(), m_priv->patternConverters.end()); - m_priv->patternFields.erase(m_priv->patternFields.begin(), m_priv->patternFields.end()); - std::vector<PatternConverterPtr> converters; - PatternParser::parse(pat, - converters, - m_priv->patternFields, - getFormatSpecifiers()); + auto converters = PatternParser::parse(pat, getFormatSpecifiers()); // // strip out any pattern converters that don't handle LoggingEvents diff --git a/src/main/cpp/patternparser.cpp b/src/main/cpp/patternparser.cpp index d27ba07e..ded7a37d 100644 --- a/src/main/cpp/patternparser.cpp +++ b/src/main/cpp/patternparser.cpp @@ -112,13 +112,23 @@ size_t PatternParser::extractOptions(const LogString& pattern, LogString::size_t return i; } +#if LOG4CXX_ABI_VERSION <= 15 void PatternParser::parse( const LogString& pattern, std::vector<PatternConverterPtr>& patternConverters, std::vector<FormattingInfoPtr>& formattingInfos, const PatternMap& rules) { + patternConverters = parse(pattern, rules); +} +#endif +PatternConverterList PatternParser::parse + ( const LogString& pattern + , const PatternMap& rules + ) +{ + PatternConverterList patternConverters; LogString currentLiteral; size_t patternLength = pattern.length(); @@ -126,7 +136,7 @@ void PatternParser::parse( logchar c; size_t i = 0; int minDigitCount{ 0 }, maxDigitCount{ 0 }; - FormattingInfoPtr formattingInfo(FormattingInfo::getDefault()); + auto formattingInfo = FormattingInfo::getDefault(); while (i < patternLength) { @@ -158,7 +168,6 @@ void PatternParser::parse( { patternConverters.push_back( LiteralPatternConverter::newInstance(currentLiteral)); - formattingInfos.push_back(FormattingInfo::getDefault()); currentLiteral.erase(currentLiteral.begin(), currentLiteral.end()); } @@ -205,7 +214,7 @@ void PatternParser::parse( { i = finalizeConverter( c, pattern, i, currentLiteral, formattingInfo, - rules, patternConverters, formattingInfos); + rules, patternConverters); // Next pattern is assumed to be a literal. state = LITERAL_STATE; @@ -239,7 +248,7 @@ void PatternParser::parse( { i = finalizeConverter( c, pattern, i, currentLiteral, formattingInfo, - rules, patternConverters, formattingInfos); + rules, patternConverters); state = LITERAL_STATE; formattingInfo = FormattingInfo::getDefault(); @@ -285,7 +294,7 @@ void PatternParser::parse( { i = finalizeConverter( c, pattern, i, currentLiteral, formattingInfo, - rules, patternConverters, formattingInfos); + rules, patternConverters); state = LITERAL_STATE; formattingInfo = FormattingInfo::getDefault(); @@ -304,8 +313,8 @@ void PatternParser::parse( { patternConverters.push_back( LiteralPatternConverter::newInstance(currentLiteral)); - formattingInfos.push_back(FormattingInfo::getDefault()); } + return patternConverters; } @@ -340,8 +349,7 @@ size_t PatternParser::finalizeConverter( logchar c, const LogString& pattern, size_t i, LogString& currentLiteral, const FormattingInfoPtr& formattingInfo, const PatternMap& rules, - std::vector<PatternConverterPtr>& patternConverters, - std::vector<FormattingInfoPtr>& formattingInfos) + PatternConverterList& patternConverters) { LogString convBuf; i = extractConverter(c, pattern, i, convBuf, currentLiteral); @@ -351,7 +359,6 @@ size_t PatternParser::finalizeConverter( LogLog::error(LOG4CXX_STR("Empty conversion specifier")); patternConverters.push_back( LiteralPatternConverter::newInstance(currentLiteral)); - formattingInfos.push_back(FormattingInfo::getDefault()); } else { @@ -372,18 +379,16 @@ size_t PatternParser::finalizeConverter( LogLog::error(msg); patternConverters.push_back( LiteralPatternConverter::newInstance(currentLiteral)); - formattingInfos.push_back(FormattingInfo::getDefault()); } else { patternConverters.push_back(pc); - formattingInfos.push_back(formattingInfo); + pc->setFormattingInfo(formattingInfo); if (currentLiteral.length() > 0) { patternConverters.push_back( LiteralPatternConverter::newInstance(currentLiteral)); - formattingInfos.push_back(FormattingInfo::getDefault()); } } } diff --git a/src/main/cpp/rollingpolicybase.cpp b/src/main/cpp/rollingpolicybase.cpp index e6630412..a4560753 100644 --- a/src/main/cpp/rollingpolicybase.cpp +++ b/src/main/cpp/rollingpolicybase.cpp @@ -95,12 +95,7 @@ LogString RollingPolicyBase::getFileNamePattern() const */ void RollingPolicyBase::parseFileNamePattern() { - m_priv->patternConverters.erase(m_priv->patternConverters.begin(), m_priv->patternConverters.end()); - m_priv->patternFields.erase(m_priv->patternFields.begin(), m_priv->patternFields.end()); - PatternParser::parse(m_priv->fileNamePatternStr, - m_priv->patternConverters, - m_priv->patternFields, - getFormatSpecifiers()); + m_priv->patternConverters = PatternParser::parse(m_priv->fileNamePatternStr, getFormatSpecifiers()); } /** @@ -114,17 +109,11 @@ void RollingPolicyBase::formatFileName( LogString& toAppendTo, Pool& pool) const { - std::vector<FormattingInfoPtr>::const_iterator formatterIter = - m_priv->patternFields.begin(); - - for (std::vector<PatternConverterPtr>::const_iterator - converterIter = m_priv->patternConverters.begin(); - converterIter != m_priv->patternConverters.end(); - converterIter++, formatterIter++) + for (auto item : m_priv->patternConverters) { auto startField = toAppendTo.length(); - (*converterIter)->format(obj, toAppendTo, pool); - (*formatterIter)->format((int)startField, toAppendTo); + item->format(obj, toAppendTo, pool); + item->getFormattingInfo().format((int)startField, toAppendTo); } } diff --git a/src/main/include/log4cxx/pattern/patternconverter.h b/src/main/include/log4cxx/pattern/patternconverter.h index 9bcf8c84..8e4852e5 100644 --- a/src/main/include/log4cxx/pattern/patternconverter.h +++ b/src/main/include/log4cxx/pattern/patternconverter.h @@ -21,6 +21,7 @@ #include <log4cxx/helpers/object.h> #include <log4cxx/logstring.h> +#include <log4cxx/pattern/formattinginfo.h> #include <vector> #define DECLARE_LOG4CXX_PATTERN(cls) DECLARE_ABSTRACT_LOG4CXX_OBJECT(cls) @@ -95,6 +96,16 @@ class LOG4CXX_EXPORT PatternConverter : public virtual helpers::Object */ virtual LogString getStyleClass(const helpers::ObjectPtr& e) const; + /** + * Provides a minimum width, a maximum width and the alignment + */ + const FormattingInfo& getFormattingInfo() const; + + /** + * Use \c newValue for the minimum width, a maximum width and the alignment + */ + void setFormattingInfo(const FormattingInfoPtr& newValue); + protected: /** * Appends content in the locale code page to a LogString. diff --git a/src/main/include/log4cxx/pattern/patternparser.h b/src/main/include/log4cxx/pattern/patternparser.h index 0cc6ba6e..bd49a216 100644 --- a/src/main/include/log4cxx/pattern/patternparser.h +++ b/src/main/include/log4cxx/pattern/patternparser.h @@ -33,7 +33,7 @@ namespace pattern typedef std::function<PatternConverterPtr(const std::vector<LogString>& options)> PatternConstructor; typedef std::map<LogString, PatternConstructor> PatternMap; - +LOG4CXX_LIST_DEF(PatternConverterList, PatternConverterPtr); // Contributors: Nelson Minar <([email protected]> // Igor E. Poteryaev <[email protected]> @@ -100,6 +100,7 @@ class LOG4CXX_EXPORT PatternParser std::vector<LogString>& options); public: +#if LOG4CXX_ABI_VERSION <= 15 /** * Parse a format specifier. * @param pattern pattern to parse. @@ -112,6 +113,16 @@ class LOG4CXX_EXPORT PatternParser std::vector<PatternConverterPtr>& patternConverters, std::vector<FormattingInfoPtr>& formattingInfos, const PatternMap& rules); +#endif + /** + * Convert \c pattern to converters using \c rules. + * @param pattern a string specifying the conversion types and their properties. + * @param rules a map of stock pattern converters keyed by name. + */ + static PatternConverterList parse + ( const LogString& pattern + , const PatternMap& rules + ); private: /** @@ -148,8 +159,8 @@ class LOG4CXX_EXPORT PatternParser logchar c, const LogString& pattern, size_t i, LogString& currentLiteral, const FormattingInfoPtr& formattingInfo, const PatternMap& rules, - std::vector<PatternConverterPtr>& patternConverters, - std::vector<FormattingInfoPtr>& formattingInfos); + PatternConverterList& patternConverter + ); static bool isUnicodeIdentifierStart(logchar c); static bool isUnicodeIdentifierPart(logchar c); diff --git a/src/main/include/log4cxx/private/patternconverter_priv.h b/src/main/include/log4cxx/private/patternconverter_priv.h index 0027ad5a..9e7b8aeb 100644 --- a/src/main/include/log4cxx/private/patternconverter_priv.h +++ b/src/main/include/log4cxx/private/patternconverter_priv.h @@ -17,6 +17,7 @@ #ifndef LOG4CXX_PATTERNCONVERTER_PRIVATE_H #define LOG4CXX_PATTERNCONVERTER_PRIVATE_H #include <log4cxx/pattern/patternconverter.h> +#include <log4cxx/pattern/formattinginfo.h> namespace LOG4CXX_NS { @@ -44,6 +45,11 @@ struct PatternConverter::PatternConverterPrivate * Converter style name. */ const LogString style; + + /** + * Provides field size limits + */ + FormattingInfoPtr info{ FormattingInfo::getDefault() }; }; } diff --git a/src/main/include/log4cxx/private/rollingpolicybase_priv.h b/src/main/include/log4cxx/private/rollingpolicybase_priv.h index 829bb036..467fa448 100644 --- a/src/main/include/log4cxx/private/rollingpolicybase_priv.h +++ b/src/main/include/log4cxx/private/rollingpolicybase_priv.h @@ -35,11 +35,6 @@ struct RollingPolicyBase::RollingPolicyBasePrivate { */ PatternConverterList patternConverters; - /** - * File name field specifiers. - */ - FormattingInfoList patternFields; - /** * File name pattern. */ diff --git a/src/main/include/log4cxx/rolling/rollingpolicybase.h b/src/main/include/log4cxx/rolling/rollingpolicybase.h index 9399286f..e2f85534 100644 --- a/src/main/include/log4cxx/rolling/rollingpolicybase.h +++ b/src/main/include/log4cxx/rolling/rollingpolicybase.h @@ -31,7 +31,6 @@ namespace LOG4CXX_NS namespace rolling { LOG4CXX_LIST_DEF(PatternConverterList, LOG4CXX_NS::pattern::PatternConverterPtr); -LOG4CXX_LIST_DEF(FormattingInfoList, LOG4CXX_NS::pattern::FormattingInfoPtr); /** * Implements methods common to most, it not all, rolling diff --git a/src/test/cpp/pattern/patternparsertestcase.cpp b/src/test/cpp/pattern/patternparsertestcase.cpp index 4a4d88c7..2c030198 100644 --- a/src/test/cpp/pattern/patternparsertestcase.cpp +++ b/src/test/cpp/pattern/patternparsertestcase.cpp @@ -170,20 +170,15 @@ public: const PatternMap & patternMap, const LogString & expected) { - std::vector<PatternConverterPtr> converters; - std::vector<FormattingInfoPtr> fields; - PatternParser::parse(pattern, converters, fields, patternMap); + auto converters = PatternParser::parse(pattern, patternMap); Pool p; LogString actual; - std::vector<FormattingInfoPtr>::const_iterator fieldIter = fields.begin(); - for (std::vector<PatternConverterPtr>::const_iterator converterIter = converters.begin(); - converterIter != converters.end(); - converterIter++, fieldIter++) + for (auto item : converters) { auto fieldStart = static_cast<int>(actual.length()); - (*converterIter)->format(event, actual, p); - (*fieldIter)->format(fieldStart, actual); + item->format(event, actual, p); + item->getFormattingInfo().format(fieldStart, actual); } LOGUNIT_ASSERT_EQUAL(expected, actual); diff --git a/src/test/cpp/patternlayouttest.cpp b/src/test/cpp/patternlayouttest.cpp index c142ae04..9b739b17 100644 --- a/src/test/cpp/patternlayouttest.cpp +++ b/src/test/cpp/patternlayouttest.cpp @@ -63,6 +63,7 @@ using namespace log4cxx::helpers; LOGUNIT_CLASS(PatternLayoutTest) { LOGUNIT_TEST_SUITE(PatternLayoutTest); + LOGUNIT_TEST(test2GbMessageFormatting); LOGUNIT_TEST(test1); LOGUNIT_TEST(test2); LOGUNIT_TEST(test3); @@ -103,6 +104,16 @@ public: } } + void test2GbMessageFormatting() + { + PatternLayout l{ LOG4CXX_STR("%p %.30m %p") }; + LogString output; + LogString msg(size_t(INT_MAX) + 1000, 'E'); + Pool p; + l.format(output, std::make_shared<spi::LoggingEvent>(logger->getName(), Level::getDebug(), msg, spi::LocationInfo::getLocationUnavailable()), p); + LOGUNIT_ASSERT_EQUAL(output, LOG4CXX_STR("DEBUG EEEEEEEEEEEEEEEEEEEEEEEEEEEEEE DEBUG")); + } + void test1() { auto status = PropertyConfigurator::configure(LOG4CXX_FILE("input/patternLayout1.properties")); diff --git a/src/test/cpp/rolling/filenamepatterntestcase.cpp b/src/test/cpp/rolling/filenamepatterntestcase.cpp index eb29cbba..01a834fa 100644 --- a/src/test/cpp/rolling/filenamepatterntestcase.cpp +++ b/src/test/cpp/rolling/filenamepatterntestcase.cpp @@ -66,23 +66,18 @@ public: LogString format(const LogString & pattern, const ObjectPtr & obj) { - std::vector<PatternConverterPtr> converters; - std::vector<FormattingInfoPtr> fields; PatternMap rules; rules.insert(PatternMap::value_type(LOG4CXX_STR("d"), (PatternConstructor) FileDatePatternConverter::newInstance)); rules.insert(PatternMap::value_type(LOG4CXX_STR("i"), (PatternConstructor) IntegerPatternConverter::newInstance)); - PatternParser::parse(pattern, converters, fields, rules); + auto converters = PatternParser::parse(pattern, rules); LogString result; Pool pool; - std::vector<FormattingInfoPtr>::const_iterator fieldIter = fields.begin(); - for (std::vector<PatternConverterPtr>::const_iterator converterIter = converters.begin(); - converterIter != converters.end(); - converterIter++, fieldIter++) + for (auto item : converters) { LogString::size_type i = result.length(); - (*converterIter)->format(obj, result, pool); - (*fieldIter)->format(i, result); + item->format(obj, result, pool); + item->getFormattingInfo().format((int)i, result); } return result;
