This is an automated email from the ASF dual-hosted git repository. freeandnil pushed a commit to branch Feature/InvalidXmlCharacters in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
commit 1403bb78aca90a69de9055c19aa5f7e41f20fed7 Author: Jan Friedrich <[email protected]> AuthorDate: Tue Feb 17 08:59:08 2026 +0100 #280 harden the handling of invalid characters for the XmlLayout classes --- .../3.3.0/280-harden-invalid-characters.xml | 10 ++++ .../Layout/XmlLayoutSchemaLog4jTest.cs | 53 +++++++++++++++++++++- src/log4net.Tests/Layout/XmlLayoutTest.cs | 50 ++++++++++++++++++++ src/log4net.Tests/Util/TransformTest.cs | 2 +- src/log4net/Layout/Internal/XmlWriterExtensions.cs | 15 +++++- src/log4net/Layout/XmlLayout.cs | 45 +++++++++--------- src/log4net/Layout/XmlLayoutSchemaLog4j.cs | 38 ++++++++-------- src/log4net/Util/Transform.cs | 5 +- 8 files changed, 165 insertions(+), 53 deletions(-) diff --git a/src/changelog/3.3.0/280-harden-invalid-characters.xml b/src/changelog/3.3.0/280-harden-invalid-characters.xml new file mode 100644 index 00000000..512ab807 --- /dev/null +++ b/src/changelog/3.3.0/280-harden-invalid-characters.xml @@ -0,0 +1,10 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="https://logging.apache.org/xml/ns"; + xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="fixed"> + <issue id="280" link="https://github.com/apache/logging-log4net/pull/280"/> + <description format="asciidoc"> + harden the handling of invalid characters for the XmlLayout classes (by @FreeAndNil in https://github.com/apache/logging-log4net/pull/280[#280]) + </description> +</entry> \ No newline at end of file diff --git a/src/log4net.Tests/Layout/XmlLayoutSchemaLog4jTest.cs b/src/log4net.Tests/Layout/XmlLayoutSchemaLog4jTest.cs index d7d14de0..06587783 100644 --- a/src/log4net.Tests/Layout/XmlLayoutSchemaLog4jTest.cs +++ b/src/log4net.Tests/Layout/XmlLayoutSchemaLog4jTest.cs @@ -18,12 +18,11 @@ #endregion using System; - using log4net.Config; +using log4net.Core; using log4net.Layout; using log4net.Repository; using log4net.Tests.Appender; - using NUnit.Framework; namespace log4net.Tests.Layout @@ -63,5 +62,55 @@ void ThrowAndLog(int foo) } } } + + /// <summary> + /// Tests the serialization of invalid characters in the Properties dictionary + /// </summary> + [Test] + public void InvalidCharacterTest() + { + StringAppender stringAppender = new() { Layout = new XmlLayoutSchemaLog4J() }; + + ILoggerRepository repository = LogManager.CreateRepository(Guid.NewGuid().ToString()); + BasicConfigurator.Configure(repository, stringAppender); + ILog log = LogManager.GetLogger(repository.Name, "TestLogger"); + + Log(); + + string logEventXml = stringAppender.GetString(); + Assert.That(logEventXml, Does.Contain("us?er")); + Assert.That(logEventXml, Does.Contain("A?B")); + Assert.That(logEventXml, Does.Contain("Log?ger")); + Assert.That(logEventXml, Does.Contain("Thread?Name")); + Assert.That(logEventXml, Does.Contain("Do?main")); + Assert.That(logEventXml, Does.Contain("Ident?ity")); + Assert.That(logEventXml, Does.Contain("User?Name")); + Assert.That(logEventXml, Does.Contain("Mess?age")); + Assert.That(logEventXml, Does.Contain("oh?my")); + + void Log() + { + // Build a LoggingEvent with an XML invalid character in a property value + LoggingEventData data = new() + { + LoggerName = "Log\u0001ger", + Level = Level.Info, + TimeStampUtc = DateTime.UtcNow, + ThreadName = "Thread\u0001Name", + Domain = "Do\u0001main", + Identity = "Ident\u0001ity", + UserName = "User\u0001Name", + Message = "Mess\u0001age", + ExceptionString = "oh\u0001my", + Properties = new() + }; + + // Value contains U+0001 which is illegal in XML 1.0 + data.Properties["us\u0001er"] = "A\u0001B"; + + // Log the event + log.Logger.Log(new(null, repository, data)); + } + } } } \ No newline at end of file diff --git a/src/log4net.Tests/Layout/XmlLayoutTest.cs b/src/log4net.Tests/Layout/XmlLayoutTest.cs index 2f323a3b..3c0dc79d 100644 --- a/src/log4net.Tests/Layout/XmlLayoutTest.cs +++ b/src/log4net.Tests/Layout/XmlLayoutTest.cs @@ -371,4 +371,54 @@ void Bar(int foo) Assert.That(sub, Does.Not.Contain(">")); } } + + /// <summary> + /// Tests the serialization of invalid characters in the Properties dictionary + /// </summary> + [Test] + public void InvalidCharacterTest() + { + StringAppender stringAppender = new() { Layout = new XmlLayout() }; + + ILoggerRepository repository = LogManager.CreateRepository(Guid.NewGuid().ToString()); + BasicConfigurator.Configure(repository, stringAppender); + ILog log = LogManager.GetLogger(repository.Name, "TestLogger"); + + Log(); + + string logEventXml = stringAppender.GetString(); + Assert.That(logEventXml, Does.Contain("us?er")); + Assert.That(logEventXml, Does.Contain("A?B")); + Assert.That(logEventXml, Does.Contain("Log?ger")); + Assert.That(logEventXml, Does.Contain("Thread?Name")); + Assert.That(logEventXml, Does.Contain("Do?main")); + Assert.That(logEventXml, Does.Contain("Ident?ity")); + Assert.That(logEventXml, Does.Contain("User?Name")); + Assert.That(logEventXml, Does.Contain("Mess?age")); + Assert.That(logEventXml, Does.Contain("oh?my")); + + void Log() + { + // Build a LoggingEvent with an XML invalid character in a property value + LoggingEventData data = new() + { + LoggerName = "Log\u0001ger", + Level = Level.Info, + TimeStampUtc = DateTime.UtcNow, + ThreadName = "Thread\u0001Name", + Domain = "Do\u0001main", + Identity = "Ident\u0001ity", + UserName = "User\u0001Name", + Message = "Mess\u0001age", + ExceptionString = "oh\u0001my", + Properties = new() + }; + + // Value contains U+0001 which is illegal in XML 1.0 + data.Properties["us\u0001er"] = "A\u0001B"; + + // Log the event + log.Logger.Log(new(null, repository, data)); + } + } } \ No newline at end of file diff --git a/src/log4net.Tests/Util/TransformTest.cs b/src/log4net.Tests/Util/TransformTest.cs index 78b106cd..b0e932ac 100644 --- a/src/log4net.Tests/Util/TransformTest.cs +++ b/src/log4net.Tests/Util/TransformTest.cs @@ -40,4 +40,4 @@ public void MaskXmlInvalidCharactersMasks0Char() const string c = "\0"; Assert.That(Transform.MaskXmlInvalidCharacters(c, "?"), Is.EqualTo("?")); } -} +} \ No newline at end of file diff --git a/src/log4net/Layout/Internal/XmlWriterExtensions.cs b/src/log4net/Layout/Internal/XmlWriterExtensions.cs index e08b759d..91c46070 100644 --- a/src/log4net/Layout/Internal/XmlWriterExtensions.cs +++ b/src/log4net/Layout/Internal/XmlWriterExtensions.cs @@ -32,10 +32,11 @@ namespace log4net.Layout.Internal; internal static partial class XmlWriterExtensions { #if NETSTANDARD2_0_OR_GREATER - private static readonly XmlWriterSettings _settings = new XmlWriterSettings + private static readonly XmlWriterSettings _settings = new() { Indent = false, - OmitXmlDeclaration = true + OmitXmlDeclaration = true, + CheckCharacters = false }; #endif @@ -71,4 +72,14 @@ internal static XmlWriter CreateXmlWriter(TextWriter writer) Namespaces = false }; #endif + + /// <summary> + /// writes the attribute and replaces invalid characters + /// </summary> + internal static void WriteAttributeStringSafe(this XmlWriter writer, string localName, string? value, string mask) + { + if (string.IsNullOrEmpty(value)) + return; + writer.WriteAttributeString(localName, Transform.MaskXmlInvalidCharacters(value!, mask)); + } } \ No newline at end of file diff --git a/src/log4net/Layout/XmlLayout.cs b/src/log4net/Layout/XmlLayout.cs index 0233808c..90b59c1c 100644 --- a/src/log4net/Layout/XmlLayout.cs +++ b/src/log4net/Layout/XmlLayout.cs @@ -192,28 +192,30 @@ public override void ActivateOptions() protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) { writer.EnsureNotNull().WriteStartElement(_eventElementName, Prefix, DefaultEventElementName, Prefix); - writer.WriteAttributeString(LoggerAttributeName, loggingEvent.EnsureNotNull().LoggerName!); + writer.WriteAttributeStringSafe(LoggerAttributeName, loggingEvent.EnsureNotNull().LoggerName, InvalidCharReplacement); - writer.WriteAttributeString(TimestampAttributeName, XmlConvert.ToString(loggingEvent.TimeStamp, XmlDateTimeSerializationMode.Local)); + writer.WriteAttributeStringSafe(TimestampAttributeName, + XmlConvert.ToString(loggingEvent.TimeStamp, XmlDateTimeSerializationMode.Local), + InvalidCharReplacement); if (loggingEvent.Level is not null) { - writer.WriteAttributeString(LevelAttributeName, loggingEvent.Level.DisplayName); + writer.WriteAttributeStringSafe(LevelAttributeName, loggingEvent.Level.DisplayName, InvalidCharReplacement); } - writer.WriteAttributeString(ThreadAttributeName, loggingEvent.ThreadName!); + writer.WriteAttributeStringSafe(ThreadAttributeName, loggingEvent.ThreadName!, InvalidCharReplacement); if (loggingEvent.Domain is not null && loggingEvent.Domain.Length > 0) { - writer.WriteAttributeString(DomainAttributeName, loggingEvent.Domain); + writer.WriteAttributeStringSafe(DomainAttributeName, loggingEvent.Domain, InvalidCharReplacement); } if (loggingEvent.Identity is not null && loggingEvent.Identity.Length > 0) { - writer.WriteAttributeString(IdentityAttributeName, loggingEvent.Identity); + writer.WriteAttributeStringSafe(IdentityAttributeName, loggingEvent.Identity, InvalidCharReplacement); } if (loggingEvent.UserName.Length > 0) { - writer.WriteAttributeString(UsernameAttributeName, loggingEvent.UserName); + writer.WriteAttributeStringSafe(UsernameAttributeName, loggingEvent.UserName, InvalidCharReplacement); } // Append the message text @@ -222,13 +224,13 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) writer.WriteStartElement(_messageElementName, Prefix, DefaultMessageElementName, Prefix); if (!Base64EncodeMessage) { - Transform.WriteEscapedXmlString(writer, loggingEvent.RenderedMessage, InvalidCharReplacement); + writer.WriteEscapedXmlString(loggingEvent.RenderedMessage, InvalidCharReplacement); } else { byte[] messageBytes = Encoding.UTF8.GetBytes(loggingEvent.RenderedMessage); string base64Message = Convert.ToBase64String(messageBytes, 0, messageBytes.Length); - Transform.WriteEscapedXmlString(writer, base64Message, InvalidCharReplacement); + writer.WriteEscapedXmlString(base64Message, InvalidCharReplacement); } writer.WriteEndElement(); } @@ -242,22 +244,17 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) foreach (KeyValuePair<string, object?> entry in properties) { writer.WriteStartElement(_dataElementName, Prefix, DefaultDataElementName, Prefix); - writer.WriteAttributeString(NameAttributeName, Transform.MaskXmlInvalidCharacters(entry.Key, InvalidCharReplacement)); + writer.WriteAttributeStringSafe(NameAttributeName, entry.Key, InvalidCharReplacement); // Use an ObjectRenderer to convert the object to a string if (loggingEvent.Repository is not null) { - string valueStr; - if (!Base64EncodeProperties) + string valueStr = loggingEvent.Repository.RendererMap.FindAndRender(entry.Value); + if (Base64EncodeProperties) { - valueStr = Transform.MaskXmlInvalidCharacters(loggingEvent.Repository.RendererMap.FindAndRender(entry.Value), InvalidCharReplacement); + valueStr = Convert.ToBase64String(Encoding.UTF8.GetBytes(valueStr)); } - else - { - byte[] propertyValueBytes = Encoding.UTF8.GetBytes(loggingEvent.Repository.RendererMap.FindAndRender(entry.Value)); - valueStr = Convert.ToBase64String(propertyValueBytes, 0, propertyValueBytes.Length); - } - writer.WriteAttributeString(ValueAttributeName, valueStr); + writer.WriteAttributeStringSafe(ValueAttributeName, valueStr, InvalidCharReplacement); } writer.WriteEndElement(); @@ -270,7 +267,7 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) { // Append the stack trace line writer.WriteStartElement(_exceptionElementName, Prefix, DefaultExceptionElementName, Prefix); - Transform.WriteEscapedXmlString(writer, exceptionStr, InvalidCharReplacement); + writer.WriteEscapedXmlString(exceptionStr, InvalidCharReplacement); writer.WriteEndElement(); } @@ -279,10 +276,10 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) if (loggingEvent.LocationInformation is LocationInfo locationInfo) { writer.WriteStartElement(_locationElementName, Prefix, DefaultLocationElementName, Prefix); - writer.WriteAttributeString(ClassAttributeName, locationInfo.ClassName!); - writer.WriteAttributeString(MethodAttributeName, locationInfo.MethodName); - writer.WriteAttributeString(FileAttributeName, locationInfo.FileName!); - writer.WriteAttributeString(LineAttributeName, locationInfo.LineNumber); + writer.WriteAttributeStringSafe(ClassAttributeName, locationInfo.ClassName, InvalidCharReplacement); + writer.WriteAttributeStringSafe(MethodAttributeName, locationInfo.MethodName, InvalidCharReplacement); + writer.WriteAttributeStringSafe(FileAttributeName, locationInfo.FileName, InvalidCharReplacement); + writer.WriteAttributeStringSafe(LineAttributeName, locationInfo.LineNumber, InvalidCharReplacement); writer.WriteEndElement(); } } diff --git a/src/log4net/Layout/XmlLayoutSchemaLog4j.cs b/src/log4net/Layout/XmlLayoutSchemaLog4j.cs index ff2df82f..c8f1636f 100644 --- a/src/log4net/Layout/XmlLayoutSchemaLog4j.cs +++ b/src/log4net/Layout/XmlLayoutSchemaLog4j.cs @@ -47,8 +47,7 @@ public class XmlLayoutSchemaLog4J : XmlLayoutBase /// Constructs an XMLLayoutSchemaLog4j /// </summary> public XmlLayoutSchemaLog4J() - { - } + { } /// <summary> /// Constructs an XMLLayoutSchemaLog4j. @@ -67,9 +66,9 @@ public XmlLayoutSchemaLog4J() /// appender as well. /// </para> /// </remarks> - public XmlLayoutSchemaLog4J(bool locationInfo) : base(locationInfo) - { - } + public XmlLayoutSchemaLog4J(bool locationInfo) + : base(locationInfo) + { } /// <summary> /// The version of the log4j schema to use. @@ -137,8 +136,7 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) } // translate appdomain name - if (loggingEvent.LookupProperty("log4japp") is null - && loggingEvent.Domain?.Length > 0) + if (loggingEvent.LookupProperty("log4japp") is null && loggingEvent.Domain?.Length > 0) { loggingEvent.GetProperties()["log4japp"] = loggingEvent.Domain; } @@ -159,7 +157,7 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) // Write the start element writer.EnsureNotNull().WriteStartElement("log4j:event", "log4j", "event", "log4net"); - writer.WriteAttributeString("logger", loggingEvent.LoggerName); + writer.WriteAttributeStringSafe("logger", loggingEvent.LoggerName, InvalidCharReplacement); // Calculate the timestamp as the number of milliseconds since january 1970 // @@ -168,18 +166,18 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) // caused by daylight savings time transitions. TimeSpan timeSince1970 = loggingEvent.TimeStampUtc - _sDate1970; - writer.WriteAttributeString("timestamp", XmlConvert.ToString((long)timeSince1970.TotalMilliseconds)); + writer.WriteAttributeStringSafe("timestamp", XmlConvert.ToString((long)timeSince1970.TotalMilliseconds), InvalidCharReplacement); if (loggingEvent.Level is not null) { - writer.WriteAttributeString("level", loggingEvent.Level.DisplayName); + writer.WriteAttributeStringSafe("level", loggingEvent.Level.DisplayName, InvalidCharReplacement); } - writer.WriteAttributeString("thread", loggingEvent.ThreadName); + writer.WriteAttributeStringSafe("thread", loggingEvent.ThreadName, InvalidCharReplacement); // Append the message text if (loggingEvent.RenderedMessage is not null) { writer.WriteStartElement("log4j:message", "log4j", "message", "log4net"); - Transform.WriteEscapedXmlString(writer, loggingEvent.RenderedMessage, InvalidCharReplacement); + writer.WriteEscapedXmlString(loggingEvent.RenderedMessage, InvalidCharReplacement); writer.WriteEndElement(); } @@ -190,7 +188,7 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) { // Append the NDC text writer.WriteStartElement("log4j:NDC", "log4j", "NDC", "log4net"); - Transform.WriteEscapedXmlString(writer, valueStr!, InvalidCharReplacement); + writer.WriteEscapedXmlString(valueStr!, InvalidCharReplacement); writer.WriteEndElement(); } } @@ -203,13 +201,13 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) foreach (KeyValuePair<string, object?> entry in properties) { writer.WriteStartElement("log4j:data", "log4j", "data", "log4net"); - writer.WriteAttributeString("name", entry.Key); + writer.WriteAttributeStringSafe("name", entry.Key, InvalidCharReplacement); // Use an ObjectRenderer to convert the object to a string string? valueStr = loggingEvent.Repository?.RendererMap.FindAndRender(entry.Value); if (!string.IsNullOrEmpty(valueStr)) { - writer.WriteAttributeString("value", valueStr); + writer.WriteAttributeStringSafe("value", valueStr, InvalidCharReplacement); } writer.WriteEndElement(); @@ -222,7 +220,7 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) { // Append the stack trace line writer.WriteStartElement("log4j:throwable", "log4j", "throwable", "log4net"); - Transform.WriteEscapedXmlString(writer, exceptionStr!, InvalidCharReplacement); + writer.WriteEscapedXmlString(exceptionStr!, InvalidCharReplacement); writer.WriteEndElement(); } @@ -231,10 +229,10 @@ protected override void FormatXml(XmlWriter writer, LoggingEvent loggingEvent) if (loggingEvent.LocationInformation is LocationInfo locationInfo) { writer.WriteStartElement("log4j:locationInfo", "log4j", "locationInfo", "log4net"); - writer.WriteAttributeString("class", locationInfo.ClassName); - writer.WriteAttributeString("method", locationInfo.MethodName); - writer.WriteAttributeString("file", locationInfo.FileName); - writer.WriteAttributeString("line", locationInfo.LineNumber); + writer.WriteAttributeStringSafe("class", locationInfo.ClassName, InvalidCharReplacement); + writer.WriteAttributeStringSafe("method", locationInfo.MethodName, InvalidCharReplacement); + writer.WriteAttributeStringSafe("file", locationInfo.FileName, InvalidCharReplacement); + writer.WriteAttributeStringSafe("line", locationInfo.LineNumber, InvalidCharReplacement); writer.WriteEndElement(); } } diff --git a/src/log4net/Util/Transform.cs b/src/log4net/Util/Transform.cs index 6ff80086..b646974b 100644 --- a/src/log4net/Util/Transform.cs +++ b/src/log4net/Util/Transform.cs @@ -26,9 +26,6 @@ namespace log4net.Util; /// Utility class for transforming strings. /// </summary> /// <remarks> -/// <para> -/// Utility class for transforming strings. -/// </para> /// </remarks> /// <author>Nicko Cadell</author> /// <author>Gert Driesen</author> @@ -46,7 +43,7 @@ public static class Transform /// or using CDATA sections. /// </para> /// </remarks> - public static void WriteEscapedXmlString(XmlWriter writer, string textData, string invalidCharReplacement) + public static void WriteEscapedXmlString(this XmlWriter writer, string textData, string invalidCharReplacement) { writer.EnsureNotNull(); string stringData = MaskXmlInvalidCharacters(textData, invalidCharReplacement);
